search

rocketscream / Low-Power

Low Power Library for Arduino
www.rocketscream.com
1.26k stars 345 forks source link

SLEEP_MODE_EXT_STANDBY not declared on ATMega168 #14

Closed gfk closed 9 years ago

gfk commented 9 years ago

When I try to compile a sketch with #include <LowPower.h> in it (even if there's no other code) and I specify the ATMega168, I get this error:

In file included from /Users/gfk/Documents/Arduino/libraries/Low-Power-master/LowPower.cpp:27:0:
/Users/gfk/Documents/Arduino/libraries/Low-Power-master/LowPower.cpp: In member function 'void LowPowerClass::powerExtStandby(period_t, adc_t, bod_t, timer2_t)':
/Users/gfk/Documents/Arduino/libraries/Low-Power-master/LowPower.cpp:823:18: error: 'SLEEP_MODE_EXT_STANDBY' was not declared in this scope
    lowPowerBodOn(SLEEP_MODE_EXT_STANDBY);
                  ^
/Users/gfk/Documents/Arduino/libraries/Low-Power-master/LowPower.cpp:823:4: note: in expansion of macro 'lowPowerBodOn'
    lowPowerBodOn(SLEEP_MODE_EXT_STANDBY);
    ^
/Users/gfk/Documents/Arduino/libraries/Low-Power-master/LowPower.cpp:828:17: error: 'SLEEP_MODE_EXT_STANDBY' was not declared in this scope
   lowPowerBodOn(SLEEP_MODE_EXT_STANDBY);
                 ^
/Users/gfk/Documents/Arduino/libraries/Low-Power-master/LowPower.cpp:828:3: note: in expansion of macro 'lowPowerBodOn'
   lowPowerBodOn(SLEEP_MODE_EXT_STANDBY);
   ^
Erreur lors de la compilation.
koseelg commented 9 years ago

Yes, I have the same problem. But with a ATmega328 all works fine.

rocketscream commented 9 years ago

Guys,

This seems to be a problem in the 1.6.x branch of the Arduino where the compiler version has been upgraded. It compiles fine on the 1.0.x branch and works accordingly. Let me get into this, probably some changes to the sleep.h file has been made in the AVR library.

koseelg commented 9 years ago

Yes, you are right. I have moved the line:

|| defined(__AVR_ATmega168__) \

in the file "sleep.h". See the code block below. This code line was includet in the following code block, after the block I added it in sleep.h file, where SLEEP_MODE_EXT_STANDBY is not declared. There, in the original block, I have deleted this line. Now the compiling of the sketches works fine. Your suggestion was right. I think, all ATmega168 shall be in the same block, in same way as ATmega328 and together with them. I remember no reason against it.

