Should schedule_function() call esp_schedule()?

[matthijskooijman]

Basic Infos

• This issue complies with the issue POLICY doc.
• I have read the documentation at readthedocs and the issue is not addressed there.
• I have tested that the issue is present in current master branch (aka latest git).
  I tried, but the latest master requires GCC10 and I'm not sure how to get this installed.
• I have searched the issue tracker for a similar issue.
• I have filled out all fields below.

Platform

• Hardware: Feather Huzzah ESP8266
• Core Version: 2.7.4
• Development Env: Arduino IDE
• Operating System: Ubuntu

Settings in IDE

-fqbn=esp8266:esp8266:huzzah:xtal=80,vt=flash,exception=legacy,ssl=all,eesz=4M2M,ip=lm2f,dbg=Serial,lvl=CORE,wipe=none,baud=460800

Problem Description

I'm building a sketch that essentially always sleeps, but needs to wakeup on a pin interrupt and then do some network traffic.

Some observations & assumptions:

• When using light sleep, AFAIU I need to put esp_yield() in my loop for when it has nothing left to do (to ensure the CPU can sleep, rather than continuously having to keep calling loop()).
• A sketch can be woken up from sleep by an interrupt using attachInterrupt() with e.g. the ONHIGH_WE.
• You cannot do real work from an ISR (e.g. do network traffic), this must be done from the main loop (or another task). This can be done using schedule_function to call some code after the next loop iteration.
• In the main loop function, the esp_yield(), will not return until esp_schedule() is called elsewhere.
• So to make this work, from the interrupt handler, I now have to call both schedule_function() to schedule some code to run later, and esp_schedule() to actually make sure the loop function continues and the scheduled function can run.
• When using other code that calls schedule_function() (e.g. Ticker.once_scheduled()) that I have no control over, this breaks (so e.g. for Ticker I have to do a non-scheduled Ticker.once() with a function that calls both schedule_function() and esp_schedule(), but that might not be always possible with other interfaces).

Maybe I'm misunderstanding how to approach my original problem, but if the above approach is correct, then maybe it would be good if schedule_function() would just always call esp_schedule()? (or, if it is harmful to call it when already inside the loop, then maybe the call can be done only when outside the main loop, e.g. by checking can_yield() or so?).

Doing so would mean that schedule_function() would just always work to ensure the given function is called, even when the mainloop is not currently running?

Sketch

To illustrate the problem, here's something that I think should turn the led on pin 0 on (LOW), and schedule a function that turns the led off after 5 seconds. I left out the actual sleeping and interrupt handling out, to simplify the example. If it helps to clarify, I can provide a more complete example with those as well.

#include <Arduino.h>
#include <Ticker.h>

Ticker ticker;

void setup() {
  pinMode(0, OUTPUT);
  digitalWrite(0, LOW);
}

extern "C" void esp_yield(void);

void loop() {
  ticker.once_scheduled(5, [](){ digitalWrite(0, HIGH); });
  esp_yield();
}

Debug Messages

SDK:2.2.2-dev(38a443e)/Core:2.7.3-3-g2843a5ac=20703003/lwIP:STABLE-2_1_2_RELEASE/glue:1.2-30-g92add50/BearSSL:5c771be

(nothing else appears on serial)

[d-a-v]

Here is a quick hack for a test-proposal:

• use recurrent scheduled function instead of scheduled functions (interval can be 0 and the lambda must return false so it is executed once, if that matches your use-case),
• apply this minor change in the core (cores/esp8266/core_esp8266_main.cpp)

static inline void esp_yield_within_cont() __attribute__((always_inline));
static void esp_yield_within_cont() {
+       run_scheduled_recurrent_functions();
        cont_yield(g_pcont);
        s_cycles_at_yield_start = ESP.getCycleCount();
-       run_scheduled_recurrent_functions();
}

extern "C" void __esp_yield() {
    if (can_yield()) {
        esp_yield_within_cont();
    }
}

