Skip to content

Make delay() as overridable as yield() already is, and add overridable loop_end() #6306

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

Merged
merged 4 commits into from
Jul 18, 2019

Conversation

dok-net
Copy link
Contributor

@dok-net dok-net commented Jul 16, 2019

The Arduino delay() (at least on AVR) cyclically calls yield(), whereas on Espressif SOCs the delay scheduling is quite different. In order to implement cooperative scheduling in a portable fashion, with global yield() and delay() in large sketches upholding the principle of least surprise, the delay() function must be supplanted. This patch allows that by the usual weak linkage approach.
Additionally, a weakly-linked loop_end() is added, that allows libraries to add special main-loop functionality, in case the Schedule recurrent et cetera functionality is not appropriate.

(EspSoftwareSerial is updated to 5.2.3, containing review results for circular_queue and minor internals)

Copy link
Collaborator

@earlephilhower earlephilhower left a comment

Choose a reason for hiding this comment

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

LGTM. Not sure of the use case, as changing the meaning of delay() in a RTOS/cooperative OS would make many sketches do odd things, but no real problem with the way it's broken up.

@dok-net
Copy link
Contributor Author

dok-net commented Jul 17, 2019

@earlephilhower First, thank you for the positive review. Now, I am honestly intrigued if you could give me some input regarding the unexpected consequences on ESP8266 sketches it can bring, if other co-op tasks/threads, besides the WLAN stack that's already there, are running. I was actually naïve enough to think that except the somewhat more slugging behavior of yield, and less precise returns from delay(), on the ESP8266 of all Arduino platforms, independent sketches should run quite fine without adaptations. As I think you've mentioned, once (Free)RTOS has to be implemented, it's getting even more interesting. Real Time OSs are really hard on their users compared to server/desktop OSes. Unfortunately I am having a hard time convincing the ESP32 maintainer of the utility of ESP8266 portability or cooperative multitasking in general. Honest, I don't think I want to HAVE TO know whether tasks at same priority are preempted among each other, or if higher priority tasks can only start once a lower priority one yields (there's a "preemptive" OS build setting than can be true or false!), just to serve a few sensors readings from the run-of-the-mill Arduino driver libraries to a webserver and control some outputs, too. And that's from a professional developer who finds this interesting, I don't think many of the 16-26 "maker" crowd feel keep on learning real time OS programming...
In short, do you have any use case, sketch, that has trouble with cooperative scheduling? Rather, I am still hoping that my work can help with BearSSL and ethernet drivers?

@earlephilhower
Copy link
Collaborator

I've seen lots of bit banging in sketches (not in the lib, except in SWSerial), and they normally do a {set gpios; delay(period); set gpios; delay(period);...} loop to do whatever. For example, a simple I2C interface over general IO pins. Or, say, WiFi connection timeouts where you give up to 1s to connect and if not, reboot. True, delay() could have some jitter today, but it is relatively small because only the OS task is doing anything. If delay() could take 100x because some other thread was calculating PI to 2000 digits, things would go pear shaped there quite fast.

I haven't used FreeRTOS in anger, but I think the real difference between them and us, for the user, is cooperative (i.e. a yield() or WaitNextEvent under Win32 or (mumble mumble) under OSX) vs. preemptive (i.e. process/thread models under any full-fat OS today). Cooperative is harder to write since you need to break work into chunks, so even a FreeRTOS with every user process at the same priority level (assuming fair round-robin scheduling) would be liberating for them.

Right now, everything we have is cooperative (between the ROM blob and the user app) anyway. So apps already have it, but the implicit contract is that delay(x) = a wall time delay of [x, x+epsilon], where epsilon is smallish...

@dok-net
Copy link
Contributor Author

dok-net commented Jul 17, 2019

I almost completely agree - just I don't see much of a problem with it. EspSoftwareSerial works fine with CoopTask performing an "alive" blink in concurrent task/"sketch". The more delays, the better, because it keeps overall delay() deviation small. It's not a multiuser system, so one should expect the programmer to know what they are doing, at the of the day.
Where we differ is the assessment if coop or preempt is harder to write. Pouring yields() all over the code isn't that hard, for the benefit of no critical sections and semaphores :-)

@earlephilhower earlephilhower merged commit d968435 into esp8266:master Jul 18, 2019
@dok-net
Copy link
Contributor Author

dok-net commented Jul 18, 2019

@earlephilhower Oh great, thanks! I could use some help with getting PR #6307 approved, if there are any changes necessary, I am quite openminded, but for my scheduling needs, recursive scheduling from a function that's running off the scheduler itself is a must-have - my PR should fix that.

@dok-net dok-net deleted the new_pluggable branch July 19, 2019 08:37
@dok-net
Copy link
Contributor Author

dok-net commented Jul 20, 2019

@earlephilhower Come to think of it, we've both failed to mention the effects of the current series of merged patches for Schedule.h. So in fact, it's no longer true that there is only the smallish ROM delay bias, because yield() in core_esp8266_main.cpp and delay() in core_esp8266_wiring.cpp now call run_scheduled_recurrent_functions() all the time.

Straight from the documentation for that:

// Test recurrence and run recurrent scheduled functions.
// (internally called at every `yield()` and `loop()`)
void run_scheduled_recurrent_functions ();
// The purpose of recurrent scheduled function is to independantly execute
// user code in CONT stack on a regular basis.
// It has been introduced with ethernet service in mind, it can also be used
// for all libraries in the need of a regular `libdaemon_handlestuff()`.

With only minor hints at the havoc this may/will cause to code that depends on timely returns from delay() in both directions:

// recurrent scheduled function:
[…]
// * Note that it may be more than <repeat_us> microseconds between calls if
//   `yield` is not called frequently, and therefore should not be used for
//   timing critical operations.
[…]
// * Long running operations or yield() or delay() are not allowed in the
//   recurrent function.
[…]
bool schedule_recurrent_function_us (const std::function<bool(void)>& fn, uint32_t repeat_us);

Consequently, cooperative linear task code, that keeps these same constraints in mind, is not at all worse that what libs scheduling their recurrent work via Schedule.h cause.

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.

3 participants