arduino / Arduino

Arduino IDE 1.x
https://www.arduino.cc/en/software
Other
14.15k stars 7.01k forks source link

set_sleep_mode(SLEEP_MODE_IDLE) has no effect on on Arduino Uno with IDE 1.8 #6933

Closed chrispbarlow closed 6 years ago

chrispbarlow commented 6 years ago

From https://github.com/chrispbarlow/arduino-tasks

void TaskSchedule::sleepNow(){
    set_sleep_mode(SLEEP_MODE_IDLE);    /* sleep mode is set here */

    sleep_enable();                     /* enables the sleep bit in the mcucr register */
                                        /* so sleep is possible. just a safety pin */
    power_adc_disable();                /* Power down some things to save power */
    power_spi_disable();
    power_timer0_disable();
    power_twi_disable();

    _schedLock = true;                  /* Prevent schedule from running on accidental wake-up */
    sleep_cpu();                        /* here the device is actually put to sleep!! */
/* THE PROGRAM IS WOKEN BY TIMER1 ISR */
}

With IDE 1.6, the MCU would go to sleep, and would be woken by the TIMER1 interrupt.

Since upgrading to IDE 1.8, the MCU stays awake. Using any of the other sleep modes, SLEEP_MODE_ADC, etc, the MCU goes to sleep, but is not woken by the timer (as expected).

It seems like there have been significant changes to avr/sleep.h between the two IDE versions, so I suspect the issue may lie there somewhere.

matthijskooijman commented 6 years ago

I believe sleep_enable() enables the sleep instruction for a few cycles, so it should be followed immediately by sleep_cpu() (with interrupts disabled to prevent an interrupt between them). e.g. see http://www.atmel.com/webdoc/avrlibcreferencemanual/group__avr__sleep.html

Perhaps the power_* functions were nops, or slightly faster before, so the sleep instruction would be soon enough, but it is not now? In any case, I'd say your code is wrong and should call he power_* stuff before sleep_enable().

chrispbarlow commented 6 years ago

Thanks for the quick response, I have it working now. Moving the sleep_enable() call didn't help, but I tried adding the cli() and sei() calls around the sleep_enable() and this works. It's strange that it works without them with the older IDE though.

The sleepNow method now looks like this:

void TaskSchedule::sleepNow(){
    power_adc_disable();                /* Power down some things to save power */
    power_spi_disable();
    power_timer0_disable();
    power_twi_disable();

    set_sleep_mode(SLEEP_MODE_IDLE);    /* sleep mode is set here */
    cli();
                                        /* so sleep is possible. just a safety pin */
    _schedLock = true;                  /* Prevent schedule from running on accidental wake-up */
    sleep_enable();                     /* enables the sleep bit in the mcucr register */
    sei();
    sleep_cpu();                        /* here the device is actually put to sleep!! */
/* THE PROGRAM IS WOKEN BY TIMER1 ISR */
    sleep_disable();        /* disable sleep */
    power_all_enable();         /* restore all power */
}

BTW, this is my first time working on an Arduino library, is this the correct forum to report things like this? Apologies if not.

chrispbarlow commented 6 years ago

Actually... I spoke too soon. It's still spinning in the loop() without going to sleep (or something is keeping it awake), but setting _schedLock in a critical section is making sure it isn't being reset by the ISR before the scbeduler is checking it (which was preventing any tasks from being dispatched before).

matthijskooijman commented 6 years ago

Are you sure it doesn't sleep very shortly everytime, being woken up by some interrupt (such as the timer0 / millis interrupt that runs every ms)?

BTW, this is my first time working on an Arduino library, is this the correct forum to report things like this? Apologies if not.

Not really, this is for bugs and feature requests in the Arduino code itself (which this sortof started out as, but I highly suspect that this is just a problem with your code, especially since you're not really using any Arduino APIs there, just plain AVR stuff).

For this reason, I'll close this issue. A better place for these questions would be the forum.