With this patch, (recurrent) scheduled functions are called before effectively yielding instead of after.

Background: recurrent scheduled functions are scheduled functions:

• that auto trigger themselves at given interval until they return false
• of which interval checking occurs at every loop and every yield

edit: embedded Ticker library does not use recurrent scheduled functions.

[matthijskooijman]

Thanks for your suggestion. However, I think it does not solve the problem yet, since recurrent functions are also only called when the loop is either running, or about to be suspended with esp_yield()? IOW, they wil not cause the loop to resume, just like normal scheduled functions cannot do so?

I guess that this does mean that my original request to call esp_schedule from schedule_function should maybe be extended to schedule_recurrent_function as well?

I haven't tested this yet (no hardware nearby), but IIUC you suggest changing my tests sketch like this:

#include <Arduino.h>
#include <Ticker.h>

Ticker ticker;

void setup() {
  pinMode(0, OUTPUT);
  digitalWrite(0, LOW);
}

extern "C" void esp_yield(void);

void loop() {
  ticker.once(5, [](){
    schedule_recurrent_function_us(0 /*us*/, []() {
      digitalWrite(0, HIGH);
      return false; /* Run only once */
    });
  });
  esp_yield();
}

AFAICS, even with your suggested patch to the core, this will not run the recurrent function?

Given that the recurrent function can be run at a later time as well, it could also replace the Ticker completely. However, AFAICS that won't fix the problem (now nothing runs after 5 seconds, rather than just the ticker firing and scheduling a function that does not run). Also, AFAICS recurrent functions do not actually set timers, so they cannot cause wakeup from sleep, unlike Ticker (which is the reason I'm esp_yielding in the first place).

For completeness, I think this would look like this:

#include <Arduino.h>

void setup() {
  pinMode(0, OUTPUT);
  digitalWrite(0, LOW);
}

extern "C" void esp_yield(void);

void loop() {
  schedule_recurrent_function_us(5000000 /*us*/, []() {
    digitalWrite(0, HIGH);
    return false; /* Run only once */
  });
  esp_yield();
}

[devyte]

Why are you calling esp_yield inside the loop? remove that.
Edit: oh, I think I understand what you're trying to do. You basically want to suspend CONT, and have it resume later when a pin interrupt happens.

• How often does the external pin interrupt happen?
• Why light sleep?
• Have you looked at deep sleep?
• Why a scheduled function at all?

I would add logic to the loop to detect the start and end of your network transaction.

• The start condition would get triggered by the external interrupt.
• The end condition would get triggered once the transaction is done, and would result in suspending CONT
• The interrupt would just trigger the start and wouldn't schedule anything, nor would it do any work.

[matthijskooijman]

From my original post:

When using light sleep, AFAIU I need to put esp_yield() in my loop for when it has nothing left to do (to ensure the CPU can sleep, rather than continuously having to keep calling loop()).

And:

I left out the actual sleeping and interrupt handling out, to simplify the example. If it helps to clarify, I can provide a more complete example with those as well.

[devyte]

See my edit above.

[matthijskooijman]

How often does the external pin interrupt happen?

When I press a button, usually minutes apart.

Why light sleep?

To stay connected to the AP and have the fastest possible response time after pressing the button combined with the lowest power usage (battery powered application).

Have you looked at deep sleep?

Yes, but that would have longer response time due to needing to reassociate to the network.

Why a scheduled function at all?

