SpenceKonde / megaTinyCore

Arduino core for the tinyAVR 0/1/2-series - Ones's digit 2,4,5,7 (pincount, 8,14,20,24), tens digit 0, 1, or 2 (featureset), preceded by flash in kb. Library maintainers: porting help available!
Other
557 stars 145 forks source link

Cannot get External ISR to wake from sleep to work #264

Closed mrWheel closed 3 years ago

mrWheel commented 3 years ago

Hi,

Trying to write a simple sketch to attach a button to PA1, go to sleep after 10 seconds and then wait for button to be pressed to wake up again ... I cannot get it to work.


#include <avr/sleep.h>

#define PUSH_BUTTON PIN_PA1
#define BLUE_LED        PIN_PC1

uint32_t  waitForSleepTimer, blinkTimer;

void wakeUpISR()
{
  // This function will bring us back from sleep. 

  /* We detach the interrupt to stop it from 
   * continuously firing while the interrupt pin
   * is low.
   */
  detachInterrupt(digitalPinToInterrupt(PUSH_BUTTON));
  //detachInterrupt(0);

  // wait for buttom te be released ..
  while(!digitalRead(PUSH_BUTTON)) { ; }

  waitForSleepTimer = millis() + 10000;

}

void enterSleep(void)
{
  attachInterrupt(digitalPinToInterrupt(PUSH_BUTTON), wakeUpISR, LOW);
  delay(100);

  set_sleep_mode(SLEEP_MODE_IDLE);  // set the sleep type
  sleep_enable();                   // enable does not enter sleep mode

  sleep_cpu();                          // Sleep the device and wait for an interrupt to continue 

  /* The program will continue from here. */
  /* First thing to do is disable sleep. */
  sleep_disable(); 

}

void setup() 
{
  Serial.begin(115200);
  while(!Serial) { delay(1); /*wait a bit */ }
  Serial.println("\nAnd than it begins ....\n");

  pinMode(BLUE_LED,    OUTPUT); 
  pinMode(PUSH_BUTTON, INPUT);

  waitForSleepTimer = millis() + 10000;

}

void loop() 
{
  if (millis() > waitForSleepTimer )
  {
    waitForSleepTimer = millis() + 10000;
    Serial.print("Entering sleep mode ...");
    enterSleep();
    Serial.println(".. and we'r back!\n");
  }
  if (millis() > blinkTimer)
  {
    if (millis() > waitForSleepTimer)
          blinkTimer = millis() + 500;
    else  blinkTimer = millis() + 1000;
    digitalWrite(BLUE_LED, CHANGE);
  }

}

What am I doing wrong?

SpenceKonde commented 3 years ago

Not a clue, I have not worked with sleep on these parts, will look into this as part of #158

SpenceKonde commented 3 years ago

Though, one thing I would note is that your debug prints might not be giving you useful output there.

Add a Serial.flush() before calling enableSleep() to ensure that it waits until it's printed everything before going to sleep.

mrWheel commented 3 years ago

