diff --git a/esphome/components/uart/software_serial_rx_decoder.h b/esphome/components/uart/software_serial_rx_decoder.h index b4be2c7b59..24ec9c8eeb 100644 --- a/esphome/components/uart/software_serial_rx_decoder.h +++ b/esphome/components/uart/software_serial_rx_decoder.h @@ -7,29 +7,20 @@ namespace esphome::uart { -/// Bit timing decoder for a software serial RX pin. -/// -/// on_edge() is fed from the pin change interrupt with the cycle count of the edge -/// and the level the line changed to; it counts how many bit times the previous -/// level lasted and runs them through a start/data/parity/stop state machine, -/// so the interrupt never waits for the line. Decoded bytes land in a ring buffer. -/// A byte whose trailing bits are idle high has no closing edge; the main loop -/// completes it with finalize() once enough time has passed. -/// -/// Kept free of platform headers so it can be unit tested on the host. The hot -/// methods are force inlined so the platform ISR that calls them stays in IRAM. +/// Bit timing decoder for a software serial RX pin: on_edge() decodes from the +/// time between edges so the ISR never waits for the line; a byte with an idle +/// high tail has no closing edge and is completed by finalize() from the loop. +/// Platform free for host tests; hot methods force inlined to stay in IRAM. class SoftwareSerialRxDecoder { public: static constexpr uint8_t RX_IDLE = 0xFF; - /// Configure the framing and buffer. Drops buffered bytes and any partial frame and - /// assumes an idle high line; call reset() afterwards with the real line level. + /// Configure framing and buffer; drops all state. Follow with reset(). void setup(uint32_t bit_cycles, uint8_t data_bits, bool parity, uint8_t stop_bits, uint8_t *buffer, size_t buffer_size) { this->bit_cycles_ = bit_cycles; this->data_bits_ = data_bits; this->stop_bit_ = data_bits + (parity ? 1 : 0); - // Runs longer than a whole frame plus one bit are idle; cap them there. this->max_run_cycles_ = bit_cycles * (this->stop_bit_ + stop_bits + 2); this->buffer_ = buffer; this->buffer_size_ = buffer_size; @@ -46,18 +37,15 @@ class SoftwareSerialRxDecoder { this->last_edge_ = now; } - /// ISR: the line changed to `level` at cycle `now`. - /// Returns true when the main loop should be woken: a byte was pushed, or a frame - /// is open and the line is high, in which case finalize() may be needed. That is - /// deliberately conservative; a data 1 and the idle tail are indistinguishable at - /// the edge, and repeat wakes are cheap because the wake flag is already set. + /// ISR: the line changed to `level` at cycle `now`. Returns true to wake the + /// loop: a byte was pushed, or a frame is open with the line high (finalize() + /// may be needed; a data 1 and the idle tail look the same at the edge). bool ESPHOME_ALWAYS_INLINE on_edge(uint32_t now, bool level) { const bool last_level = this->last_level_; - // Two edges collapsed into one interrupt: skip it so the run is still - // measured from the last real edge and the frame stays aligned. + // Collapsed edges: skip to keep the frame aligned to the last real edge. if (level == last_level) return false; - // Bits since the last edge, rounded to nearest; no hardware divider on the LX106, so count. + // Bits since the last edge, rounded; LX106 has no divider, so count. uint32_t delta = now - this->last_edge_; if (delta > this->max_run_cycles_) delta = this->max_run_cycles_; @@ -73,10 +61,10 @@ class SoftwareSerialRxDecoder { return pushed || (level && this->bit_ != RX_IDLE); } - /// True while a frame is open and the line sits idle high, so a byte may be waiting on finalize(). + /// A frame is open with the line idle high: a byte may be waiting on finalize(). bool pending() const { return this->bit_ != RX_IDLE && this->last_level_; } - /// Cheap unlocked check whether the pending byte's tail has elapsed by `now`. + /// Unlocked check whether the pending byte's tail has elapsed. bool finalize_due(uint32_t now) const { const uint8_t bit = this->bit_; if (bit == RX_IDLE) @@ -92,7 +80,7 @@ class SoftwareSerialRxDecoder { this->consume_run_(this->stop_bit_ + 1 - bit, true); } - /// Store a decoded byte, dropping it when the buffer is full. Also used by the start bit sampler. + /// Store a byte, dropping it when full. Also used by the start bit sampler. bool ESPHOME_ALWAYS_INLINE push_byte(uint8_t data) { size_t in = this->in_pos_; size_t next = in + 1; @@ -127,19 +115,18 @@ class SoftwareSerialRxDecoder { } protected: - /// Cycles the line must stay high after the last edge to hold every bit up to the first stop bit. + /// Cycles of high line needed to finish the frame through the first stop bit. uint32_t tail_cycles_(uint8_t bit) const { return (this->stop_bit_ + 1 - bit) * this->bit_cycles_ + this->bit_cycles_ / 2; } - /// Feed `bits` consecutive bits at `level` into the frame. Returns true when a byte was pushed. + /// Feed `bits` bits at `level` into the frame; true when a byte was pushed. bool ESPHOME_ALWAYS_INLINE consume_run_(uint32_t bits, bool level) { uint8_t bit = this->bit_; uint8_t cur = this->cur_byte_; bool pushed = false; while (bits > 0) { if (bit == RX_IDLE) { - // Idle line, or what is left of a run after a framing error. if (level) break; // Start bit @@ -159,7 +146,7 @@ class SoftwareSerialRxDecoder { bit++; bits--; } else { - // Stop bit; a low level here is a framing error, drop the byte. + // Stop bit; low is a framing error, drop the byte. if (level) pushed = this->push_byte(cur); bit = RX_IDLE; @@ -179,7 +166,7 @@ class SoftwareSerialRxDecoder { size_t buffer_size_{0}; volatile size_t in_pos_{0}; volatile size_t out_pos_{0}; - /// Index of the next frame bit after the start bit (data, then parity, then stop at stop_bit_) or RX_IDLE. + /// Next frame bit (data, parity, then stop at stop_bit_) or RX_IDLE. volatile uint8_t bit_{RX_IDLE}; volatile uint8_t cur_byte_{0}; volatile bool last_level_{true}; diff --git a/esphome/components/uart/uart_component_esp8266.cpp b/esphome/components/uart/uart_component_esp8266.cpp index 514510a6cf..6226f2bba4 100644 --- a/esphome/components/uart/uart_component_esp8266.cpp +++ b/esphome/components/uart/uart_component_esp8266.cpp @@ -15,8 +15,7 @@ namespace esphome::uart { static const char *const TAG = "uart"; -// Edge decoder up to this baud rate; above it the start bit sampler, whose -// whole-byte block in the ISR is short there and whose timing still holds up. +// Edge decoder up to this baud rate, start bit sampler above it. static constexpr uint32_t SW_SERIAL_EDGE_MODE_MAX_BAUD = 38400; bool ESP8266UartComponent::serial0_in_use = false; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) @@ -239,7 +238,7 @@ UARTFlushResult ESP8266UartComponent::flush() { void ESP8266SoftwareSerial::setup(InternalGPIOPin *tx_pin, InternalGPIOPin *rx_pin, uint32_t baud_rate, uint8_t stop_bits, uint32_t data_bits, UARTParityOptions parity, size_t rx_buffer_size) { - // load_settings() re-enters here: stop the RX interrupt before touching anything it reads. + // load_settings() re-enters here: detach the ISR before touching its state. if (this->gpio_rx_pin_ != nullptr) this->gpio_rx_pin_->detach_interrupt(); this->bit_time_ = F_CPU / baud_rate; @@ -315,8 +314,7 @@ void IRAM_ATTR ESP8266SoftwareSerial::gpio_intr_edge(ESP8266SoftwareSerial *arg) void ESP8266SoftwareSerial::rx_finalize_pending_() { if (!this->rx_.finalize_due(arch_get_cpu_cycle_count())) { #ifdef USE_UART_WAKE_LOOP_ON_RX - // Not old enough yet: once the caller has drained what is there, run the loop - // again right away instead of after a full loop_interval_. + // Byte not old enough yet: re-run the loop once the buffer is drained. if (this->rx_.available() == 0) wake_loop_threadsafe(); #endif diff --git a/tests/components/uart/test_software_serial_rx_decoder.cpp b/tests/components/uart/test_software_serial_rx_decoder.cpp index 89e65f759f..caacdfe343 100644 --- a/tests/components/uart/test_software_serial_rx_decoder.cpp +++ b/tests/components/uart/test_software_serial_rx_decoder.cpp @@ -14,9 +14,8 @@ namespace { // 80 MHz ESP8266 clock constexpr uint32_t CPU_HZ = 80000000; -// Drives a SoftwareSerialRxDecoder with a simulated line: frames are turned into -// edges at their ideal cycle counts (plus optional jitter), and the main loop is -// emulated by poll(), which finalizes a pending byte and drains the buffer. +// Simulated line: frames become edges at ideal cycle counts plus jitter; +// poll() emulates the main loop (finalize pending byte, drain buffer). class LineSim { public: LineSim(uint32_t baud, uint8_t data_bits, bool parity, bool odd, uint8_t stop_bits, size_t buffer_size = 64) @@ -45,8 +44,7 @@ class LineSim { this->received_.push_back(this->dec_.read_byte()); } - // Emit one frame starting at cycle `start`; `jitter` is the per edge offset in cycles - // (positive or negative) supplied by the caller. Returns the cycle at which the frame ends. + // Emit one frame at `start` with per edge `jitter` in cycles; returns the end cycle. uint32_t send( uint8_t value, uint32_t start, const std::function &jitter = [] { return 0; }) { std::vector bits; @@ -123,8 +121,7 @@ TEST(SoftwareSerialRxDecoder, DecodesBackToBackFramesAtCommonBaudRates) { } TEST(SoftwareSerialRxDecoder, ToleratesEdgeJitterUpToAQuarterBit) { - // Consecutive edges can each be off by up to a quarter bit in either direction, - // so the measured run length is within half a bit of the true one. + // A quarter bit per edge keeps each run within the half bit rounding budget. run_stream({9600, 8, false, false, 1}, 0.24, 0, 2000, 1); run_stream({9600, 8, false, false, 1}, 0.24, 7, 2000, 2); run_stream({38400, 8, false, false, 1}, 0.24, 4, 2000, 3); @@ -141,11 +138,10 @@ TEST(SoftwareSerialRxDecoder, AllOnesByteCompletesOnlyByFinalize) { LineSim sim(9600, 8, false, false, 1); const uint32_t bit = sim.bit_cycles(); uint32_t t = 5000; - // Start bit, then the line goes high and stays there: 0xFF has no closing edge. + // 0xFF: start bit, then the line stays high with no closing edge. sim.edge(t, false); sim.edge(t + bit, true); EXPECT_TRUE(sim.decoder().pending()); - // Eight data bits plus the stop bit must elapse after the rising edge. sim.poll(t + bit + 8 * bit); EXPECT_TRUE(sim.received().empty()); EXPECT_FALSE(sim.decoder().finalize_due(t + bit + 8 * bit)); @@ -191,9 +187,7 @@ TEST(SoftwareSerialRxDecoder, BreakConditionIsDroppedAndResyncs) { TEST(SoftwareSerialRxDecoder, CollapsedEdgeIsIgnoredAndStreamRealignsAtIdle) { LineSim sim(9600, 8, false, false, 1); const uint32_t bit = sim.bit_cycles(); - // 0x31 = 0b00110001: start, 1, 0, 0, 0, 1, 1, 0, 0, stop. Lose the rising edge - // of data bit 4 (ISR delayed past two edges), so the next edge arrives at the - // same level as the last one the decoder saw. + // 0x31: lose the rising edge of bit 4, so the next edge repeats the last level. uint32_t t = 5000; sim.edge(t, false); // start sim.edge(t + 1 * bit, true); // bit0 = 1 @@ -220,8 +214,7 @@ TEST(SoftwareSerialRxDecoder, DropsBytesWhenBufferIsFullAndKeepsOldest) { } TEST(SoftwareSerialRxDecoder, SetupAgainDropsStaleStateAndUsesNewBufferAndFraming) { - // Mirrors load_settings(): bytes buffered and a frame left open under 8N1 in a - // 64 byte buffer, then setup() again with 5E2 in a 4 byte buffer. + // Mirrors load_settings(): buffered 8N1 bytes and an open frame, then setup() as 5E2. LineSim sim(9600, 8, false, false, 1); uint32_t t = 5000; for (int n = 0; n < 10; n++) @@ -237,8 +230,7 @@ TEST(SoftwareSerialRxDecoder, SetupAgainDropsStaleStateAndUsesNewBufferAndFramin EXPECT_FALSE(dec.pending()); EXPECT_EQ(dec.read_byte(), 0); - // 5E2 frames fed straight into the reconfigured decoder: capacity is 3, the - // rest are dropped and nothing is written past the end of the new buffer. + // Capacity of the 4 byte buffer is 3; the spare slot must stay untouched. auto send_5e2 = [&](uint8_t value, uint32_t start) { bool line = true; uint32_t at = start;