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

SLEEP_MODE_EXT_STANDBY not declared on ATMega168 #14

Closed
gfk opened this issue Jul 30, 2015 · 9 comments

Comments

@gfk
Copy link

@gfk gfk commented Jul 30, 2015

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

This comment has been minimized.

Copy link

@koseelg koseelg commented Sep 6, 2015

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

@rocketscream

This comment has been minimized.

Copy link
Owner

@rocketscream rocketscream commented Sep 6, 2015

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

This comment has been minimized.

Copy link

@koseelg koseelg commented Sep 6, 2015

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

This comment has been minimized.

Copy link
Owner

@rocketscream rocketscream commented Sep 7, 2015

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

This comment has been minimized.

Copy link

@eried eried commented Dec 18, 2015

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

This comment has been minimized.

Copy link

@Vojta01 Vojta01 commented Oct 7, 2016

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

This comment has been minimized.

Copy link
Owner

@rocketscream rocketscream commented Nov 13, 2016

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

This comment has been minimized.

Copy link

@Eheran1 Eheran1 commented Jan 2, 2017

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

This comment has been minimized.

Copy link

@sqfield sqfield commented Mar 26, 2017

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.

wingunder added a commit to wingunder/Low-Power that referenced this issue Mar 17, 2018
This patch attempts to achieve the following:

   * Supplies a Makefile for building a library per supported mcu.
   * Works around issue rocketscream#14 in a single line inside the Makefile.
   * Fixes some warnings of unused variables for certain mcus
   * Removes the unnecessary inclusion of <Arduino.h>.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.