Because doing network traffic (i.e. an HTTP request and waiting from the response) did not work from inside the button ISR (which makes sense), but also not from the Ticker handler IIRC (which I think is just another task and not a timer ISR, but it still didn't work). It's been a while since I dug into this, but IIRC some of the networking code was written in a way that only worked from inside the main loop / CONT.

The start condition would get triggered by the external interrupt.
The end condition would get triggered once the transaction is done, and would result in suspending CONT
The interrupt would just trigger the start and wouldn't schedule anything, nor would it do any work.

Note that my actual case is slightly complicated since the work is done 1000ms after the interrupt (which is what I'm using Ticker for now), but the flow could be the same (ISR triggers Ticker, ticker triggers start condition).

But this would also require the interrupt to call esp_schedule to resume CONT, right? So then I would call esp_schedule() and flag a start condition, which is handled by the loop. In that case, I might as well call esp_schedule() and schedule_function() (which allows slightly nicer code structure, since the code for the work can be kept where it is triggered, rather than in the loop), which is also my current workaround.

But the main point of this request is that IMHO schedule_function should just work, even when CONT is suspended, by resuming CONT when it is suspended (regardless of why it is suspended of who is calling schedule_function why).

[devyte]

Because doing network traffic (i.e. an HTTP request and waiting from the response) did not work from inside the button ISR (which makes sense), but also not from the Ticker handler IIRC (which I think is just another task and not a timer ISR, but it still didn't work). It's been a while since I dug into this, but IIRC some of the networking code was written in a way that only worked from inside the main loop / CONT.

That's not what I meant by "why a scheduled function at all". You can't do network ops from an ISR, and the Ticker cb executes in SYS, not in CONT, so also no. But that doesn't mean a scheduled function. What I meant was: why are you set on a scheduled function at all? There are other ways to trigger loop code, such as a global flag, which is what I'm suggesting with the start/end.

You can think of it as a state machine.

• The initial state, I. e. first time through, it calls the sleep code (yield)
• An external trigger, I. e. the ISR, resumes the loop (schedule), and transitions to start state
• After whatever network initial comms are done, you transition to a comms state to actually do the communications
• Once the comms are done, you transition into the initial state again, I. e. sleep (yield)
• the above should have appropriate error handling and timeouts
• all of the above doesn't happen in a single loop pass, the loop executes as many times as needed to achieve all of the transitions. In each pass, the code decides which state handler has to be called

Of course, I'm simplifying. There could be more states needed in your code, that's something you would need to figure out.

[matthijskooijman]

There are other ways to trigger loop code, such as a global flag, which is what I'm suggesting with the start/end.

Thanks for that suggestion, I can see how that would indeed work. However, that's not the way I want to structure my code, scheduling functions to be called ASAP with schedule_function is pretty much exactly what I do want. So, I come back to my original suggestion.

Any response to the suggestion itself? If the answer to my suggestion is "No, schedule_function should not call esp_schedule "because that causes problems or is unwanted for other reasons, and/or "If you call esp_yield, you're responsible to call esp_schedule yourself too, that's fine, then this issue can be closed and I'll stick to my current approach (using the non-scheduled TIcker and calling both schedule_function and esp_schedule in the non-scheduled callback).

[devyte]

At top level, you're not supposed to use esp_yield/esp_schedule yourself. Those are low level functions that deal with the interactions between SYS and CONT. The user is supposed to only call yield/delay, which wraps the details. As such, if you do decide to use them, strictly speaking you are responsible for doing it correctly.

Having said that, your general idea seems to have merit, and I would say it's a valid use case.

Changing how the scheduled functions are currently called on switching to SYS and back could cause trouble. We've already gone through several incarnations of that, and I'd prefer to keep it as it is, i. e. call on returning.

There is also the fact that the interrupt could come in while there is an ongoing delay, in which case things might get... interesting.

Let's see if your current approach can be improved.

[d-a-v]

@matthijskooijman

From OP:

When using other code that calls schedule_function() (e.g. Ticker.once_scheduled()) that I have no control over, this breaks (so e.g. for Ticker I have to do a non-scheduled Ticker.once() with a function that calls both schedule_function() and esp_schedule(), but that might not be always possible with other interfaces).

it would be good if schedule_function() would just always call esp_schedule()?

After rereading, there is a misunderstanding from my side.
Let's suppose esp_schedule() (that will "unsuspend" the next call to esp_yield() - or resume the current one) is called by schedule_function() which is itself called by ticker.once_scheduled(5 seconds, ...)

void loop() {
  ticker.once_scheduled(5, [](){ digitalWrite(0, HIGH); });
  esp_yield();
}

The esp_yield() above will be "unsuspended" because the added call to schedule_function().
That will lead to the end of the loop but will not trigger your user tickered function (delayed to 5s later).
Then loop restarts, and the tickered function is replaced by a new one that is supposed to be fired 5s later.
And this story loops over. The sketch will not be sleeping, but the tickered function will not be fired.

Scheduled function will not work (*) if there is an esp_yield() that prevents any further loop (with or without any esp_schedule() at registering time). Once esp_yield() has been executed, the esp has to be waken up by something coming from SYS (an ISR, a timer, network event...).

(*) except if delay is 0 and recurrent scheduled functions are used, that leads to your first answer:

If we claim that every esp_yield() should match an esp_schedule(), (push/pop, call/return, malloc/free) then your first example in your first answer

void loop() {
  ticker.once(5, [](){
    schedule_recurrent_function_us(0 /*us*/, []() {
      digitalWrite(0, HIGH);
      return false; /* Run only once */
    });
  });
  esp_yield();
}

should be written like:

void loop() {
  ticker.once(5, [](){
    schedule_recurrent_function_us(0 /*us*/, []() {
      digitalWrite(0, HIGH);
      return false; /* Run only once */
    });
    esp_schedule(); // <== unhidden esp_yield() counterpart
  });
  esp_yield();
}

Thus, I agree with you about this:

But this would also require the interrupt to call esp_schedule to resume CONT, right? So then I would call esp_schedule() and flag a start condition, which is handled by the loop. In that case, I might as well call esp_schedule() and schedule_function() (which allows slightly nicer code structure, since the code for the work can be kept where it is triggered, rather than in the loop), which is also my current workaround.

And disagree with that:

But the main point of this request is that IMHO schedule_function should just work, even when CONT is suspended, by resuming CONT when it is suspended (regardless of why it is suspended of who is calling schedule_function why).

.. because

• one _yield one _schedule is a nice rule :)
• resuming CONT (calling esp_schedule()) at registering time makes no sense when the time interval is not 0.
  Would it make sense to call esp_schedule() only when time interval is 0 ?

However, that's not the way I want to structure my code, scheduling functions to be called ASAP with schedule_function is pretty much exactly what I do want. So, I come back to my original suggestion.
Any response to the suggestion itself?

You had the answer before I made my own way through it:

"If you call esp_yield, you're responsible to call esp_schedule yourself too"

and I'll stick to my current approach (using the non-scheduled TIcker and calling both schedule_function and esp_schedule in the non-scheduled callback).

That's indeed my POV. And your use-case should go into or update an already existing example.

[matthijskooijman]

Thanks for more in-depth replies, much appreciated.

Responding to @devyte first:

At top level, you're not supposed to use esp_yield/esp_schedule yourself. Those are low level functions that deal with the interactions between SYS and CONT. The user is supposed to only call yield/delay, which wraps the details.

Yeah, I guessed as much, but I couldn't think of any way to get the light sleeping to work correctly without esp_yield().

Having said that, your general idea seems to have merit, and I would say it's a valid use case.

Ideally, there would be a well-supported/well-defined API to support this usecase, if esp_yield() / esp_schedule() is not it :-)

There is also the fact that the interrupt could come in while there is an ongoing delay, in which case things might get... interesting.

Do you mean the case where, inside the delay CONT is currently suspended but will be resumed after a timer, so calling esp_schedule() in that case would end up with one resume to many in the queue? If so, this is essentially the one suspend == one resume rule that @d-a-v also states.

This one-on-one mapping is something that might indeed be tricky to take into account (In my original sketch that I did not show here, these "extra resumes" might also occur, though since the loop does not do anything except esp_yield() that probably does not cause any issues).

Thinking about this, maybe a higher level API for this might also solve the one-on-one mapping problem, by exposing a suspend_loop() function that suspends the loop and resume_loop() that resumes the loop only if it is currently suspended? I.e. something like:

bool loop_suspended = false;
void suspend_loop() {
  loop_suspended = true;
  esp_yield();
}
void resume_loop() {
  if (loop_suspended) {
    loop_suspended = false
    esp_schedule();
  }
}

The idea as that there is a separate mechanism (the loop_suspended variable) that ensures that for these higher level API functions will balance their yield/schedule calls. And then things like schedule_function could maybe just call resume_loop, which just does the right thing in all cases.

However, after writing the above, I realize that this does not actually work like this yet, i.e. if you suspend_loop and something else calls esp_schedule and then you call suspend_loop again, the balance is off. I'd have to think a bit about how this API should work exactly (and what the constraints on them are), but I'm out of time for now :-)

