rocketscream / Low-Power

Low Power Library for Arduino
www.rocketscream.com
1.27k stars 346 forks source link

Suggestion for improvement to allow calibration of Watchdog Timer period #111

Open julianmorrison opened 3 years ago

julianmorrison commented 3 years ago

Great library and I am using it in my project. I need to be able to get a more accurate duration of the watchdog timout period so I can correct for this in the application. My measurements show that the watchdog timer is not very accurate at all! eg using SLEEP_4S produces a time of over 4.5 seconds on my Nano. This is not a problem as long as I can correct for this. What's needed is a way to call LowPower.powerDown (for example) with a flag so it doesn't actually go to sleep and keeping millis() running when it is set. Then I could calculate the time before and after the call and get a reasonably accurate millisecond duration.

I made some changes my copy of the library and it all seems to work as I wanted: LowPower.h

// add noLowPower argument to powerDown with default for backward compatability void powerDown(period_t period, adc_t adc, bod_t bod, bool noLowPower = false) attribute((optimize("-O1"))); // add new static flag static volatile bool wdtFinished;

LowPower.cpp

// add new wdtFinished flag volatile bool LowPowerClass::wdtFinished;

// use new noLowPower argument to prevent entering low power and block until wdt has fired void LowPowerClass::powerDown(period_t period, adc_t adc, bod_t bod, bool noLowPower) { if (adc == ADC_OFF) ADCSRA &= ~(1 << ADEN);

if (period != SLEEP_FOREVER) { wdt_enable(period); WDTCSR |= (1 << WDIE); }

if (noLowPower) { wdtFinished = false; while (!wdtFinished) {;} } else { if (bod == BOD_OFF) {

if defined (AVR_ATmega328P) || defined (AVR_ATmega168P)

    lowPowerBodOff(SLEEP_MODE_PWR_DOWN);
  #else
    lowPowerBodOn(SLEEP_MODE_PWR_DOWN);
  #endif
}
else
{
  lowPowerBodOn(SLEEP_MODE_PWR_DOWN);
}

if (adc == ADC_OFF) ADCSRA |= (1 << ADEN);

} } // set wdt finished flag ISR (WDT_vect) { // WDIE & WDIF is cleared in hardware upon entering this ISR wdt_disable(); LowPowerClass::wdtFinished = true; }

I then use it it: // calibrate watchdog timeout sleepPeriodMs = millis(); LowPower.powerDown(SLEEP_4S, ADC_OFF, BOD_ON, true); sleepPeriodMs = millis() - sleepPeriodMs; Serial.println(sleepPeriodMs);

I have only implemented the change to powerDown but it could also be added to the other calls.

Cheers, Julian

MojaveTom commented 3 years ago

Julian,

See my suggestion #112. My suggestion is similar in some ways to yours, but with some improvements. You should be aware that the WDT on most micros is not crystal controlled, so the WDT clock varies significantly with supply voltage and temperature. Trying to get a consistent time from it is probably futile. In any event, if you are determined to proceed, you should not rely on the millis() function as a timing standard. I suggest that you put a simple print command in your loop() function that prints out the micro's idea of time, and record it in a terminal app that can timestamp the received data. (On my Mac, "CoolTerm" has this capability.) If you do this, your time standard is your host computer, which should be much better than the millis() function on your micro, also, you may not need to spin wait for the time period to elapse in the powerDown(...) function if you incrementally add the wait time to the millis() results. Something like this:

uint32_t time = 0, now = 0;

void loop() {
  now = millis();
  Serial.print("My time is: ");
  Serial.print(time);
  Serial.print(", millis: ")
  Serial.println(now);
...
  time = time + 8000 + millis() - now;
  LowPower.powerDown(SLEEP_8S, ADC_OFF, BOD_OFF);
// end of loop
}

(compliments of Felix of LowPowerLab.com)

Using this approach, on one of my Moteino systems from LowPowerLab, the WDT timer is about 90% of real time. I'm using time to decide when to sample my sensors and the timing is very non-critical; so this time is just fine. The sensor data is tagged with a more precise timestamp when it is received by the host.

Tom

julianmorrison commented 3 years ago

