Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Delay(0) is an obfuscation of yield() or esp_schedule(); esp_yield() #7146

Closed
wants to merge 2 commits into from
Closed

Delay(0) is an obfuscation of yield() or esp_schedule(); esp_yield() #7146

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ba73d82
Delay(0) is just a convoluted way of saying yield()
dok-net Mar 10, 2020
cfde64b
yield() without panic() can do the right thing in CONT and SYS alike.
dok-net Mar 10, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cores/esp8266/PolledTimeout.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ struct DoNothing

struct YieldOrSkip
{
static void execute() {delay(0);}
static void execute() {yield();}
};

template <unsigned long delayMs>
Expand Down
5 changes: 1 addition & 4 deletions cores/esp8266/core_esp8266_main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,13 +113,10 @@ extern "C" IRAM_ATTR void esp_schedule() {
}

extern "C" void __yield() {
esp_schedule();
if (can_yield()) {
esp_schedule();
esp_yield_within_cont();
}
else {
panic();
}
}

extern "C" void yield(void) __attribute__ ((weak, alias("__yield")));
Expand Down
2 changes: 1 addition & 1 deletion cores/esp8266/core_esp8266_waveform.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ int startWaveform(uint8_t pin, uint32_t timeHighUS, uint32_t timeLowUS, uint32_t
}
}
while (waveformToEnable) {
delay(0); // Wait for waveform to update
yield(); // Wait for waveform to update
}
}

Expand Down
4 changes: 2 additions & 2 deletions cores/esp8266/uart.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,7 @@ uart_wait_tx_empty(uart_t* uart)
return;

while(uart_tx_fifo_available(uart->uart_nr) > 0)
delay(0);
yield();

}

Expand Down Expand Up @@ -892,7 +892,7 @@ inline void
uart_write_char_delay(const int uart_nr, char c)
{
while(uart_tx_fifo_full(uart_nr))
delay(0);
yield();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at uart_write_char_delay, it is used by the uart{0,1}_write_char and those specific write funcs are installed as ets_install_putc1(...) when debug mode is set. Does this mean that SYS calls this part, not CONT?

Copy link
Collaborator

@d-a-v d-a-v Mar 10, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is right, esp_schedule() + esp_yield() should replace delay(0) here.
So uart{n}_write_char() should be in IRAM right ?
And uart_write_char_delay() is inline, maybe the attribute __attribute__((always_inline)) should be added ?

Maybe adding a function yield_from_isr() would be useful to help through reading the code, for when we need to migrate to another SDK ?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

re IRAM and inline. It seems to be inlined anyway, no symbol is created for any functions down the chain besides delay(). Is it a concern for sdk code in any way?
Other q i think is answered below.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mcspr
inline is an indication (like register which is deprecated in latest gcc version I believe) and compiler is free to not honor this request, which is fair.
But in that case, the function needs to be in iram because it can be called from an ISR.
Furthermore we can't declare a function to be at the same time inline and in iram.
So the proposal above is to force the compiler to always use this function inline.


USF(uart_nr) = c;

Expand Down
8 changes: 4 additions & 4 deletions libraries/ESP8266HTTPClient/src/ESP8266HTTPClient.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -854,7 +854,7 @@ int HTTPClient::sendRequest(const char * type, Stream * stream, size_t size)
len -= readBytes;
}

delay(0);
yield();
} else {
delay(1);
}
Expand Down Expand Up @@ -1000,7 +1000,7 @@ int HTTPClient::writeToStream(Stream * stream)
return returnError(HTTPC_ERROR_READ_TIMEOUT);
}

delay(0);
yield();
}
} else {
return returnError(HTTPC_ERROR_ENCODING);
Expand Down Expand Up @@ -1387,7 +1387,7 @@ int HTTPClient::handleHeaderResponse()
if((millis() - lastDataTime) > _tcpTimeout) {
return HTTPC_ERROR_READ_TIMEOUT;
}
delay(0);
yield();
}
}

Expand Down Expand Up @@ -1483,7 +1483,7 @@ int HTTPClient::writeToStreamDataBlock(Stream * stream, int size)
len -= bytesRead;
}

delay(0);
yield();
}

free(buff);
Expand Down
2 changes: 1 addition & 1 deletion libraries/ESP8266HTTPUpdateServer/src/ESP8266HTTPUpdateServer-impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ void ESP8266HTTPUpdateServerTemplate<ServerType>::setup(ESP8266WebServerTemplate
Update.end();
if (_serial_output) Serial.println("Update was aborted");
}
delay(0);
yield();
});
}

