diff --git a/esphome/components/esphome/ota/__init__.py b/esphome/components/esphome/ota/__init__.py index 816cb7b6c8..1aced2847b 100644 --- a/esphome/components/esphome/ota/__init__.py +++ b/esphome/components/esphome/ota/__init__.py @@ -308,9 +308,7 @@ async def to_code(config: ConfigType) -> None: if config.get(CONF_ALLOW_PARTITION_ACCESS): cg.add_define("USE_OTA_PARTITIONS") - # ESP8266 and RP2040 inflate a gzip image at reboot from their bootloader - # or OTA stub; every other platform inflates a deflate stream on the fly - # while it receives the image + # ESP8266 and RP2040 inflate gzip at reboot; the rest inflate on the fly if not (CORE.is_esp8266 or CORE.is_rp2): cg.add_define("USE_OTA_DEFLATE") diff --git a/esphome/components/esphome/ota/ota_esphome.cpp b/esphome/components/esphome/ota/ota_esphome.cpp index e39da87716..68e048eeed 100644 --- a/esphome/components/esphome/ota/ota_esphome.cpp +++ b/esphome/components/esphome/ota/ota_esphome.cpp @@ -189,9 +189,8 @@ static constexpr uint8_t CLIENT_NOISE_FEATURES = static constexpr uint8_t SERVER_FEATURE_SUPPORTS_COMPRESSION = 0x01; static constexpr uint8_t SERVER_FEATURE_SUPPORTS_PARTITION_ACCESS = 0x02; static constexpr uint8_t SERVER_FEATURE_SUPPORTS_NOISE = 0x04; -// The device inflates a raw deflate stream (window <= OTA_INFLATE_WINDOW_SIZE). -// The offer is binding: a client that asked for it must then send the inflated -// size frame and a deflate stream, the device does not check again. +// Raw deflate, window <= OTA_INFLATE_WINDOW_SIZE. Binding once offered: the +// client must then send the image size frame and a deflate stream. static constexpr uint8_t SERVER_FEATURE_SUPPORTS_DEFLATE = 0x08; inline bool ESPHomeOTAComponent::extended_proto_() const { @@ -320,8 +319,7 @@ void ESPHomeOTAComponent::handle_handshake_() { this->handshake_buf_[1] |= SERVER_FEATURE_SUPPORTS_NOISE; #endif #ifdef USE_OTA_DEFLATE - // Offer to inflate on the fly once the session memory (a few KB) is in - // hand; otherwise the upload stays uncompressed + // Offered only once the session memory is in hand; else uncompressed if ((this->ota_features_ & CLIENT_FEATURE_SUPPORTS_DEFLATE) != 0) { this->inflate_.reset(new (std::nothrow) InflateSession()); if (this->inflate_ != nullptr) { @@ -478,7 +476,6 @@ void ESPHomeOTAComponent::handle_data_() { goto error; // NOLINT(cppcoreguidelines-avoid-goto) image_size = xfer.ota_size; #ifdef USE_OTA_DEFLATE - // 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 @@ -820,9 +817,8 @@ void ESPHomeOTAComponent::send_chunk_acks_(DataTransfer &xfer) { } #ifdef USE_OTA_DEFLATE -// 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. +// The window doubles as the output buffer; flushed bytes stay as back +// reference history for the next windowful. ota::OTAResponseTypes ESPHomeOTAComponent::inflate_flush_(InflateSession &session) { const size_t produced = session.dest - session.window; const size_t pending = produced - session.flushed; @@ -850,9 +846,8 @@ ota::OTAResponseTypes ESPHomeOTAComponent::inflate_data_(uint8_t *in, size_t ima session.written = 0; session.error = ota::OTA_RESPONSE_OK; ota_inflate_init(&session, session.window, OTA_INFLATE_WINDOW_SIZE); - // Pulls the next compressed chunk when the decoder runs dry. Where the ack - // must follow the write, everything decoded so far is written and acked - // first, or the client would wait for an ack while the decoder waits for it. + // Where the ack must follow the write, flush and ack before waiting for + // input, or the client waits for an ack while the decoder waits for data session.source_read_cb = [](OtaInflateState *d) -> int { auto *s = static_cast(d); if (ACK_AFTER_WRITE) { @@ -860,7 +855,7 @@ ota::OTAResponseTypes ESPHomeOTAComponent::inflate_data_(uint8_t *in, size_t ima if (s->error != ota::OTA_RESPONSE_OK) return -1; } - // The stream wants more than announced; the size check below reports it + // More input than announced; reported by the size check below if (s->xfer->total >= s->xfer->ota_size) return -1; ssize_t read = s->self->receive_data_(s->in, *s->xfer); @@ -876,14 +871,12 @@ ota::OTAResponseTypes ESPHomeOTAComponent::inflate_data_(uint8_t *in, size_t ima int res; do { - // The ring index wrapped to 0 exactly when the window filled, so the - // window and the output cursor stay in lockstep + // The ring index wrapped to 0 exactly when the window filled session.dest = session.window; session.dest_limit = session.window + OTA_INFLATE_WINDOW_SIZE; session.flushed = 0; res = ota_inflate(&session); - // A stored block keeps decoding zeros after the read callback failed, so - // eof is checked as well + // A stored block keeps emitting zeros after a failed read, hence eof if (res < 0 || session.eof) break; session.error = this->inflate_flush_(session); diff --git a/esphome/components/esphome/ota/ota_esphome.h b/esphome/components/esphome/ota/ota_esphome.h index 2ddf33cfe0..fcb276bdb5 100644 --- a/esphome/components/esphome/ota/ota_esphome.h +++ b/esphome/components/esphome/ota/ota_esphome.h @@ -132,12 +132,10 @@ class ESPHomeOTAComponent final : public ota::OTAComponent { uint32_t last_data_ms{0}; uint32_t last_progress{0}; }; - // Receives up to OTA_BUFFER_SIZE bytes of upload data into buf, waiting up to - // the data timeout, and updates xfer. Returns bytes read, -1 on failure (logged). + // Up to OTA_BUFFER_SIZE bytes into buf; returns bytes read, -1 on failure (logged) inline ssize_t receive_data_(uint8_t *buf, DataTransfer &xfer); - // When lwIP runs in this loop the radio is deaf while a sector is written, so - // the client is kept quiet until the block is in flash; with a socket task the - // ack goes out on receipt so the next block arrives while this one is written + // Raw lwIP cannot service the radio during a sector write, so the ack waits + // for the write there; a socket task lets the next block arrive meanwhile #ifdef USE_SOCKET_IMPL_LWIP_TCP static constexpr bool ACK_AFTER_WRITE = true; #else @@ -209,12 +207,10 @@ class ESPHomeOTAComponent final : public ota::OTAComponent { "OTA_BUFFER_SIZE must fit a full encrypted data frame"); #endif #ifdef USE_OTA_DEFLATE - // Deflate back references reach 1 << espota2.DEFLATE_WINDOW_BITS bytes; the - // ring window must be at least that. It also serves as the inflate output - // buffer, so it is flushed to the backend one windowful at a time. + // At least 1 << espota2.DEFLATE_WINDOW_BITS; also the inflate output buffer static constexpr size_t OTA_INFLATE_WINDOW_SIZE = 4096; - // Heap-allocated only while a deflate-compressed upload is negotiated; the - // decoder state is the base so its read callback can recover the session + // Heap-allocated only while a deflate upload is negotiated; the decoder + // state is the base so the read callback can recover the session struct InflateSession : OtaInflateState { ESPHomeOTAComponent *self; DataTransfer *xfer; @@ -229,7 +225,7 @@ class ESPHomeOTAComponent final : public ota::OTAComponent { static_assert(!ota::OTABackend::supports_compression(), "USE_OTA_DEFLATE is for backends that cannot store a gzip image"); #endif - // Writes the decoded bytes not yet in flash; dest stays put so the ring history is intact + // Writes the decoded bytes not yet in flash without moving dest ota::OTAResponseTypes inflate_flush_(InflateSession &session); ota::OTAResponseTypes inflate_data_(uint8_t *in, size_t image_size, DataTransfer &xfer); std::unique_ptr inflate_; diff --git a/esphome/components/esphome/ota/ota_esphome_inflate.h b/esphome/components/esphome/ota/ota_esphome_inflate.h index 15307b5b1f..197e20e42d 100644 --- a/esphome/components/esphome/ota/ota_esphome_inflate.h +++ b/esphome/components/esphome/ota/ota_esphome_inflate.h @@ -1,8 +1,6 @@ #pragma once -// Raw deflate decoder for compressed OTA uploads, cut down from uzlib -// (https://github.com/pfalcon/uzlib, zlib licence, see the .c file). -// Kept in C so it stays close to upstream; the decoder writes through a -// ring window so the image never has to be held in RAM. +// Raw deflate decoder cut down from uzlib (https://github.com/pfalcon/uzlib, +// zlib licence, see the .c file); output goes through a ring window. #include #include @@ -56,9 +54,9 @@ struct OtaInflateState { uint16_t dtrans[32]; /* the distance alphabet has 30 symbols, so the tree is kept small */ }; -/* dict must be at least as large as the window the encoder used (its max back reference distance) */ +/* dict must cover the encoder's window (its largest back reference) */ 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 */ +/* Fills dest up to dest_limit (OK) or to the end of the stream (DONE) */ int ota_inflate(struct OtaInflateState *d); #ifdef __cplusplus diff --git a/esphome/components/ota/ota_backend.h b/esphome/components/ota/ota_backend.h index 09612d5592..a505d58efe 100644 --- a/esphome/components/ota/ota_backend.h +++ b/esphome/components/ota/ota_backend.h @@ -103,9 +103,8 @@ enum OTAType : uint8_t { // - set_update_md5: expected digest of the incoming image, hex string. // - write: consume the next chunk; end: finalize and mark bootable. // - abort: safe to call in any state, including after end(). -// - supports_compression: static constexpr, whether a gzip image can be stored -// as is and inflated at reboot; the esphome platform asserts it at compile -// time when it builds its own inflater. +// - supports_compression: constexpr, whether a gzip image is stored as is and +// inflated at reboot. template concept OTABackendContract = requires(T backend, size_t image_size, uint8_t *data, size_t len, const char *md5) { { backend.begin(image_size, OTA_TYPE_UPDATE_APP) } -> std::same_as; diff --git a/esphome/components/ota/ota_backend_arduino_rp2.h b/esphome/components/ota/ota_backend_arduino_rp2.h index 8874800609..fd72685823 100644 --- a/esphome/components/ota/ota_backend_arduino_rp2.h +++ b/esphome/components/ota/ota_backend_arduino_rp2.h @@ -15,11 +15,9 @@ class ArduinoRP2OTABackend final { OTAResponseTypes write(uint8_t *data, size_t len); OTAResponseTypes end(); void abort(); - // A gzip image is staged on LittleFS as is; the core's OTA stub inflates it - // into the app region at reboot (arduino-pico 2.4.0 and later, RP2350 from - // 4.0.3; ESPHome requires 6.0.0), the same way the ESP8266 bootloader does. - // begin() then sees the gzip size, so only the staging space is checked up - // front; the inflated size is not known until the stub reads the trailer. + // The core's OTA stub inflates a staged gzip image at reboot (arduino-pico + // 2.4.0, RP2350 4.0.3; ESPHome pins 6.0.0). begin() only sees the gzip + // size; the inflated size is known when the stub reads the trailer. static constexpr bool supports_compression() { return true; } private: diff --git a/esphome/espota2.py b/esphome/espota2.py index 9d4f94dde9..79d494edc0 100644 --- a/esphome/espota2.py +++ b/esphome/espota2.py @@ -70,13 +70,10 @@ CLIENT_FEATURE_SUPPORTS_DEFLATE = 0x10 SERVER_FEATURE_SUPPORTS_COMPRESSION = 0x01 SERVER_FEATURE_SUPPORTS_PARTITION_ACCESS = 0x02 SERVER_FEATURE_SUPPORTS_NOISE = 0x04 -# Binding once offered: the device then expects the image size frame and a -# deflate stream, so there is no opting out per upload +# Binding once offered: the device then expects the image size and a deflate stream SERVER_FEATURE_SUPPORTS_DEFLATE = 0x08 -# Window of the raw deflate stream sent to a device that inflates on the fly. -# Part of the protocol: the server's deflate bit promises a 4 KB ring window -# (OTA_INFLATE_WINDOW_SIZE), so a larger window needs a new feature bit +# Wire constant: the deflate bit promises a 4 KB window (OTA_INFLATE_WINDOW_SIZE) DEFLATE_WINDOW_BITS = 12 NOISE_FRAME_INDICATOR = 0x01 @@ -729,15 +726,13 @@ def perform_ota( prepare_start = time.perf_counter() send_check(sock, upload_size.to_bytes(SIZE_FIELD_BYTES, "big"), "binary size") if deflate: - # The device sizes the partition by the inflated image; its own frame, - # as an encrypted session carries one field per frame + # Own frame: an encrypted session carries one field per frame send_check(sock, file_size.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) - # The device hashes what it writes to flash: the inflated image for a - # deflate upload, the received bytes otherwise (the gzip file on ESP8266) + # The device hashes what it writes: the inflated image, else the received bytes upload_md5 = hashlib.md5(file_contents if deflate else upload_contents).hexdigest() _LOGGER.debug("MD5 of upload is %s", upload_md5) diff --git a/tests/components/esphome/__init__.py b/tests/components/esphome/__init__.py index 80bdc5c4a5..713c0d5a63 100644 --- a/tests/components/esphome/__init__.py +++ b/tests/components/esphome/__init__.py @@ -3,10 +3,9 @@ from tests.testing_helpers import ComponentManifestOverride def override_manifest(manifest: ComponentManifestOverride) -> None: - # to_code must run: it emits the component count the application needs + # to_code emits the component count the application needs manifest.enable_codegen() - # The deflate decoder lives with the ota platform, which is not part of - # this build; only the decoder itself is under test here + # Only the decoder is under test; its ota platform is not in this build manifest.resources = manifest.resources + [ FileResource("esphome.components.esphome", "ota/ota_esphome_inflate.c"), FileResource("esphome.components.esphome", "ota/ota_esphome_inflate.h"), diff --git a/tests/components/esphome/test_ota_inflate.cpp b/tests/components/esphome/test_ota_inflate.cpp index b40b64373b..7a106ce08e 100644 --- a/tests/components/esphome/test_ota_inflate.cpp +++ b/tests/components/esphome/test_ota_inflate.cpp @@ -8,11 +8,7 @@ namespace esphome::testing { -// The plaintext below, as built by build_plain(): repeated text, a pseudo random -// run, a zero run and the text again, so literals, short and long back references -// and stored data are all exercised across the 4 KB window. Regenerate with the -// window the CLI uses (espota2.DEFLATE_WINDOW_BITS): -// plain = build_plain() written out by the same recipe in Python +// build_plain() compressed with the CLI's window (espota2.DEFLATE_WINDOW_BITS): // DEFLATED = zlib.compress(plain, 9, wbits=-12) // STORED = zlib.compress(plain[:300], 0, wbits=-12) static const uint8_t DEFLATED[] = { @@ -185,7 +181,6 @@ static const uint8_t DEFLATED[] = { 0x90, 0x3b, 0x14, 0x38, 0xe0, 0x80, 0x03, 0x0e, 0x38, 0xe0, 0x80, 0xfb, 0xff, 0xba, 0xff, 0x00, }; -// The first 300 bytes of the same plaintext as a stored (uncompressed) block static const uint8_t STORED[] = { 0x01, 0x2c, 0x01, 0xd3, 0xfe, 0x65, 0x73, 0x70, 0x68, 0x6f, 0x6d, 0x65, 0x20, 0x6f, 0x74, 0x61, 0x20, 0x64, 0x65, 0x66, 0x6c, 0x61, 0x74, 0x65, 0x20, 0x65, 0x73, 0x70, 0x68, 0x6f, 0x6d, 0x65, 0x20, 0x6f, 0x74, 0x61, @@ -209,7 +204,6 @@ static const uint8_t STORED[] = { static constexpr size_t WINDOW = 4096; static constexpr size_t PLAIN_SIZE = 16000; -// Pseudo random bytes reproducible from Python for the vectors above static uint8_t lcg_next(uint32_t &x) { x = (x * 1103515245u + 12345u) & 0x7fffffffu; return (x >> 16) & 0xff; @@ -229,8 +223,7 @@ static std::vector build_plain() { return plain; } -// Mirrors the OTA session: the state is the base, input arrives through the -// read callback in chunks, the window doubles as the output buffer. +// Mirrors the OTA session: chunked input through the read callback, window as output struct Session : OtaInflateState { const uint8_t *in; size_t in_len; @@ -310,9 +303,7 @@ TEST(OtaInflate, TruncatedStoredBlockFails) { } TEST(OtaInflate, CorruptStreamsNeverEscapeTheWindow) { - // Every third byte of the stream flipped in turn, plus pseudo random garbage: - // the sanitizers check that the decoder never reads or writes out of bounds - // whatever it returns. + // Flipped bytes and garbage; the sanitizers check the decoder stays in bounds auto s = std::make_unique(); std::vector bad(DEFLATED, DEFLATED + sizeof(DEFLATED)); for (size_t i = 0; i < bad.size(); i += 3) { diff --git a/tests/integration/test_host_ota.py b/tests/integration/test_host_ota.py index d25f9e872d..97b0525fa5 100644 --- a/tests/integration/test_host_ota.py +++ b/tests/integration/test_host_ota.py @@ -185,7 +185,7 @@ async def test_host_ota_self_update( def on_log(line: str) -> None: if "OTA staged at" in line: staged.set() - # The host backend has no gzip support, so the upload negotiates deflate + # The host backend cannot store gzip, so the upload negotiates deflate if "Inflated " in line and " bytes from " in line: inflated.set() dev.on_log(line) diff --git a/tests/unit_tests/test_espota2.py b/tests/unit_tests/test_espota2.py index 0a0be90bc0..d3da77472b 100644 --- a/tests/unit_tests/test_espota2.py +++ b/tests/unit_tests/test_espota2.py @@ -601,10 +601,7 @@ def test_perform_ota_upload_error(mock_socket: Mock, mock_file: io.BytesIO) -> N def _no_auth_handshake(version: int, server_features: int | None = None) -> list[bytes]: - """Recv responses for a handshake without auth, up to the MD5 check. - - With server_features the device answers with the extended feature flags. - """ + """Recv responses for a handshake without auth, up to the MD5 check.""" if server_features is None: features = [bytes([espota2.RESPONSE_HEADER_OK])] else: @@ -1532,13 +1529,13 @@ _UPLOAD_TAIL = [ "server_features", [ espota2.SERVER_FEATURE_SUPPORTS_DEFLATE, - # A deflate offer is binding, so it wins should a device set both bits + # Binding offer: deflate wins over gzip espota2.SERVER_FEATURE_SUPPORTS_DEFLATE | espota2.SERVER_FEATURE_SUPPORTS_COMPRESSION, ], ) def test_perform_ota_with_deflate(mock_socket: Mock, server_features: int) -> None: - """A device that inflates on the fly gets a raw deflate stream, both sizes and the image MD5.""" + """The device gets a raw deflate stream, both sizes and the image MD5.""" original_content = b"firmware" * 100 mock_socket.recv.side_effect = ( _no_auth_handshake(espota2.OTA_VERSION_2_0, server_features) + _UPLOAD_TAIL @@ -1552,6 +1549,5 @@ def test_perform_ota_with_deflate(mock_socket: Mock, server_features: int) -> No assert sent[4] == len(original_content).to_bytes(espota2.SIZE_FIELD_BYTES, "big") payload = sent[6] assert len(payload) == sent_size < len(original_content) - # The device decodes through a window of 1 << DEFLATE_WINDOW_BITS bytes assert zlib.decompress(payload, -espota2.DEFLATE_WINDOW_BITS) == original_content assert sent[5] == hashlib.md5(original_content).hexdigest().encode()