Good idea Tom. However, I am not trying to get absolute accuracy and apricate that the system clock driving millis() is not very accurate (unless is crystal driven). However the watchdog is driven by another internal R/C oscillator which is WAY less accurate and as you say drifts with voltage and temperature. All I was trying to do is periodically calibrate the watchdog time period against the better time millis() source. I am doing a calibration every 60 seconds during a long 5min sleep (in 4s chunks) and I can now determine fairly accurately what time was spent sleeping.

MojaveTom commented 3 years ago

Julian,

Sounds like a plan. For your purposes, what you are doing is probably the way to go.

Tom

On Feb 16, 2021, at 10:05 AM, julianmorrison notifications@github.com wrote:

Good idea Tom. However, I am not trying to get absolute accuracy and apricate that the system clock driving millis() is not very accurate (unless is crystal driven). However the watchdog is driven by another internal R/C oscillator which is WAY less accurate and as you say drifts with voltage and temperature. All I was trying to do is periodically calibrate the watchdog time period against the better time millis() source. I am doing a calibration every 60 seconds during a long 5min sleep (in 4s chunks) and I can now determine fairly accurately what time was spent sleeping.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/rocketscream/Low-Power/issues/111#issuecomment-779977916, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADL2G3KZ5EGHUTNN2T4XNR3S7KQVVANCNFSM4XU7C3HA.

eddyp commented 3 years ago

In my application i used @LowPowerLab's fork and because I have some buttons that can wake up the system, for debouncing them and to have a responsive system, I am actually starting from the lowest sleep interval 15MS, and increase to the next sleep interval until I get to 8s per sleep cycle.

When a button is pressed, I reset once again the sleep period, so debouncing happens as fast as possible.

The problem for me is that I also need to have my own ISR(WDT_vect) so I can read which was the achieved sleep period and to mark the sleep cycle was complete, so I can add the sleep period to my "allMillis" count.

I am aware I am losing the count of millis for incomplete sleep cycla and that the time is inaccurate, but for my application, it doesn't really matter as my system should typically do some active work at very long intervals (e.g. every 3 hours or even 6 hours) and precision is really not important.

I am wondering if instead of forking, it isn't possible to add some hooks of optional behaviors so the library provides it by default.

I'm thinking this might work:


class LowPowerClass
{
    public:

    #if (defined(LOW_POWER_WITH_COMPLETE_SLEEP_FLAG))
    volatile bool wdtFinished = false;
    #endif

    #if (defined(LOW_POWER_WITH_WDT_PRESCALER_READ))
    volatile byte wdtCompletedWDP;
    #endif

    ...

}

...
ISR(WDT_vect)
{
    #if (defined(LOW_POWER_WITH_COMPLETE_SLEEP_FLAG))
    wdtFinished = true;
    #endif

    #if (defined(LOW_POWER_WITH_WDT_PRESCALER_READ))
    byte wdtcsr = WDTCSR;
    wdtCompletedWDP = (wdtcsr & 0x20 >> 2) | (wdtcsr & 0x07)
    #endif

    wdt_disable();
}

...

void idle(....)
{

    noInterrupt();

    #if (defined(LOW_POWER_WITH_COMPLETE_SLEEP_FLAG))
    wdtFinished = false;
    #endif

    #if (defined(LOW_POWER_WITH_WDT_PRESCALER_READ))
    byte wdtcsr = WDTCSR;
    wdtCompletedWDP = (wdtcsr & 0x20 >> 2) | (wdtcsr & 0x07)
    #endif

}  

So in my main program I can do:


#define LOW_POWER_WITH_COMPLETE_SLEEP_FLAG
#define LOW_POWER_WITH_WDT_PRESCALER_READ
#include <LowPower>

...
const ulong wdtToMillis[] = {16, 32, 64, 125, 250, 500, 1000, 2000, 4000, 8000};
...

    idle(...);

    if (LowPower::wdtFinished) 
    {
         millis_in_sleep += wdtToMillis[LowPower::wdtCompletedWDT];
    }
    ...

unsigend long allMillis()
{
    return millis_in_sleep + millis();
}

It might make sense to add the allMillis() part or at least the millis_in_sleep calculation in the lib, too, under a flag.