Then, @d-a-v writes:

The esp_yield() above will be "unsuspended" because the added call to schedule_function().
That will lead to the end of the loop but will not trigger your user tickered function (delayed to 5s later).

resuming CONT (calling esp_schedule()) at registering time makes no sense when the time interval is not 0.
Would it make sense to call esp_schedule() only when time interval is 0 ?

I think both of these statements suggest that you're thinking about calling esp_schedule at the moment where you start the ticker? My suggestion would to call it inside schedule_function, which is called by Ticker after 5s, not when starting the ticker?

Another way to look at it, is to note that schedule_function always schedules a function to be run ASAP, which somewhat corresponds to interval=0.

However, I do now realize that my suggestion could only possibly work for schedule_function, not for schedule_recurrent_function, since that does not trigger a timer, but just polls the time and thus only works when the mainloop is still running. So my suggestion would introduce some inconsistency then, if schedule_function would work like this and schedule_recurrent_function would not.

one _yield one _schedule is a nice rule :)

Yeah, I think I can imagine how that would be required. Then having a function that maybe calls esp_schedule is going to be counter-productive.

That's indeed my POV. And your use-case should go into or update an already existing example.

That would indeed make sense. Maybe the documentation could also be somewhat updated about how this stuff works internally (even if a better API for this would be added).