...
#elif defined(__AVR_AT90USB1286__) \
|| defined(__AVR_AT90USB1287__) \
|| defined(__AVR_AT90USB646__) \
|| defined(__AVR_AT90USB647__) \
|| defined(__AVR_ATA6614Q__) \
|| defined(__AVR_ATmega128__) \
|| defined(__AVR_ATmega128A__) \
|| defined(__AVR_ATmega1280__) \
|| defined(__AVR_ATmega1281__) \
|| defined(__AVR_ATmega1284__) \
|| defined(__AVR_ATmega1284P__) \
|| defined(__AVR_ATmega128RFA1__) \
|| defined(__AVR_ATmega128RFR2__) \
|| defined(__AVR_ATmega1284RFR2__) \
|| defined(__AVR_ATmega16__) \
|| defined(__AVR_ATmega16A__) \
|| defined(__AVR_ATmega162__) \
|| defined(__AVR_ATmega164A__) \
|| defined(__AVR_ATmega164P__) \
|| defined(__AVR_ATmega164PA__) \
|| defined(__AVR_ATmega168__) \         // <<==
|| defined(__AVR_ATmega168A__) \
|| defined(__AVR_ATmega168P__) \
|| defined(__AVR_ATmega168PA__) \
|| defined(__AVR_ATmega168PB__) \
|| defined(__AVR_ATmega16HVA2__) \
|| defined(__AVR_ATmega16U4__) \
|| defined(__AVR_ATmega2560__) \
|| defined(__AVR_ATmega2561__) \
|| defined(__AVR_ATmega256RFR2__) \
|| defined(__AVR_ATmega2564RFR2__) \
|| defined(__AVR_ATmega32__) \
|| defined(__AVR_ATmega32A__) \
|| defined(__AVR_ATmega323__) \
|| defined(__AVR_ATmega324A__) \
|| defined(__AVR_ATmega324P__) \
|| defined(__AVR_ATmega324PA__) \
|| defined(__AVR_ATmega328__) \
|| defined(__AVR_ATmega328P__) \
|| defined(__AVR_ATmega32C1__) \
|| defined(__AVR_ATmega32U4__) \
|| defined(__AVR_ATmega32U6__) \
|| defined(__AVR_ATmega48A__) \
|| defined(__AVR_ATmega48PA__) \
|| defined(__AVR_ATmega48PB__) \
|| defined(__AVR_ATmega48P__) \
|| defined(__AVR_ATmega64__) \
|| defined(__AVR_ATmega64A__) \
|| defined(__AVR_ATmega640__) \
|| defined(__AVR_ATmega644__) \
|| defined(__AVR_ATmega644A__) \
|| defined(__AVR_ATmega644P__) \
|| defined(__AVR_ATmega644PA__) \
|| defined(__AVR_ATmega64C1__) \
|| defined(__AVR_ATmega64RFR2__) \
|| defined(__AVR_ATmega644RFR2__) \
|| defined(__AVR_ATmega8515__) \
|| defined(__AVR_ATmega8535__) \
|| defined(__AVR_ATmega88A__) \
|| defined(__AVR_ATmega88P__) \
|| defined(__AVR_ATmega88PA__) \
|| defined(__AVR_ATmega88PB__) 

    #define SLEEP_MODE_IDLE         (0)
    #define SLEEP_MODE_ADC          _BV(SM0)
    #define SLEEP_MODE_PWR_DOWN     _BV(SM1)
    #define SLEEP_MODE_PWR_SAVE     (_BV(SM0) | _BV(SM1))
    #define SLEEP_MODE_STANDBY      (_BV(SM1) | _BV(SM2))
    #define SLEEP_MODE_EXT_STANDBY  (_BV(SM0) | _BV(SM1) | _BV(SM2))

    #define set_sleep_mode(mode) \
    do { \
        _SLEEP_CONTROL_REG = ((_SLEEP_CONTROL_REG & ~(_BV(SM0) | _BV(SM1) | _BV(SM2))) | (mode)); \
    } while(0)
 ...
rocketscream commented 9 years ago

It seems that extended standby is not supported on the ATmega168. I must have been referring to the older avrlibc distribution instead of the datasheet while writing the library. Will do a patch to restrict the usage only to the chips having the extended standby mode support. For now, refrain from using that mode under ATmega168.

eried commented 8 years ago

Hello, I think this still has a bug, maybe the condition should look like this:

/*if (bod == BOD_OFF)   
{*/
    #if defined SLEEP_MODE_EXT_STANDBY
        lowPowerBodOff(SLEEP_MODE_EXT_STANDBY);
    #else
        lowPowerBodOn(SLEEP_MODE_PWR_DOWN);
    #endif
/*}
else    
{
    lowPowerBodOn(SLEEP_MODE_EXT_STANDBY);
}*/
Vojta01 commented 7 years ago

Hi, is there patch available? Or any other woraround? It is impossible to use this library with ATmega168, since only including the library in the sketch triggers that error.

rocketscream commented 7 years ago

You can add these to the LowPower.cpp file after line #106:

#if defined __AVR_ATmega168__
#ifndef SLEEP_MODE_EXT_STANDBY
#define SLEEP_MODE_EXT_STANDBY (0x07<<1)
#endif
#endif

Only the 168 variant doesn't have this declared but newer 168P & 168PA has them properly declared. Will add this patch in the next revision. Thank you for reporting.

Eheran1 commented 7 years ago

