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

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

Open
julianmorrison opened this issue Feb 15, 2021 · 4 comments

Comments

@julianmorrison
Copy link

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
Copy link

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
Copy link
Author

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
Copy link

MojaveTom commented Feb 16, 2021 via email

@eddyp
Copy link

eddyp commented Jul 29, 2021

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.

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

No branches or pull requests

3 participants