arduino-libraries / ArduinoLowPower

Powersave features for SAMD boards
GNU Lesser General Public License v2.1
80 stars 57 forks source link

LowPower.idle() doesn't work as expected, and the argument to LowPower.idle(millis) has absolutely no effect. #39

Open be-philippe opened 3 years ago

be-philippe commented 3 years ago

This simple sketch (on a SAMD21, for ex. MKR WiFi 1010):

#include <ArduinoLowPower.h>

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

void loop()
{
  LowPower.idle();
  digitalWrite(0, !digitalRead(0));
}

produces a nice ~500 Hz square wave on pin D0, because the IDLE state is ended by the SysTick interrupt every ms. This is fine and useful, but should be properly documented.

Adding any value as argument to LowPower.idle(): (I use random() here to emphasize any)

  LowPower.idle((int)random(1000));

lowers the frequency to ~10.6 Hz.

Setting the RTC alarm takes ~46 ms, and then the IDLE state is ended on the next SysTick interrupt.

Most of the time needed to set the RTC alarm comes from synchronisation calls in RTCZero.cpp like

    while (RTCisSyncing())
      ;

I guess looping around the wfi instruction waiting for the right time to pass would be a better solution than using the RTC...

be-philippe commented 3 years ago

Possible solution:

void ArduinoLowPowerClass::idle(uint32_t delay)
{
  uint32_t ticks = delay;
  while (ticks > 0)
  {
    uint32_t lastMillis = millis();
    idle();
    uint32_t delta = millis() - lastMillis;
    if (delta > ticks) {
      ticks = 0;
    } else {
      ticks -= delta;
    }
  }
}
jsiddall commented 3 years ago

Note that it is possible to turn off systick which prevents idle from returning every millisecond:

SysTick->CTRL &= ~SysTick_CTRL_TICKINT_Msk;
LowPower.idle();
SysTick->CTRL |= SysTick_CTRL_TICKINT_Msk;
be-philippe commented 3 years ago

Sure, I know.

But what's the interest ? Without SysTick, delay(), millis(), etc. don't work anymore.

If the device is internally driven, say take a measure every so often and record it somehow, putting the controller to sleep is fine, and 50 ms of full speed CPU is an acceptable price to pay, even once per sec.

If you have to handle interrupts and debounce button presses, it's not an option.

jsiddall commented 3 years ago

Agree, use case is limited, but sometimes it might be useful. It relates to this issue because it solves the problem of "idle() doesn't work as expected" by preventing it from returning every millisecond.

However, I did some testing testing and looping on idle() every millisecond (ex: 100 loops for 100 milliseconds) takes only a few percent more power than waiting for an interrupt from the RTC with idle(100), so not a lot to be gained.

be-philippe commented 3 years ago

The way LowPower.idle() (the version without parameter) works is perfect for me, just as I said in my initial post. It just needs to be documented.

And yes, said documentation could explain how to disable SysTick, or even how to configure the controller such that it returns to idle state after having handled an interrupt.

One the other hand, LowPower.idle(uint32_t millis) doesn't at all do what it's supposed to, and needs to be fixed.