Same issue. Cant use the lib with my Atmega168. Line #106 is in the middle of comments: "* (c) SLEEP_60MS - 60 ms sleep"

Oh, there is also that typo in LowPower.h with a lowercase "s" in the 15ms Setting. All the other times have uppercase "S". Tho everything with a lowercase would be better since its second and that is always a lowercase "s". Same with the "milli" that is a uppercase "M" in LowPower.h which is mega and not milli. But I can see that u wouldnt want to change everything and keep it uppercase verywhere.

sqfield commented 7 years ago

This problem still exists. If you chose to compile LowPower.cpp and select pro8MHzatmega168 as the board (in platformIO, but the same happens in the Arduino IDE), you get this error:

In file included from C:\Users\User\Documents\Arduino\libraries\Low-Power\LowPower.cpp:32:0: ​​C:\Users\User\Documents\Arduino\libraries\Low-Power\LowPower.cpp: In member function 'void LowPowerClass::powerExtStandby(period_t, adc_t, bod_t, timer2_t)': C:\Users\User\Documents\Arduino\libraries\Low-Power\LowPower.cpp:980:18: error: 'SLEEP_MODE_EXT_STANDBY' was not declared in this scope lowPowerBodOn(SLEEP_MODE_EXT_STANDBY); ^ C:\Users\User\Documents\Arduino\libraries\Low-Power\LowPower.cpp:980:4: note: in expansion of macro 'lowPowerBodOn' lowPowerBodOn(SLEEP_MODE_EXT_STANDBY); ^ C:\Users\User\Documents\Arduino\libraries\Low-Power\LowPower.cpp:985:17: error: 'SLEEP_MODE_EXT_STANDBY' was not declared in this scope lowPowerBodOn(SLEEP_MODE_EXT_STANDBY); ^ C:\Users\User\Documents\Arduino\libraries\Low-Power\LowPower.cpp:985:3: note: in expansion of macro 'lowPowerBodOn' lowPowerBodOn(SLEEP_MODE_EXT_STANDBY); ^*** [.pioenvs\pro8MHzatmega168\lib\Low-Power\LowPower.o] Error 1

The method that has the error looks like this:

void LowPowerClass::powerExtStandby(period_t period, adc_t adc, bod_t bod, timer2_t timer2)
{
    // Temporary clock source variable 
    unsigned char clockSource = 0;

    #if !defined(__AVR_ATmega32U4__)
    if (timer2 == TIMER2_OFF)
    {
        if (TCCR2B & CS22) clockSource |= (1 << CS22);
        if (TCCR2B & CS21) clockSource |= (1 << CS21);
        if (TCCR2B & CS20) clockSource |= (1 << CS20);

        // Remove the clock source to shutdown Timer2
        TCCR2B &= ~(1 << CS22);
        TCCR2B &= ~(1 << CS21);
        TCCR2B &= ~(1 << CS20);
    }
    #endif

    if (adc == ADC_OFF) ADCSRA &= ~(1 << ADEN);

    if (period != SLEEP_FOREVER)
    {
        wdt_enable(period);
        WDTCSR |= (1 << WDIE);  
    }
    if (bod == BOD_OFF) 
    {
        #if defined __AVR_ATmega328P__
            lowPowerBodOff(SLEEP_MODE_EXT_STANDBY);
        #else
            lowPowerBodOn(SLEEP_MODE_EXT_STANDBY);                  /////// THIS LINE DOESN'T COMPILE ON AN ATmega168 ///////////
        #endif
    }
    else    
    {
        lowPowerBodOn(SLEEP_MODE_EXT_STANDBY);
    }

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

    #if !defined(__AVR_ATmega32U4__)
    if (timer2 == TIMER2_OFF)
    {
        if (clockSource & CS22) TCCR2B |= (1 << CS22);
        if (clockSource & CS21) TCCR2B |= (1 << CS21);
        if (clockSource & CS20) TCCR2B |= (1 << CS20);  
    }
    #endif
}

No matter what processor is used, the code is going to need to have SLEEP_MODE_EXT_STANDBY defined. But the vanilla ATmega168 does not define it. Only the ATmega168a and ATmega168p define it.

