MCUdude / MiniCore

Arduino hardware package for ATmega8, ATmega48, ATmega88, ATmega168, ATmega328 and ATmega328PB
Other
1k stars 246 forks source link

Incorrect sleep management constants #317

Closed ElectricWater96 closed 1 month ago

ElectricWater96 commented 2 months ago

When attempting to use sleepMode() with an ATmega328P I found that the SLEEP_... macros are defined with incorrect values in the arduino.h file. The values defined range from 0 through 5, but they should be double what they are, due to a missing left bitshift that is required to align their bits with the SM[2:0] bits in the SMCR register. For example, sleepMode(SLEEP_POWER_DOWN) should pass a binary value of 0100 to SMCR, setting the sleep mode to "Power-down", but it passes 0010, which sets the sleep mode to "ADC Noise Reduction".

MCUdude commented 1 month ago

I haven't touched the code in years, and I didn't even write it. But it is correct, I double checked.

I do agree that the constants defined in Arduino.h doesn't allign with the MCRSR register:

/* Sleep management constants */
#define SLEEP_IDLE 0
#define SLEEP_ADC 1
#define SLEEP_POWER_DOWN 2
#define SLEEP_POWER_SAVE 3
#define SLEEP_STANDBY 4
#define SLEEP_EXTENDED_STANDBY 5

However, these macros aren't used directly. They are just used in a switch/case statement to pass

#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)

which are defined in the iom328p.h file, which are included automatically.

static inline void sleepMode(uint8_t P)
{
  #if defined(set_sleep_mode)
  switch (P)
  {
    case SLEEP_IDLE:
      #if defined SLEEP_MODE_IDLE
        set_sleep_mode(SLEEP_MODE_IDLE);
      #endif
      break;
    case SLEEP_ADC:
      #if defined SLEEP_MODE_ADC
        set_sleep_mode(SLEEP_MODE_ADC);
      #endif
      break;
    case SLEEP_POWER_DOWN:
      #if defined SLEEP_MODE_PWR_DOWN
        set_sleep_mode(SLEEP_MODE_PWR_DOWN);
      #endif
      break;
    case SLEEP_POWER_SAVE:
      #if defined SLEEP_MODE_PWR_SAVE
        set_sleep_mode(SLEEP_MODE_PWR_SAVE);
      #endif
      break;
    case SLEEP_STANDBY:
      #if defined SLEEP_MODE_STANDBY
        set_sleep_mode(SLEEP_MODE_STANDBY);
      #endif
      break;
    case SLEEP_EXTENDED_STANDBY:
      #if defined SLEEP_MODE_EXT_STANDBY
        set_sleep_mode(SLEEP_MODE_EXT_STANDBY);
      #endif
      break;
    default:
      break;
  }
  #endif
}