From c5e320a977eacf22c8ce6ddcc05066ce9c5abade Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 12 Sep 2026 15:10:54 -0500 Subject: [PATCH] Read the clock select bit once per received byte and leave transmit on the compile time bit time --- .../uart/uart_component_esp8266.cpp | 49 ++++++++++++------- .../components/uart/uart_component_esp8266.h | 18 ++----- esphome/core/helpers.h | 6 ++- 3 files changed, 40 insertions(+), 33 deletions(-) diff --git a/esphome/components/uart/uart_component_esp8266.cpp b/esphome/components/uart/uart_component_esp8266.cpp index d856735e5f7..bdb82f8f64f 100644 --- a/esphome/components/uart/uart_component_esp8266.cpp +++ b/esphome/components/uart/uart_component_esp8266.cpp @@ -1,5 +1,6 @@ #ifdef USE_ESP8266 #include "uart_component_esp8266.h" +#include #include "esphome/core/application.h" #include "esphome/core/defines.h" #include "esphome/core/helpers.h" @@ -251,24 +252,36 @@ void ESP8266SoftwareSerial::setup(InternalGPIOPin *tx_pin, InternalGPIOPin *rx_p gpio_rx_pin_->attach_interrupt(ESP8266SoftwareSerial::gpio_intr, this, gpio::INTERRUPT_FALLING_EDGE); } } +// Cycles per bit for a byte received now. An 80 MHz build runs at 160 MHz while a CpuFrequencyBoost is open +// and a byte can arrive during one, so the live clock select bit doubles the compile time value. The whole byte +// is read inside the ISR, so the clock cannot change partway through. A 160 MHz build never changes clock. +__attribute__((always_inline)) static inline uint32_t rx_bit_time(uint32_t bit_time) { +#if F_CPU != 160000000L + // NOLINTNEXTLINE(clang-analyzer-core.FixedAddressDereference) -- CPU2X is MMIO at a fixed address + return bit_time << (CPU2X & 1); +#else + return bit_time; +#endif +} + void IRAM_ATTR ESP8266SoftwareSerial::gpio_intr(ESP8266SoftwareSerial *arg) { - const uint32_t bit_time = arg->bit_time_now_(); + const uint32_t bit_time = rx_bit_time(arg->bit_time_); uint32_t wait = bit_time + bit_time / 3 - 500; const uint32_t start = arch_get_cpu_cycle_count(); uint8_t rec = 0; // Manually unroll the loop for (int i = 0; i < arg->data_bits_; i++) - rec |= arg->read_bit_(&wait, start) << i; + rec |= arg->read_bit_(&wait, start, bit_time) << i; /* If parity is enabled, just read it and ignore it. */ /* TODO: Should we check parity? Or is it too slow for nothing added..*/ if (arg->parity_ == UART_CONFIG_PARITY_EVEN || arg->parity_ == UART_CONFIG_PARITY_ODD) - arg->read_bit_(&wait, start); + arg->read_bit_(&wait, start, bit_time); // Stop bit - arg->wait_(&wait, start); + arg->wait_(&wait, start, bit_time); if (arg->stop_bits_ == 2) - arg->wait_(&wait, start); + arg->wait_(&wait, start, bit_time); arg->rx_buffer_[arg->rx_in_pos_] = rec; arg->rx_in_pos_ = (arg->rx_in_pos_ + 1) % arg->rx_buffer_size_; @@ -297,37 +310,39 @@ void IRAM_ATTR HOT ESP8266SoftwareSerial::write_byte(uint8_t data) { } { + // Transmit runs from the main loop and never overlaps a CpuFrequencyBoost, so the compile time bit time holds InterruptLock lock; - uint32_t wait = this->bit_time_now_(); + const uint32_t bit_time = this->bit_time_; + uint32_t wait = bit_time; const uint32_t start = arch_get_cpu_cycle_count(); // Start bit - this->write_bit_(false, &wait, start); + this->write_bit_(false, &wait, start, bit_time); for (int i = 0; i < this->data_bits_; i++) { bool bit = data & (1 << i); - this->write_bit_(bit, &wait, start); + this->write_bit_(bit, &wait, start, bit_time); if (need_parity_bit) parity_bit ^= bit; } if (need_parity_bit) - this->write_bit_(parity_bit, &wait, start); + this->write_bit_(parity_bit, &wait, start, bit_time); // Stop bit - this->write_bit_(true, &wait, start); + this->write_bit_(true, &wait, start, bit_time); if (this->stop_bits_ == 2) - this->wait_(&wait, start); + this->wait_(&wait, start, bit_time); } } -void IRAM_ATTR ESP8266SoftwareSerial::wait_(uint32_t *wait, const uint32_t &start) { +void IRAM_ATTR ESP8266SoftwareSerial::wait_(uint32_t *wait, const uint32_t &start, uint32_t bit_time) { while (arch_get_cpu_cycle_count() - start < *wait) ; - *wait += this->bit_time_now_(); + *wait += bit_time; } -bool IRAM_ATTR ESP8266SoftwareSerial::read_bit_(uint32_t *wait, const uint32_t &start) { - this->wait_(wait, start); +bool IRAM_ATTR ESP8266SoftwareSerial::read_bit_(uint32_t *wait, const uint32_t &start, uint32_t bit_time) { + this->wait_(wait, start, bit_time); return this->rx_pin_.digital_read(); } -void IRAM_ATTR ESP8266SoftwareSerial::write_bit_(bool bit, uint32_t *wait, const uint32_t &start) { +void IRAM_ATTR ESP8266SoftwareSerial::write_bit_(bool bit, uint32_t *wait, const uint32_t &start, uint32_t bit_time) { this->tx_pin_.digital_write(bit); - this->wait_(wait, start); + this->wait_(wait, start, bit_time); } uint8_t ESP8266SoftwareSerial::read_byte() { if (this->rx_in_pos_ == this->rx_out_pos_) diff --git a/esphome/components/uart/uart_component_esp8266.h b/esphome/components/uart/uart_component_esp8266.h index 9a7c584b124..4064da40846 100644 --- a/esphome/components/uart/uart_component_esp8266.h +++ b/esphome/components/uart/uart_component_esp8266.h @@ -3,7 +3,6 @@ #ifdef USE_ESP8266 #include -#include #include #include "esphome/core/component.h" #include "esphome/core/hal.h" @@ -29,19 +28,10 @@ class ESP8266SoftwareSerial { protected: static void gpio_intr(ESP8266SoftwareSerial *arg); - void wait_(uint32_t *wait, const uint32_t &start); - bool read_bit_(uint32_t *wait, const uint32_t &start); - void write_bit_(bool bit, uint32_t *wait, const uint32_t &start); - - // Cycles per bit at the compile time clock. An 80 MHz build runs at 160 MHz while a CpuFrequencyBoost is - // open, so the live clock select bit doubles it there; a 160 MHz build never changes clock. - __attribute__((always_inline)) inline uint32_t bit_time_now_() const { -#if F_CPU != 160000000L - return this->bit_time_ << (CPU2X & 1); -#else - return this->bit_time_; -#endif - } + // bit_time is the cycles per bit at the clock the byte is running at + void wait_(uint32_t *wait, const uint32_t &start, uint32_t bit_time); + bool read_bit_(uint32_t *wait, const uint32_t &start, uint32_t bit_time); + void write_bit_(bool bit, uint32_t *wait, const uint32_t &start, uint32_t bit_time); uint32_t bit_time_{0}; uint8_t *rx_buffer_{nullptr}; diff --git a/esphome/core/helpers.h b/esphome/core/helpers.h index feab0fe24e8..44e28a70bad 100644 --- a/esphome/core/helpers.h +++ b/esphome/core/helpers.h @@ -2043,8 +2043,10 @@ extern "C" bool system_update_cpu_freq(uint8_t freq); /** Runs the CPU at 160 MHz while the object is alive. * * Only does anything on an ESP8266 built for 80 MHz; the Arduino core puts the clock back before every loop() - * pass, so a scope must open and close within one pass. On an ESP8266 already built for 160 MHz and on every - * other platform it is an inline no-op that compiles to nothing. Peripheral clocks are unchanged. + * pass, so a scope must open and close within one pass. Scopes must not nest, and the code inside one must not + * yield to the scheduler or the main loop, since the destructor always returns to 80 MHz. On an ESP8266 already + * built for 160 MHz and on every other platform it is an inline no-op that compiles to nothing. Peripheral + * clocks are unchanged. */ class CpuFrequencyBoost { public: