From 27a87689869eb61e5cd13b9357e4379fd74d0470 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 8 Sep 2026 10:16:11 +0200 Subject: [PATCH] Use clang-tidy style names in the decoder header and name the wire constants --- .../components/esphome/ota/ota_esphome.cpp | 59 +++++++++---------- esphome/components/esphome/ota/ota_esphome.h | 5 +- .../esphome/ota/ota_esphome_inflate.c | 4 +- .../esphome/ota/ota_esphome_inflate.h | 22 +++---- esphome/espota2.py | 15 +++-- 5 files changed, 56 insertions(+), 49 deletions(-) diff --git a/esphome/components/esphome/ota/ota_esphome.cpp b/esphome/components/esphome/ota/ota_esphome.cpp index 208f1409a6..4a2df014d7 100644 --- a/esphome/components/esphome/ota/ota_esphome.cpp +++ b/esphome/components/esphome/ota/ota_esphome.cpp @@ -44,6 +44,8 @@ const noise::NoiseContext &ESPHomeOTAComponent::noise_context_() const { static constexpr uint16_t OTA_BLOCK_SIZE = 8192; static constexpr uint32_t OTA_SOCKET_TIMEOUT_HANDSHAKE = 20000; // milliseconds for initial handshake static constexpr uint32_t OTA_SOCKET_TIMEOUT_DATA = 90000; // milliseconds for data transfer +static constexpr uint32_t OTA_PROGRESS_INTERVAL_MS = 1000; +static constexpr size_t OTA_SIZE_FIELD_BYTES = 4; // sizes on the wire are 4 bytes MSB first // Single-instance pointer — multi-port configs are rejected in final_validate. // NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) @@ -472,24 +474,13 @@ void ESPHomeOTAComponent::handle_data_() { } ESP_LOGV(TAG, "OTA type is 0x%02x", ota_type); - // Read size, 4 bytes MSB first - if (!this->data_readall_(buf, 4)) { - this->log_read_error_(LOG_STR("size")); + if (!this->read_size_(buf, xfer.ota_size, LOG_STR("size"))) goto error; // NOLINT(cppcoreguidelines-avoid-goto) - } - xfer.ota_size = encode_uint32(buf[0], buf[1], buf[2], buf[3]); - ESP_LOGV(TAG, "Size is %zu bytes", xfer.ota_size); image_size = xfer.ota_size; #ifdef USE_OTA_DEFLATE - if (this->inflate_ != nullptr) { - // A deflate upload also announces the inflated size, 4 bytes MSB first - if (!this->data_readall_(buf, 4)) { - this->log_read_error_(LOG_STR("image size")); - goto error; // NOLINT(cppcoreguidelines-avoid-goto) - } - image_size = encode_uint32(buf[0], buf[1], buf[2], buf[3]); - ESP_LOGV(TAG, "Inflated size is %zu bytes", image_size); - } + // A deflate upload also announces the inflated size + if (this->inflate_ != nullptr && !this->read_size_(buf, image_size, LOG_STR("image size"))) + goto error; // NOLINT(cppcoreguidelines-avoid-goto) #endif #ifndef USE_OTA_PARTITIONS @@ -746,6 +737,16 @@ bool ESPHomeOTAComponent::try_write_(size_t to_write, const LogString *desc) { return this->handshake_buf_pos_ >= to_write; } +bool ESPHomeOTAComponent::read_size_(uint8_t *buf, size_t &size, const LogString *desc) { + if (!this->data_readall_(buf, OTA_SIZE_FIELD_BYTES)) { + this->log_read_error_(desc); + return false; + } + size = encode_uint32(buf[0], buf[1], buf[2], buf[3]); + ESP_LOGV(TAG, "%s is %zu bytes", LOG_STR_ARG(desc), size); + return true; +} + ssize_t ESPHomeOTAComponent::receive_data_(uint8_t *buf, DataTransfer &xfer) { const size_t remaining = xfer.ota_size - xfer.total; const size_t requested = std::min(remaining, OTA_BUFFER_SIZE); @@ -795,7 +796,7 @@ ssize_t ESPHomeOTAComponent::receive_data_(uint8_t *buf, DataTransfer &xfer) { xfer.acknowledged += OTA_BLOCK_SIZE; } #endif - if (now - xfer.last_progress > 1000) { + if (now - xfer.last_progress > OTA_PROGRESS_INTERVAL_MS) { xfer.last_progress = now; float percentage = (xfer.total * 100.0f) / xfer.ota_size; ESP_LOGD(TAG, "Progress: %0.1f%%", percentage); @@ -809,29 +810,27 @@ ssize_t ESPHomeOTAComponent::receive_data_(uint8_t *buf, DataTransfer &xfer) { } #ifdef USE_OTA_DEFLATE -int ESPHomeOTAComponent::inflate_read_cb_(ota_inflate_state *d) { - // state is the first member, so the session is the same address (checked below) - auto *session = reinterpret_cast(d); - ssize_t read = session->self->receive_data_(session->in, *session->xfer); - if (read <= 0) - return -1; - d->source = session->in + 1; - d->source_limit = session->in + read; - return session->in[0]; -} - // The window doubles as the output buffer: the decoder fills it, we flush it to // the backend, and its bytes remain available as the back-reference history for // the next windowful. ota::OTAResponseTypes ESPHomeOTAComponent::inflate_data_(uint8_t *in, size_t image_size, DataTransfer &xfer) { - static_assert(offsetof(InflateSession, state) == 0, "inflate_read_cb_ recovers the session from &state"); + static_assert(offsetof(InflateSession, state) == 0, "the read callback recovers the session from &state"); InflateSession &session = *this->inflate_; - ota_inflate_state &state = session.state; + OtaInflateState &state = session.state; session.self = this; session.xfer = &xfer; session.in = in; ota_inflate_init(&state, session.window, OTA_INFLATE_WINDOW_SIZE); - state.source_read_cb = &ESPHomeOTAComponent::inflate_read_cb_; + // Pulls the next compressed chunk when the decoder runs dry + state.source_read_cb = [](OtaInflateState *d) -> int { + auto *s = reinterpret_cast(d); + ssize_t read = s->self->receive_data_(s->in, *s->xfer); + if (read <= 0) + return -1; + d->source = s->in + 1; + d->source_limit = s->in + read; + return s->in[0]; + }; size_t written = 0; int res; diff --git a/esphome/components/esphome/ota/ota_esphome.h b/esphome/components/esphome/ota/ota_esphome.h index 3187e20e6d..17bf2778d7 100644 --- a/esphome/components/esphome/ota/ota_esphome.h +++ b/esphome/components/esphome/ota/ota_esphome.h @@ -136,6 +136,8 @@ class ESPHomeOTAComponent final : public ota::OTAComponent { // the data timeout; updates xfer and sends chunk acks. Returns bytes read, -1 // on failure (logged). ssize_t receive_data_(uint8_t *buf, DataTransfer &xfer); + // Reads a 4 byte MSB first size field; buf must hold OTA_BUFFER_SIZE bytes + bool read_size_(uint8_t *buf, size_t &size, const LogString *desc); bool try_read_(size_t to_read, const LogString *desc); bool try_write_(size_t to_write, const LogString *desc); @@ -196,13 +198,12 @@ class ESPHomeOTAComponent final : public ota::OTAComponent { static constexpr size_t OTA_INFLATE_WINDOW_SIZE = 4096; // Heap-allocated only while a deflate-compressed upload is negotiated. struct InflateSession { - ota_inflate_state state; // first member: the read callback casts back from it + OtaInflateState state; // first member: the read callback casts back from it ESPHomeOTAComponent *self; DataTransfer *xfer; uint8_t *in; // caller's buffer for the compressed input, valid during inflate_data_ uint8_t window[OTA_INFLATE_WINDOW_SIZE]; }; - static int inflate_read_cb_(ota_inflate_state *d); ota::OTAResponseTypes inflate_data_(uint8_t *in, size_t image_size, DataTransfer &xfer); std::unique_ptr inflate_; #endif diff --git a/esphome/components/esphome/ota/ota_esphome_inflate.c b/esphome/components/esphome/ota/ota_esphome_inflate.c index 8494fb276c..95603fa995 100644 --- a/esphome/components/esphome/ota/ota_esphome_inflate.c +++ b/esphome/components/esphome/ota/ota_esphome_inflate.c @@ -45,8 +45,8 @@ #define TINF_DONE OTA_INFLATE_DONE #define TINF_DATA_ERROR OTA_INFLATE_DATA_ERROR #define TINF_DICT_ERROR OTA_INFLATE_DICT_ERROR -#define TINF_DATA struct ota_inflate_state -#define TINF_TREE ota_inflate_tree_t +#define TINF_DATA struct OtaInflateState +#define TINF_TREE struct OtaInflateTree #define TINF_ARRAY_SIZE(arr) (sizeof(arr) / sizeof(*(arr))) /* every output byte also goes into the ring window */ diff --git a/esphome/components/esphome/ota/ota_esphome_inflate.h b/esphome/components/esphome/ota/ota_esphome_inflate.h index 61d82e1489..ebceef357f 100644 --- a/esphome/components/esphome/ota/ota_esphome_inflate.h +++ b/esphome/components/esphome/ota/ota_esphome_inflate.h @@ -11,25 +11,25 @@ extern "C" { #endif -enum ota_inflate_result { +enum OtaInflateResult { OTA_INFLATE_OK = 0, /* more data produced, call again */ OTA_INFLATE_DONE = 1, /* end of compressed stream reached */ OTA_INFLATE_DATA_ERROR = -3, OTA_INFLATE_DICT_ERROR = -5, }; -typedef struct { - unsigned short table[16]; /* table of code length counts */ - unsigned short trans[288]; /* code -> symbol translation table */ -} ota_inflate_tree_t; +struct OtaInflateTree { + uint16_t table[16]; /* table of code length counts */ + uint16_t trans[288]; /* code -> symbol translation table */ +}; -struct ota_inflate_state { +struct OtaInflateState { /* Next byte in the input buffer and one past its end */ const unsigned char *source; const unsigned char *source_limit; /* Called when source is exhausted; returns the next byte or -1 at EOF. It may refill source/source_limit for buffered operation. */ - int (*source_read_cb)(struct ota_inflate_state *d); + int (*source_read_cb)(struct OtaInflateState *d); unsigned int tag; unsigned int bitcount; @@ -49,14 +49,14 @@ struct ota_inflate_state { unsigned int dict_size; unsigned int dict_idx; - ota_inflate_tree_t ltree; /* dynamic length/symbol tree */ - ota_inflate_tree_t dtree; /* dynamic distance tree */ + struct OtaInflateTree ltree; /* dynamic length/symbol tree */ + struct OtaInflateTree dtree; /* dynamic distance tree */ }; /* dict must be at least as large as the window the encoder used (its max back reference distance) */ -void ota_inflate_init(struct ota_inflate_state *d, unsigned char *dict, unsigned int dict_len); +void ota_inflate_init(struct OtaInflateState *d, unsigned char *dict, unsigned int dict_len); /* Produce output until dest reaches dest_limit (OK), the stream ends (DONE) or an error occurs */ -int ota_inflate(struct ota_inflate_state *d); +int ota_inflate(struct OtaInflateState *d); #ifdef __cplusplus } diff --git a/esphome/espota2.py b/esphome/espota2.py index 529763aeac..0220815a75 100644 --- a/esphome/espota2.py +++ b/esphome/espota2.py @@ -94,6 +94,9 @@ _SUPPORTED_OTA_TYPES: frozenset[int] = frozenset( ) UPLOAD_BLOCK_SIZE = 8192 +# Sizes on the wire are 4 bytes MSB first +SIZE_FIELD_BYTES = 4 +COMPRESS_LEVEL = 9 UPLOAD_BUFFER_SIZE = UPLOAD_BLOCK_SIZE * 8 # Flaky Wi-Fi links often drop the first OTA attempt, and the device may need time @@ -651,11 +654,13 @@ def perform_ota( deflate = False if features & SERVER_FEATURE_SUPPORTS_COMPRESSION: # The device stores the gzip file and inflates it when it reboots - upload_contents = gzip.compress(file_contents, compresslevel=9) + upload_contents = gzip.compress(file_contents, compresslevel=COMPRESS_LEVEL) _LOGGER.info("Compressed to %s bytes", len(upload_contents)) elif extended_proto and features & SERVER_FEATURE_SUPPORTS_DEFLATE: # The device inflates while receiving through a small ring window - compressor = zlib.compressobj(9, zlib.DEFLATED, -DEFLATE_WINDOW_BITS) + compressor = zlib.compressobj( + COMPRESS_LEVEL, zlib.DEFLATED, -DEFLATE_WINDOW_BITS + ) upload_contents = compressor.compress(file_contents) + compressor.flush() deflate = True _LOGGER.info("Compressed to %s bytes (deflate)", len(upload_contents)) @@ -717,7 +722,7 @@ def perform_ota( send_check(sock, ota_type, "ota type") upload_size = len(upload_contents) - upload_size_encoded = upload_size.to_bytes(4, "big") + upload_size_encoded = upload_size.to_bytes(SIZE_FIELD_BYTES, "big") # The device erases flash between receiving the size and acking the # prepare, so this window shows the erase cost (near zero when the # device erases lazily during the upload) @@ -726,7 +731,9 @@ def perform_ota( if deflate: # The device sizes the partition by the inflated image; its own frame, # as an encrypted session carries one field per frame - send_check(sock, len(file_contents).to_bytes(4, "big"), "image size") + send_check( + sock, len(file_contents).to_bytes(SIZE_FIELD_BYTES, "big"), "image size" + ) receive_exactly(sock, 1, "update prepare result", RESPONSE_UPDATE_PREPARE_OK) prepare_duration = time.perf_counter() - prepare_start _LOGGER.info("Preparing for upload took %.2f seconds", prepare_duration)