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

Proposal: add hal_jump_to_the_future_us(usec) #756

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

d-a-v
Copy link

@d-a-v d-a-v commented Jun 11, 2021

When MCU is put in sleep mode, micros() can be not updated.
A new function hal_jump_to_the_future_us(usec) is proposed to tell HAL that we just woke up in the future.
It is not enabled by default, and is conditioned by the macro HAL_ALLOW_FUTURE_JUMP.

edit:

  • micros() overflow and future_usec overflow are separately handled
  • overall cost when enabled: around 130 flash bytes on AVR

edit2: use case:
in onEvent():

    case EV_TXCOMPLETE:
        ...
        os_setTimedCallback(&sendjob, os_getTime() + sec2osticks(txIntervalSeconds), do_send);
        goDeepSleep = true;
        break;

in loop():

void loop()
{
    if (goDeepSleep) {
        goDeepSleep = false;
        // put mcu to deep sleep / power down (micros() won't work during this time)
        mcuDependentAPI_PowerDown(txIntervalSeconds);
        // MCU has just woken up, but `micros()` has the same value as before deep
        // sleep. So in order to not wait another `txIntervalSeconds`, we jump to the future:
        hal_jump_to_the_future_us((uint32_t)txIntervalSeconds * 1000000UL);
    }
    os_runloop_once();
}

edit3:
It allows to avoid specific code for every underlying MCU like this one for AVR.
TBH I was looking for an equivalent hack for the SAMD21 until I thought that such feature had to be generic.

@d-a-v
Copy link
Author

d-a-v commented Oct 11, 2021

@terrillmoore if this pull request is fair for further consideration,
Should the use case described above be integrated to another or added to an existing example, and/or should a paragraph about HAL_ALLOW_FUTURE_JUMP be added inside config.h ?

@terrillmoore
Copy link
Member

@d-a-v thanks for the contribution, but this is disruptive and conflicts with how we've done this in other ports. I don't want to add an API (forever) without a lot of thought.

@d-a-v
Copy link
Author

d-a-v commented Oct 11, 2021

I like to use the most standard and supported code I can find in supported libraries, but I haven't found low-power use-case in the examples.
For my information, what are you referring to with "done this in other ports" ?

@terrillmoore
Copy link
Member

For my information, what are you referring to with "done this in other ports" ?

Catena Arduino Platform, and MCCI's uses in non-Arduino environments. Despite the "arduino-lmic" name, this is very important to MCCI, and possibly for others.

In particular, MCCI's BSPs have a way to adjust the Arduino's answer on micros() and millis(). This allows other things to survive sleep properly, not just the LMIC, and lets us use more things unchanged. (Many things need to know about the passage of real time.) I tried pushing a change to Adafruit when we were heavily using SAMDs; they said "try upstream"; but I didn't have the time to deal with the resistance. And on non-Arduino platforms, the platform correctly keeps time through sleep.

If the Arduino core team ever comes up with a portable, cross-platform API for low-power systems, then we should adopt it. But that also involves thinking hard about how to handle loop() in that context -- constant polling is convenient but results in more power consumption than an event-driven system.

Anyway, panning up: there is a real cost (maintenance, test, etc) in putting things into the LMIC. Sorry I'm so conservative in this regard. (Example: I proposed #576 about a year and a half ago; it's a very simple, non-intrusive change, but it still had to wait a long time before I decided to incorporate it.) Also, there are support issues with any HAL change; it becomes another variable to consider when thinking about trouble reports, and... that's an issue, especially when timing is involved.

Not saying we won't do it, but didn't feel I had time to consider this properly for v4.1. Thanks...

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.

2 participants