Expand Down
4 changes: 2 additions & 2 deletions libraries/ESP8266WiFi/src/ESP8266WiFiGeneric.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -523,9 +523,9 @@ bool ESP8266WiFiGenericClass::forceSleepBegin(uint32 sleepUs) {
}

wifi_fpm_set_sleep_type(MODEM_SLEEP_T);
delay(0);
yield();
wifi_fpm_open();
delay(0);
yield();
auto ret = wifi_fpm_do_sleep(sleepUs);
if (ret != 0)
{
Expand Down
8 changes: 4 additions & 4 deletions libraries/ESP8266WiFi/src/ESP8266WiFiMulti.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ wl_status_t ESP8266WiFiMulti::run(void) {
DEBUG_WIFI_MULTI("[WIFI] no networks found\n");
WiFi.scanDelete();
DEBUG_WIFI_MULTI("\n\n");
delay(0);
yield();
WiFi.disconnect();
DEBUG_WIFI_MULTI("[WIFI] start scan\n");
// scan wifi async mode
Expand All @@ -81,7 +81,7 @@ wl_status_t ESP8266WiFiMulti::run(void) {
int32_t bestChannel;

DEBUG_WIFI_MULTI("[WIFI] scan done\n");
delay(0);
yield();

DEBUG_WIFI_MULTI("[WIFI] %d networks found\n", scanResult);
for(int8_t i = 0; i < scanResult; ++i) {
Expand Down Expand Up @@ -118,14 +118,14 @@ wl_status_t ESP8266WiFiMulti::run(void) {
}

DEBUG_WIFI_MULTI(" %d: [%d][%02X:%02X:%02X:%02X:%02X:%02X] %s (%d) %c\n", i, chan_scan, BSSID_scan[0], BSSID_scan[1], BSSID_scan[2], BSSID_scan[3], BSSID_scan[4], BSSID_scan[5], ssid_scan.c_str(), rssi_scan, (sec_scan == ENC_TYPE_NONE) ? ' ' : '*');
delay(0);
yield();
}

// clean up ram
WiFi.scanDelete();

DEBUG_WIFI_MULTI("\n\n");
delay(0);
yield();

if(bestNetwork.ssid) {
DEBUG_WIFI_MULTI("[WIFI] Connecting BSSID: %02X:%02X:%02X:%02X:%02X:%02X SSID: %s Channel: %d (%d)\n", bestBSSID[0], bestBSSID[1], bestBSSID[2], bestBSSID[3], bestBSSID[4], bestBSSID[5], bestNetwork.ssid, bestChannel, bestNetworkDb);
Expand Down
2 changes: 1 addition & 1 deletion libraries/ESP8266WiFi/src/ESP8266WiFiScan.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ int8_t ESP8266WiFiScanClass::scanNetworks(bool async, bool show_hidden, uint8 ch
ESP8266WiFiScanClass::_scanStarted = true;

if(ESP8266WiFiScanClass::_scanAsync) {
delay(0); // time for the OS to trigger the scan
yield(); // time for the OS to trigger the scan
return WIFI_SCAN_RUNNING;
}

Expand Down
2 changes: 1 addition & 1 deletion libraries/ESP8266WiFi/src/include/ClientContext.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,7 @@ class ClientContext
last_sent = millis();
}

delay(0); // from sys or os context
yield(); // from sys or os context

if ((state() != ESTABLISHED) || (sndbuf == TCP_SND_BUF)) {
break;
Expand Down
4 changes: 2 additions & 2 deletions libraries/SDFS/src/SDFSFormatter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ class SDFSFormatter {
esp8266::polledTimeout::periodicFastMs timeToYield(5); // Yield every 5ms of runtime
for (uint32_t i = 0; i < count; i++) {
if (timeToYield) {
delay(0); // WDT feed
yield(); // WDT feed
}
if (!card->writeData(cache->data)) {
DEBUGV("SDFS: Clear FAT/DIR writeData failed");
Expand Down Expand Up @@ -383,7 +383,7 @@ class SDFSFormatter {
if (!card->erase(firstBlock, lastBlock)) {
return false; // Erase fail
}
delay(0); // yield to the OS to avoid WDT
yield(); // yield to the OS to avoid WDT
firstBlock += ERASE_SIZE;
} while (firstBlock < cardSizeBlocks);

Expand Down
2 changes: 1 addition & 1 deletion libraries/esp8266/examples/SerialStress/SerialStress.ino
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ void loop() {
if ((out_idx += local_written_size) == BUFFER_SIZE) {
out_idx = 0;
}
delay(0);
yield();

DEBUG(logger->printf("----------\n"));

Expand Down