​My file includes ​:\Users\User.platformio\packages\framework-arduinoavr\cores\arduino/Arduino.h which on line 28 says:

include <avr/pgmspace.h>

​ ​This causes ​​c:\users\user.platformio\packages\toolchain-atmelavr\avr\include\avr\pgmspace.h to be included, whose line 90 says:

include <avr/io.h>

This causes ​c:\users\user.platformio\packages\toolchain-atmelavr\avr\include\avr\io.h to be included, whose line 324 says (with some surrounding lines for context):

...
#elif defined (__AVR_ATmega165A__)
#  include <avr/iom165a.h>
#elif defined (__AVR_ATmega165P__)
#  include <avr/iom165p.h>
#elif defined (__AVR_ATmega165PA__)
#  include <avr/iom165pa.h>
#elif defined (__AVR_ATmega168__)
#  include <avr/iom168.h>                                           // This is line 324
#elif defined (__AVR_ATmega168A__)
#  include <avr/iom168a.h>
#elif defined (__AVR_ATmega168P__)
#  include <avr/iom168p.h>
#elif defined (__AVR_ATmega168PA__)
#  include <avr/iom168pa.h>
#elif defined (__AVR_ATmega168PB__)​
​...​

​This causes ​​​c:\users\user.platformio\packages\toolchain-atmelavr\avr\include\avr\iom168.h to be included, whose last lines are:

​/* Signature */
#define SIGNATURE_0 0x1E
#define SIGNATURE_1 0x94
#define SIGNATURE_2 0x06

#define SLEEP_MODE_IDLE (0x00<<1)
#define SLEEP_MODE_ADC (0x01<<1)
#define SLEEP_MODE_PWR_DOWN (0x02<<1)
#define SLEEP_MODE_PWR_SAVE (0x03<<1)
#define SLEEP_MODE_STANDBY (0x06<<1)

#endif /* _AVR_IOM168_H_ */​

The file c:\users\user.platformio\packages\toolchain-atmelavr\avr\include\avr\iom168a.h consists of a comment and these two lines:

#include "iom168.h"
#define SLEEP_MODE_EXT_STANDBY (0x07<<1)

​The file c:\users\user.platformio\packages\toolchain-atmelavr\avr\include\avr\iom168a.h consists of a lot of new defines and ends with these lines:​

​/* Signature */
#define SIGNATURE_0 0x1E
#define SIGNATURE_1 0x94
#define SIGNATURE_2 0x0B

#define SLEEP_MODE_IDLE (0x00<<1)
#define SLEEP_MODE_ADC (0x01<<1)
#define SLEEP_MODE_PWR_DOWN (0x02<<1)
#define SLEEP_MODE_PWR_SAVE (0x03<<1)
#define SLEEP_MODE_STANDBY (0x06<<1)
#define SLEEP_MODE_EXT_STANDBY (0x07<<1)

#endif  /* _AVR_IOM168P_H_ */​

​THE FIX: ​The code in LowPower.cpp could be changed to test if SLEEP_MODE_EXT_STANDBY is defined, and "do the right thing", which may be to use a different sleep mode, such as SLEEP_MODE_STANDBY. Knowing whether this is the right thing to do would require reading more about the ATmega168 and its available low power modes. Alternatively, the board definition for the Pro Mini ATmega168 could define AVR_ATmega168P as the processor type, on the assumption that this is the newer version of the chip, and that all Pro Mini chips available now use the more recent version. However, this would still require LowPower.h to be modified, since it has a line that explicitly requires AVR_ATmega168: #if defined (AVR_ATmega328P) || defined (AVR_ATmega168) As my work-around, I added these lines to LowPower.h:

      #if defined (__AVR_ATmega168__)
        #define SLEEP_MODE_EXT_STANDBY SLEEP_MODE_STANDBY
      #endif

There are thus two issues. LowPower.cpp should be fixed to handle processors that do not have extended sleep modes. But the Arduino IDE and platformIO should probably also have board definitions for the newer ATmega168a and ATmega168p.