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

Open
wants to merge 48 commits into
base: master
from

Conversation

@dok-net
Copy link
Contributor

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.

@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

This comment has been minimized.

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 left a comment

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

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

Copy link
Contributor Author

dok-net commented Nov 15, 2019

@devyte

This comment has been minimized.

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 dok-net force-pushed the dok-net:esp_yield_mt branch from 3ecedc5 to d30f0b1 Nov 16, 2019
@dok-net

This comment has been minimized.

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 dok-net:esp_yield_mt branch 4 times, most recently from 20d3d9c to df917d6 Nov 18, 2019
@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 force-pushed the dok-net:esp_yield_mt branch from df917d6 to 5292b2f Nov 21, 2019
@dok-net dok-net requested a review from devyte Nov 22, 2019
@dok-net dok-net force-pushed the dok-net:esp_yield_mt branch 2 times, most recently from 09b3ddc to 6143da6 Nov 24, 2019
@dok-net dok-net force-pushed the dok-net:esp_yield_mt branch from 6143da6 to 3de4344 Dec 2, 2019
@dok-net dok-net force-pushed the dok-net:esp_yield_mt branch 7 times, most recently from 6be60a4 to 9133c23 Dec 11, 2019
@dok-net dok-net force-pushed the dok-net:esp_yield_mt branch from 9133c23 to 9a888e7 Dec 20, 2019
@dok-net dok-net force-pushed the dok-net:esp_yield_mt branch from c2eeffd to 6eb126e Jan 18, 2020
@dok-net dok-net force-pushed the dok-net:esp_yield_mt branch from 6eb126e to 00257fb Jan 19, 2020
dok-net added 17 commits Jan 21, 2020
…ATTR attribute.

This affects the unnamed namespace or static non-member functions.
…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.
@dok-net dok-net force-pushed the dok-net:esp_yield_mt branch from 00257fb to bb1d0e5 Jan 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.