Added the Serial.flush() but no avail :-(

This is the output I get:

12:22:25.486 -> Entering sleep mode ...
12:22:25.588 -> .. and we'r back!
12:22:25.588 -> 
12:22:35.462 -> Entering sleep mode ...
12:22:35.563 -> .. and we'r back!
12:22:35.563 -> 
12:22:45.458 -> Entering sleep mode ...
12:22:45.559 -> .. and we'r back!
12:22:45.559 -> 
12:22:55.431 -> Entering sleep mode ...
12:22:55.531 -> .. and we'r back!

Hope you (or someone else) comes up with a solution ...

SpenceKonde commented 3 years ago

Uhmmm... just noticed what is likely the problem - You have set the button pin to INPUT, hence it would be expected to ick up ambient electrical noise from the environment and transition randomly. Set it to INPUT_PULLUP or use an external pullup.

There is a word for such a "floating" pin connected to the input pin of an IC when the device is designed to make sense of said electronic noise: Antenna

If you do have an external pullup, more debugging effort is required.... A start would be, maybe after the first conditional block, else { digitalWrite(BLUE_LED,!digitalRead(PUSH_BUTTON));} - hence when it thought the button was "pushed", the LED would go on (assuming the LED is wired to be active HIGH; remove the ! if active LOW). the idea being to determine whether the problem has anything to do with sleep mode at all. If it thinks the button is pressed when it's not, then sort that out before you try to make sleep work based on that... if it's flickering on and off, suspect pin is floating, etc.

mrWheel commented 3 years ago

@SpenceKonde Thanks again for your reply and suggestions. The switch has a pullup resister so that will most likely not be the problem. I think the problem is in the way I put my ATTiny3216 to sleep (SLEEP_MODE_IDLE). I changed that to SLEEP_MODE_STANDBY and made some other alterations to the sketch .. and now it works!

/************************************************
 * Demo program to test external interrupt 
 * and sleep mode!
 * 
 ************************************************
*/

#include <avr/sleep.h>

#define PUSH_BUTTON PIN_PA1
#define BLUE_LED    PIN_PC1

uint32_t      waitForSleepTimer, blinkTimer;

//-------------------------------------------------
//--- ISR seems to be a "pre-defined" name --------
//-------------------------------------------------
ISR(PORTA_PORT_vect)
{
  // This function will bring us back from sleep
  byte flags=PORTA.INTFLAGS;

  PORTA.INTFLAGS=flags; //clear flags
  if (flags&0x02) {
    // The program will continue from here.
    // First thing to do is disable sleep.
    sleep_disable(); 
  }
  else
  {
    return;
  }
  Serial.println("\n=> in ISR()");

  // wait for buttom te be released ..
  while(!digitalRead(PUSH_BUTTON)) { ; }

  waitForSleepTimer = millis() + 10000;

} // ISR()

//-------------------------------------------------
void enterSleep(void)
{
  set_sleep_mode(SLEEP_MODE_STANDBY);  // set the sleep type
  sleep_enable();                   // enable does not enter sleep mode

  // wait for buttom te be released ..
  while(!digitalRead(PUSH_BUTTON)) { ; }
  digitalWrite(BLUE_LED, LOW);

  sleep_cpu();                          // Sleep the device and wait for an interrupt to continue 

  /* The program will continue from here. */
  /* First thing to do is disable sleep. */
  sleep_disable(); 

} //  enterSleep()

//-------------------------------------------------
void setup() 
{
  Serial.begin(115200);
  while(!Serial) { delay(1); /*wait a bit */ }
  Serial.println("\nAnd than it begins ....\n");

  pinMode(BLUE_LED,    OUTPUT); 
  pinMode(PUSH_BUTTON, INPUT);
  //-------------------+-- pullup?
  //                   |    001 = trigger on both?
  //                   |    010 = trigger on rising?
  //                   |    011 = trigger on falling? 
  //                   |    100 = digital input buffer disabled entirely (what does that mean?)
  //                   Vvvv 101 = interrupt on LOW level
  PORTA.PIN1CTRL=0b00001101; //PULLUPEN=1, ISC=3 trigger on falling, ISC=2 trigger rising
  waitForSleepTimer = millis() + 20000;

} //  setup()

//-------------------------------------------------
void loop() 
{
  if ( millis() > waitForSleepTimer )
  {
    waitForSleepTimer = millis() + 10000;
    Serial.println("\nEntering sleep mode ...");
    Serial.flush();
    enterSleep();
    Serial.println(".. and we'r back!\n");
  }
  if (millis() > blinkTimer)
  {
    blinkTimer = millis() + 1000;
    digitalWrite(BLUE_LED, CHANGE);
    Serial.print(".");
  }

} // loop()

Thank you for your great work on the tiny processors (where would we be without you)!!

SpenceKonde commented 3 years ago

Oh... duh.... of course... In IDLE sleep mode, all interrupt sources will wake the part.....

Including the millis timekeeping one...

IDLE sleep mode is pretty useless, frankly... the useful ones are STANDBY and POWERDOWN. STANDBY is probably the good one for most applications (do you really care if you're using 1uA or 0.1uA, when the self-discharge rate of any real battery is at least an order of magnitude higher, probably 2?)

Also, initial version was missing the INTFLAGS bit, which you also figured out....

mrWheel commented 3 years ago

Ok, did not know that. Also SLEEP_MODE_PWR_DOWN does not work (with the fore-mentioned sketch)!

SpenceKonde commented 3 years ago

Also, don't do this:

  pinMode(PUSH_BUTTON, INPUT);
  //-------------------+-- pullup?
  //                   |    001 = trigger on both?
  //                   |    010 = trigger on rising?
  //                   |    011 = trigger on falling? 
  //                   |    100 = digital input buffer disabled entirely (what does that mean?)
  //                   Vvvv 101 = interrupt on LOW level
  PORTA.PIN1CTRL=0b00001101; //PULLUPEN=1, ISC=3 trigger on falling, ISC=2 trigger rising

Problems: if you're using pinMode, use INPUT_PULLUP. if you're manually setting the pullup, don't bother with pinMode. Actually - you don't ever need to set a pin to INPUT in setup - All pins start up as INPUT! I do that sometimes in code intended for consumption by novices, helps in terms of making the configuration explicit, but I always mention this in a comment right next to them. You enable the interrupt right there, when you're not sleeping and never disable it...

What I'd probably do is, leave it disabled in setup, enable it in enterSleep() (PORTA.PIN1CTRL|=PORT_ISC_LEVEL_gc) and disable it as soon as we wake (PORTA.PIN1CTRL&=PORT_ISC_gm). And do the waiting until it's not pressed anymore in enterSleep not the ISR (if you feel you need to at all)

SpenceKonde commented 3 years ago

Don't immediately see why powerdown would be different here, hmm.

SpenceKonde commented 3 years ago

Closing since problem is sorted now, powerdown issue on list for #158

mrWheel commented 3 years ago

@SpenceKonde I stay corrected! With last sketch SLEEP_MODE_PWR_DOWN does work!

Excuses for the confusion ..

pfoun commented 3 years ago

@SpenceKonde Thank you for your wonderful work on megaTinyCore and DxCore. I have been following them with interest.

After reading these comments I would pose this question about using an external interrupt to wake from sleep:

Is it true that only the LOW level interrupt trigger, applied to the interrupt pin, can bring the ATtiny-0-1 MCU out of SLEEP_MODE_PWR_DOWN, because all other external triggers require a clock, that is not available in this sleep mode, to be detected?

SpenceKonde commented 3 years ago

No, that is not correct. All pins can wake the chip on "both edges" (ie on change - like classic AVR PCINT) as well as low level. Additionally, there are a small number of "fully asynchronous" pins (pins 2 and 6 in each port), marked on the included pinout images, which are capable of waking the chip on not only change or low level, but also on rising or falling edge. I mention it in passing here: https://github.com/SpenceKonde/megaTinyCore/blob/master/megaavr/extras/PinInterrupts.md One thing that is worth noting however about the fully asynchronous pins, is that if used for a normal interrupt (not just waking rom sleep; not sure how wake from sleep impacts this phenomenon; suggest someone who has time test it and report back :-P ) the fact that they will respond to events shorter than a system clock cycle means that they are far more sensitive to noise. I tripped over this a while back.

SpenceKonde commented 3 years ago

Refer to table 16-3 in the datasheet (that's the number for the table on the 1614 and 3216 datasheets, at least)

SpenceKonde commented 3 years ago

The whole thing is described in greater detail in chapter 16 of the datasheet; the same principles also apply to the megaAVR 0-series, tinyAVR 2-series and AVR Dx-series.

pfoun commented 3 years ago

@SpenceKonde Thank you for pointing me in the right direction. Keep up the good work. You may not hear from me often, but I appreciate your efforts very much.