From 041d2bb16b552076eddaa58922eb02a79a9f8f5a Mon Sep 17 00:00:00 2001 From: "Dirk O. Kaar" Date: Wed, 31 Mar 2021 04:04:15 +0200 Subject: [PATCH 1/7] Conservative adjustment of optimistic_yield intervals. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 10000µs for just-checking "available"-like scenenarios. --- libraries/ESP8266WiFi/src/WiFiClient.cpp | 2 +- libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.cpp | 2 +- libraries/ESP8266WiFi/src/WiFiUdp.cpp | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/libraries/ESP8266WiFi/src/WiFiClient.cpp b/libraries/ESP8266WiFi/src/WiFiClient.cpp index fb10209ec0..de2c0c3b6f 100644 --- a/libraries/ESP8266WiFi/src/WiFiClient.cpp +++ b/libraries/ESP8266WiFi/src/WiFiClient.cpp @@ -251,7 +251,7 @@ int WiFiClient::available() int result = _client->getSize(); if (!result) { - optimistic_yield(100); + optimistic_yield(10000); } return result; } diff --git a/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.cpp b/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.cpp index 53b606f8c3..491dfca1e5 100644 --- a/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.cpp +++ b/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.cpp @@ -487,7 +487,7 @@ int WiFiClientSecureCtx::_run_until(unsigned target, bool blocking) { esp8266::polledTimeout::oneShotMs loopTimeout(_timeout); for (int no_work = 0; blocking || no_work < 2;) { - optimistic_yield(100); + optimistic_yield(1000); if (loopTimeout) { DEBUG_BSSL("_run_until: Timeout\n"); diff --git a/libraries/ESP8266WiFi/src/WiFiUdp.cpp b/libraries/ESP8266WiFi/src/WiFiUdp.cpp index a45659d844..35e9db3b24 100644 --- a/libraries/ESP8266WiFi/src/WiFiUdp.cpp +++ b/libraries/ESP8266WiFi/src/WiFiUdp.cpp @@ -117,7 +117,7 @@ int WiFiUDP::available() { if (!result) { // yielding here will not make more data "available", // but it will prevent the system from going into WDT reset - optimistic_yield(1000); + optimistic_yield(10000); } return result; @@ -194,7 +194,7 @@ int WiFiUDP::parsePacket() return 0; if (!_ctx->next()) { - optimistic_yield(100); + optimistic_yield(10000); return 0; } From 671a9679de1b62a241148c483f1a6d24f0f0968b Mon Sep 17 00:00:00 2001 From: "Dirk O. Kaar" Date: Wed, 31 Mar 2021 11:38:35 +0200 Subject: [PATCH 2/7] Review provided rationale for original interval value. --- libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.cpp b/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.cpp index 491dfca1e5..53b606f8c3 100644 --- a/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.cpp +++ b/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.cpp @@ -487,7 +487,7 @@ int WiFiClientSecureCtx::_run_until(unsigned target, bool blocking) { esp8266::polledTimeout::oneShotMs loopTimeout(_timeout); for (int no_work = 0; blocking || no_work < 2;) { - optimistic_yield(1000); + optimistic_yield(100); if (loopTimeout) { DEBUG_BSSL("_run_until: Timeout\n"); From 3cb53ba04fa17c33bec0d5b009d46bc66e7658fe Mon Sep 17 00:00:00 2001 From: "Dirk O. Kaar" Date: Thu, 8 Apr 2021 17:45:48 +0200 Subject: [PATCH 3/7] More frequent optimistic_yield looks safe to me here. --- cores/esp8266/uart.cpp | 6 ++++-- libraries/ESP8266WiFi/src/WiFiUdp.cpp | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/cores/esp8266/uart.cpp b/cores/esp8266/uart.cpp index 802072d438..0534d0e204 100644 --- a/cores/esp8266/uart.cpp +++ b/cores/esp8266/uart.cpp @@ -510,7 +510,10 @@ uart_tx_fifo_full(const int uart_nr) static void uart_do_write_char(const int uart_nr, char c) { - while(uart_tx_fifo_full(uart_nr)); + while(uart_tx_fifo_full(uart_nr)) + { + optimistic_yield(1000UL); + } USF(uart_nr) = c; } @@ -544,7 +547,6 @@ uart_write(uart_t* uart, const char* buf, size_t size) const int uart_nr = uart->uart_nr; while (size--) { uart_do_write_char(uart_nr, pgm_read_byte(buf++)); - optimistic_yield(10000UL); } return ret; diff --git a/libraries/ESP8266WiFi/src/WiFiUdp.cpp b/libraries/ESP8266WiFi/src/WiFiUdp.cpp index 35e9db3b24..0770d94deb 100644 --- a/libraries/ESP8266WiFi/src/WiFiUdp.cpp +++ b/libraries/ESP8266WiFi/src/WiFiUdp.cpp @@ -194,7 +194,7 @@ int WiFiUDP::parsePacket() return 0; if (!_ctx->next()) { - optimistic_yield(10000); + optimistic_yield(1000); return 0; } From f9646008c6cf047123c36c166d9304300537479f Mon Sep 17 00:00:00 2001 From: "Dirk O. Kaar" Date: Thu, 8 Apr 2021 17:47:16 +0200 Subject: [PATCH 4/7] Optimistic_yield interval changes from PR #7832, these need intense review. --- cores/esp8266/core_esp8266_i2s.cpp | 6 +++--- cores/esp8266/core_esp8266_wiring_pulse.cpp | 2 +- cores/esp8266/flash_hal.cpp | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/cores/esp8266/core_esp8266_i2s.cpp b/cores/esp8266/core_esp8266_i2s.cpp index 05a70f18d6..c1586189b5 100644 --- a/cores/esp8266/core_esp8266_i2s.cpp +++ b/cores/esp8266/core_esp8266_i2s.cpp @@ -313,7 +313,7 @@ static bool _i2s_write_sample(uint32_t sample, bool nb) { if (tx->slc_queue_len > 0) { break; } else { - optimistic_yield(10000); + optimistic_yield(1000); } } } @@ -362,7 +362,7 @@ static uint16_t _i2s_write_buffer(const int16_t *frames, uint16_t frame_count, b if (tx->slc_queue_len > 0) { break; } else { - optimistic_yield(10000); + optimistic_yield(1000); } } } @@ -422,7 +422,7 @@ bool i2s_read_sample(int16_t *left, int16_t *right, bool blocking) { if (rx->slc_queue_len > 0){ break; } else { - optimistic_yield(10000); + optimistic_yield(1000); } } } diff --git a/cores/esp8266/core_esp8266_wiring_pulse.cpp b/cores/esp8266/core_esp8266_wiring_pulse.cpp index 8e124ac312..ac7e466116 100644 --- a/cores/esp8266/core_esp8266_wiring_pulse.cpp +++ b/cores/esp8266/core_esp8266_wiring_pulse.cpp @@ -31,7 +31,7 @@ extern uint32_t xthal_get_ccount(); if (xthal_get_ccount() - start_cycle_count > timeout_cycles) { \ return 0; \ } \ - optimistic_yield(5000); \ + optimistic_yield(1000); \ } // max timeout is 27 seconds at 160MHz clock and 54 seconds at 80MHz clock diff --git a/cores/esp8266/flash_hal.cpp b/cores/esp8266/flash_hal.cpp index 87b34830fa..5240239d8a 100644 --- a/cores/esp8266/flash_hal.cpp +++ b/cores/esp8266/flash_hal.cpp @@ -30,7 +30,7 @@ extern "C" { } int32_t flash_hal_read(uint32_t addr, uint32_t size, uint8_t *dst) { - optimistic_yield(10000); + optimistic_yield(1000); // We use flashRead overload that handles proper alignment if (ESP.flashRead(addr, dst, size)) { @@ -41,7 +41,7 @@ int32_t flash_hal_read(uint32_t addr, uint32_t size, uint8_t *dst) { } int32_t flash_hal_write(uint32_t addr, uint32_t size, const uint8_t *src) { - optimistic_yield(10000); + optimistic_yield(1000); // We use flashWrite overload that handles proper alignment if (ESP.flashWrite(addr, src, size)) { @@ -60,7 +60,7 @@ int32_t flash_hal_erase(uint32_t addr, uint32_t size) { const uint32_t sector = addr / SPI_FLASH_SEC_SIZE; const uint32_t sectorCount = size / SPI_FLASH_SEC_SIZE; for (uint32_t i = 0; i < sectorCount; ++i) { - optimistic_yield(10000); + optimistic_yield(1000); if (!ESP.flashEraseSector(sector + i)) { DEBUGV("_spif_erase addr=%x size=%d i=%d\r\n", addr, size, i); return FLASH_HAL_ERASE_ERROR; From 07566e46a567d4b3f396b1048707737d95158402 Mon Sep 17 00:00:00 2001 From: "Dirk O. Kaar" Date: Fri, 9 Apr 2021 22:28:44 +0200 Subject: [PATCH 5/7] =?UTF-8?q?Wire=20has=20an=20available()=20function=20?= =?UTF-8?q?-=2010000=C2=B5s=20optimistic=5Fyield=20interval=20is=20the=20r?= =?UTF-8?q?ule=20of=20thumb.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- libraries/Wire/Wire.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/Wire/Wire.cpp b/libraries/Wire/Wire.cpp index b2d9a6affc..333b65ccb1 100644 --- a/libraries/Wire/Wire.cpp +++ b/libraries/Wire/Wire.cpp @@ -227,7 +227,7 @@ int TwoWire::available(void) { // yielding here will not make more data "available", // but it will prevent the system from going into WDT reset - optimistic_yield(1000); + optimistic_yield(10000UL); } return result; From d3bf7f974e967bd43d79f07984dd17de6e4aa71c Mon Sep 17 00:00:00 2001 From: "Dirk O. Kaar" Date: Sun, 11 Apr 2021 23:37:36 +0200 Subject: [PATCH 6/7] Per review by @earlephilhower --- cores/esp8266/core_esp8266_i2s.cpp | 6 +++--- cores/esp8266/flash_hal.cpp | 6 +++--- cores/esp8266/uart.cpp | 2 +- libraries/ESP8266WiFi/src/WiFiClient.cpp | 2 +- libraries/ESP8266WiFi/src/WiFiUdp.cpp | 4 ++-- libraries/Wire/Wire.cpp | 2 +- 6 files changed, 11 insertions(+), 11 deletions(-) diff --git a/cores/esp8266/core_esp8266_i2s.cpp b/cores/esp8266/core_esp8266_i2s.cpp index c1586189b5..05a70f18d6 100644 --- a/cores/esp8266/core_esp8266_i2s.cpp +++ b/cores/esp8266/core_esp8266_i2s.cpp @@ -313,7 +313,7 @@ static bool _i2s_write_sample(uint32_t sample, bool nb) { if (tx->slc_queue_len > 0) { break; } else { - optimistic_yield(1000); + optimistic_yield(10000); } } } @@ -362,7 +362,7 @@ static uint16_t _i2s_write_buffer(const int16_t *frames, uint16_t frame_count, b if (tx->slc_queue_len > 0) { break; } else { - optimistic_yield(1000); + optimistic_yield(10000); } } } @@ -422,7 +422,7 @@ bool i2s_read_sample(int16_t *left, int16_t *right, bool blocking) { if (rx->slc_queue_len > 0){ break; } else { - optimistic_yield(1000); + optimistic_yield(10000); } } } diff --git a/cores/esp8266/flash_hal.cpp b/cores/esp8266/flash_hal.cpp index 5240239d8a..87b34830fa 100644 --- a/cores/esp8266/flash_hal.cpp +++ b/cores/esp8266/flash_hal.cpp @@ -30,7 +30,7 @@ extern "C" { } int32_t flash_hal_read(uint32_t addr, uint32_t size, uint8_t *dst) { - optimistic_yield(1000); + optimistic_yield(10000); // We use flashRead overload that handles proper alignment if (ESP.flashRead(addr, dst, size)) { @@ -41,7 +41,7 @@ int32_t flash_hal_read(uint32_t addr, uint32_t size, uint8_t *dst) { } int32_t flash_hal_write(uint32_t addr, uint32_t size, const uint8_t *src) { - optimistic_yield(1000); + optimistic_yield(10000); // We use flashWrite overload that handles proper alignment if (ESP.flashWrite(addr, src, size)) { @@ -60,7 +60,7 @@ int32_t flash_hal_erase(uint32_t addr, uint32_t size) { const uint32_t sector = addr / SPI_FLASH_SEC_SIZE; const uint32_t sectorCount = size / SPI_FLASH_SEC_SIZE; for (uint32_t i = 0; i < sectorCount; ++i) { - optimistic_yield(1000); + optimistic_yield(10000); if (!ESP.flashEraseSector(sector + i)) { DEBUGV("_spif_erase addr=%x size=%d i=%d\r\n", addr, size, i); return FLASH_HAL_ERASE_ERROR; diff --git a/cores/esp8266/uart.cpp b/cores/esp8266/uart.cpp index 0534d0e204..de57ff880a 100644 --- a/cores/esp8266/uart.cpp +++ b/cores/esp8266/uart.cpp @@ -512,7 +512,7 @@ uart_do_write_char(const int uart_nr, char c) { while(uart_tx_fifo_full(uart_nr)) { - optimistic_yield(1000UL); + optimistic_yield(10000UL); } USF(uart_nr) = c; diff --git a/libraries/ESP8266WiFi/src/WiFiClient.cpp b/libraries/ESP8266WiFi/src/WiFiClient.cpp index de2c0c3b6f..fb10209ec0 100644 --- a/libraries/ESP8266WiFi/src/WiFiClient.cpp +++ b/libraries/ESP8266WiFi/src/WiFiClient.cpp @@ -251,7 +251,7 @@ int WiFiClient::available() int result = _client->getSize(); if (!result) { - optimistic_yield(10000); + optimistic_yield(100); } return result; } diff --git a/libraries/ESP8266WiFi/src/WiFiUdp.cpp b/libraries/ESP8266WiFi/src/WiFiUdp.cpp index 0770d94deb..8a7e0cff0b 100644 --- a/libraries/ESP8266WiFi/src/WiFiUdp.cpp +++ b/libraries/ESP8266WiFi/src/WiFiUdp.cpp @@ -117,7 +117,7 @@ int WiFiUDP::available() { if (!result) { // yielding here will not make more data "available", // but it will prevent the system from going into WDT reset - optimistic_yield(10000); + optimistic_yield(100UL); } return result; @@ -194,7 +194,7 @@ int WiFiUDP::parsePacket() return 0; if (!_ctx->next()) { - optimistic_yield(1000); + optimistic_yield(100UL); return 0; } diff --git a/libraries/Wire/Wire.cpp b/libraries/Wire/Wire.cpp index 333b65ccb1..a68b8962ba 100644 --- a/libraries/Wire/Wire.cpp +++ b/libraries/Wire/Wire.cpp @@ -227,7 +227,7 @@ int TwoWire::available(void) { // yielding here will not make more data "available", // but it will prevent the system from going into WDT reset - optimistic_yield(10000UL); + optimistic_yield(100UL); } return result; From 342c94edd6981cc4e6764501462bc391c605a8c9 Mon Sep 17 00:00:00 2001 From: "Dirk O. Kaar" Date: Sun, 16 May 2021 15:11:26 +0200 Subject: [PATCH 7/7] Refactor macro to an inline function --- cores/esp8266/core_esp8266_wiring_pulse.cpp | 31 +++++++++++++++------ 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/cores/esp8266/core_esp8266_wiring_pulse.cpp b/cores/esp8266/core_esp8266_wiring_pulse.cpp index ac7e466116..b20bdb4c45 100644 --- a/cores/esp8266/core_esp8266_wiring_pulse.cpp +++ b/cores/esp8266/core_esp8266_wiring_pulse.cpp @@ -26,13 +26,20 @@ extern "C" { extern uint32_t xthal_get_ccount(); -#define WAIT_FOR_PIN_STATE(state) \ - while (digitalRead(pin) != (state)) { \ - if (xthal_get_ccount() - start_cycle_count > timeout_cycles) { \ - return 0; \ - } \ - optimistic_yield(1000); \ +namespace { + inline __attribute__((always_inline)) bool waitForPinState( + const uint8_t pin, const uint8_t state, + const uint32_t timeout_cycles, const uint32_t start_cycle_count) + { + while (digitalRead(pin) != state) { + if (xthal_get_ccount() - start_cycle_count > timeout_cycles) { + return false; + } + optimistic_yield(1000); + } + return true; } +} // max timeout is 27 seconds at 160MHz clock and 54 seconds at 80MHz clock unsigned long pulseIn(uint8_t pin, uint8_t state, unsigned long timeout) @@ -43,10 +50,16 @@ unsigned long pulseIn(uint8_t pin, uint8_t state, unsigned long timeout) } const uint32_t timeout_cycles = microsecondsToClockCycles(timeout); const uint32_t start_cycle_count = xthal_get_ccount(); - WAIT_FOR_PIN_STATE(!state); - WAIT_FOR_PIN_STATE(state); + if (!waitForPinState(pin, !state, timeout_cycles, start_cycle_count)) { + return 0; + } + if (!waitForPinState(pin, state, timeout_cycles, start_cycle_count)) { + return 0; + } const uint32_t pulse_start_cycle_count = xthal_get_ccount(); - WAIT_FOR_PIN_STATE(!state); + if (!waitForPinState(pin, !state, timeout_cycles, start_cycle_count)) { + return 0; + } return clockCyclesToMicroseconds(xthal_get_ccount() - pulse_start_cycle_count); }