[d-a-v]

The esp_yield() above will be "unsuspended" because the added call to schedule_function().
That will lead to the end of the loop but will not trigger your user tickered function (delayed to 5s later).

resuming CONT (calling esp_schedule()) at registering time makes no sense when the time interval is not 0.
Would it make sense to call esp_schedule() only when time interval is 0 ?

I think both of these statements suggest that you're thinking about calling esp_schedule at the moment where you start the ticker? My suggestion would to call it inside schedule_function, which is called by Ticker after 5s, not when starting the ticker?

This API is tricky. esp_schedule() does not necessarily act at the time of calling it. This function posts an event in the underlying OS (that should be freertos hidden behind the nonosdkapi). If it was posted in CONT, this event is taken into account after esp_yield() is called; When posted from SYS, it wakes up from last already-called-yield.
Briefly, this event triggers resuming to the last esp_yield() at next context change.

So my suggestion would introduce some inconsistency then, if schedule_function would work like this and schedule_recurrent_function would not.

Historically, scheduled functions are triggerred "ASAP" meaning at next loop. They can be much delayed in some situations.
There was a need to trigger them both regularly and also before next loop, in case of too large delay. So recurrent scheduled function appeared, with polled timers, but are checked also when yield() is called. They are both polled/software only.

That would indeed make sense. Maybe the documentation could also be somewhat updated about how this stuff works internally (even if a better API for this would be added).

Someone may correct me about what's above. Indeed that should be documented. That is (probably) mainly the result of fine hacking nonos-sdk to cope with both its requirement and arduino needs (just check cont.S). This is not current maintainers work but should give insights on how to move to rtos-sdk while still keeping up with the current model.