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.

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

Shimmable suspend-CONT API for async esp_yield() / esp_schedule() "stop the Arduino world" metaphor #6782

Closed
wants to merge 20 commits into from

Conversation

dok-net
Copy link
Contributor

@dok-net dok-net commented Nov 15, 2019

In order to let Arduino-like scheduling libraries, that shim yield() and/or delay(), make use of CPU cycles that are available while async callbacks are awaited, the CONT task must not be completely stopped.
This PR changes the way async callbacks are handled in ESP8266WiFi, currently the only code that works with NonOS async callbacks in ESP8266 Arduino core and core libraries according to Unix grep.
Fixes #7969

@dok-net dok-net changed the title Change async esp_yield() / esp_schedule() "stop the Arduino world" metaphor sites Change async esp_yield() / esp_schedule() "stop the Arduino world"-metaphor sites Nov 15, 2019
@dok-net
Copy link
Contributor Author

dok-net commented Nov 15, 2019

FYI: Alters part of #6107. Conflicts/supersedes #6212. Of interest besides the obvious Arduino incompatibility, in my mind is the question of scheduling efficiency: does the esp scheduler internally consume a significantly different number of cycles for a suspended (esp_yield()) CONT task while a Timer is active, vs. starting, then suspending again, a busy-locked CONT task that checks a condition continually. And if so, are those cycles put to any better use.

Copy link
Collaborator

@devyte devyte left a comment

Choose a reason for hiding this comment

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

This will require extensive testing for functional correctness in different scenarios, behavior if new code vs. current,. and power usage.
About that last, it is my understanding that a loop-yield is more cpu intensive than a loop-delay (n). Also, the sweet spot for such loops is delay(5), determined both from past experiences and from independent conclusions reached recently by @Tech-TX.

At a glance, I see at least one code change that changes behavior: replacing delay(0) by yield(). They are not equivalent in this context. I suspect apps that scan from SYS, e. g. from callbacks that run there, would break.

I'll leave specific comments once I review in detail.

@dok-net
Copy link
Contributor Author

dok-net commented Nov 15, 2019

@devyte Delay(0) vs yield() in master, actual code path unrolled, illegal (panic) path and do nothing pruned:

delay(0) =>

esp_schedule();
cont_yield(g_pcont);
run_scheduled_recurrent_functions(); 

yield() =>

esp_schedule();
cont_yield(g_pcont);
run_scheduled_recurrent_functions();

@dok-net
Copy link
Contributor Author

dok-net commented Nov 15, 2019

esp_schedule() can be un-removed in the CBs if there is no ill effect in calling it on non-suspended CONT, just to make the change smaller. Reverting to delay(x) (x > 0) though turns this PR on its head.

@dok-net
Copy link
Contributor Author

dok-net commented Nov 15, 2019 via email

@devyte
Copy link
Collaborator

devyte commented Nov 15, 2019

No hurry, this is one of those cases where extremely careful consideration and thought is needed, or you risk breaking/worsening very subtle things that eventually blow things up in seemingly unrelated places. I suggest taking your time, doing broad and careful testing, and reporting your findings/progress back here for further discussion.

@dok-net
Copy link
Contributor Author

dok-net commented Nov 16, 2019

With the latest commit, this PR is supposed to introduce basically no difference in behavior over master, when the weak bindings are not touched.
One pattern is required and implemented, though.
For any delay() that can be resumed by esp_schedule(), a guard loop is required, like so:

//delay(timeout_ms);
auto op_start_time = millis();
// will continue on timeout or when wifi_dns_found_callback fires
while (millis() - op_start_time < timeout_ms && _dns_lookup_pending) {
    esp_delay(timeout_ms);
}

Use esp_delay(1) in order to "give scheduled functions a chance to run (e.g. Ethernet uses recurrent)".

For any esp_yield() that is resumed by esp_schedule(), a guard is also required, for example:

//esp_yield();
_wps_config_pending = true;
// will continue when wifi_wps_status_cb fires
while (_wps_config_pending) {
    esp_yield();
}
_wps_config_pending = false;

@dok-net dok-net force-pushed the esp_yield_mt branch 4 times, most recently from 20d3d9c to df917d6 Compare November 20, 2019 06:44
@dok-net dok-net changed the title Change async esp_yield() / esp_schedule() "stop the Arduino world"-metaphor sites Shimmable suspend-CONT API for async esp_yield() / esp_schedule() "stop the Arduino world" metaphor Nov 20, 2019
@dok-net dok-net requested a review from devyte November 22, 2019 10:27
@dok-net dok-net force-pushed the esp_yield_mt branch 2 times, most recently from 09b3ddc to 6143da6 Compare November 24, 2019 15:33
@dok-net dok-net force-pushed the esp_yield_mt branch 7 times, most recently from 6be60a4 to 9133c23 Compare December 17, 2019 16:17
@dok-net dok-net force-pushed the esp_yield_mt branch 3 times, most recently from 7748ea0 to 905f269 Compare May 25, 2021 16:06
@dok-net dok-net force-pushed the esp_yield_mt branch 3 times, most recently from e65a724 to d07620d Compare June 8, 2021 01:48
…s esp_yield and esp_delay with function arguments.
Add host mockup for esp_delay overload
…rgument for C-style FP is now ONLY in 3rd template argument.
…scriptive esp_suspend(...).

Provide esp_suspend(void) as wrapper for esp_yield().
… don't thrash the recurrent functions scheduler by using intvl_ms != 0. Keeping the MCU out of the idle task increases power consumption.
@dok-net
Copy link
Contributor Author

dok-net commented Oct 16, 2021

This PR was included in merge c312a2e, and is therefore withdrawn.

@dok-net dok-net closed this Oct 16, 2021
@dok-net dok-net deleted the esp_yield_mt branch October 16, 2021 22:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

WiFiScan.ino ported to CoopTask libraries doesn't find any networks
2 participants