Missed Interrupt Problem with Attiny85

Question

I'm trying to use an Attiny85 (digispark) to wake another controller from sleep (an ESP8266).

The Attiny is connected to an IR receiver which has an active low output. Basically I have connected an output from the Attiny to the Reset pin on the ESP8266, so when an IR signal is received, it resets the ESP8266 but then ignores future IR until it gets a signal from the ESP that it is going to sleep again.

The Attiny should also sleep but wake on pin interrupts on either pin 0 (IR in) or pin 2 (resetEnable from the ESP to signify that the ESP is going to sleep and will need to be woken).

I've got code that works sometimes but seems to mostly (but not always) miss the Reset Enable pulse that the ESP sends to notify before it goes to sleep. I have verified with the oscilloscope that the ESP is sending a 3ms High pulse every time it goes to sleep, but the Attiny is missing most of those pulses (ie, the resetEnable flag remains low).

The way I imagine it should work is that the interrupt routine determines which pin has triggered it, then if it's the Reset Enable pin (pin 2) going high, it sets a flag, so the next low on the IR pin will send an active low reset pulse to the ESP RST pin connected to pin 3 of the Attiny).

The first IR pulse received after boot of the Attiny does always reset the ESP as it should, since the resetEnable flag is set true in the setup(). This makes me think that most of my code is working, except the interrupt routine to set the resetEnable and resetEsp flags.

Here's my code, which was mostly pieced together from bits and pieces stolen from the web:

#include <avr/sleep.h>

#ifndef cbi
#define cbi(sfr, bit) (_SFR_BYTE(sfr) &= ~_BV(bit))
#endif
#ifndef sbi
#define sbi(sfr, bit) (_SFR_BYTE(sfr) |= _BV(bit))
#endif

int pinIR = 0;
int pinLed = 1;
int pinRSTen = 2;
int pinRST = 3;

volatile bool resetEsp = false;
volatile bool resetEnabled = true;

void setup(){
  pinMode(pinIR,INPUT);
  pinMode(pinLed,OUTPUT);
  pinMode(pinRSTen,INPUT);
  pinMode(pinRST,OUTPUT);
  digitalWrite(pinRST,HIGH); // let the Esp boot

  flash(4,500); // flash the led to show attiny has started

  sbi(GIMSK,PCIE); // Turn on Pin Change interrupt
  sbi(PCMSK,PCINT0); // Which pins are affected by the interrupt
  sbi(PCMSK,PCINT2);
  sei();
}

void loop(){
  system_sleep();
  if (resetEsp) {
    digitalWrite(pinRST, LOW); // reset the ESP
    delayMicroseconds(240);
    digitalWrite(pinRST, HIGH); // let the ESP Boot    
    flash(10, 50); // flash the led fast to show we're waking the ESP
    resetEsp = false; // clear the flags
    resetEnabled = false;
  } else if (resetEnabled) {
    flash(10, 500);  // mostly never get here
  } else {
    flash(2, 500);   // these are the flashes I see most of the time
  }


}

// From http://interface.khm.de/index.php/lab/experiments/sleep_watchdog_battery/
void system_sleep() {
  cbi(ADCSRA,ADEN); // Switch Analog to Digital converter OFF
  set_sleep_mode(SLEEP_MODE_PWR_DOWN); // Set sleep mode
  sleep_mode(); // System sleeps here
  //sbi(ADCSRA,ADEN);  // Switch Analog to Digital converter ON
}

ISR(PCINT0_vect) {
   if (digitalRead(pinRSTen) == HIGH) {
     resetEnabled = true;  // set the flag so the next IR pulse will reset the ESP
   }
   if (resetEnabled && (digitalRead(pinIR) == LOW)) {
     resetEsp = true;  // reset the ESP
   }
}

void flash(int num, int wait) {
  for (int i=0;i<num;i++) {
    digitalWrite(pinLed, HIGH);
    delay(50);
    digitalWrite(pinLed, LOW);
    delay(wait);
  }
}

My code does trigger reliably and give the two flashes every time I send IR to it. Just the resetEnable flag doesn't seem to get set reliably when the ESP sends the 3ms high pulse on pin 2.

After I get the interrupt code working reliably, I would also like to get the Attiny to ignore IR pulses less than 320us, as the IR detector seems to glitch low every now and then for less than 320us, and I don't want the ESP8266 to be woken in that case.

Edit:

<meta> I never thought something that seems so simple could turn into something so complicated! Massive thanks to jms and JimmyB for your help, your comments were like gold to me, and I’ve learned a lot. It's very difficult to debug an Attiny running interrupt code that I didn’t really understand (the first interrupt code I’ve ever used), with only a single led with which to communicate, and on top of that, while it is flashing the single led, it's actually missing interrupts! </meta>

Now I've said that, here's the bad news... It's still not working. :-(

I've used the code that both jms and JimmyB have given, they were almost identical.

Unfortunately the Attiny still doesn't sleep properly though.

When the resetEnable pulse comes in on PB2, it doesn't set the resetEnable flag I think. Using the code below it just flashes the led once every time, just the same as when IR comes in on PB0. The only time the led is solidly lit (to show resetEnable is true), is when the Attiny is first booted. It will reset the ESP as it should the first time some IR comes in, but never again as resetEnable isn't getting set again I believe.

If I comment out the sleep_cpu(); line, then everything works perfectly though (but obviously the Attiny isn't sleeping). I can't work out why it doesn't work with the sleep in there.

Here's the exact code I'm using now:

#include <avr/sleep.h>

#ifndef cbi
#define cbi(sfr, bit) (_SFR_BYTE(sfr) &= ~_BV(bit))
#endif
#ifndef sbi
#define sbi(sfr, bit) (_SFR_BYTE(sfr) |= _BV(bit))
#endif

int pinIR = 0;
int pinLed = 1;
int pinRSTen = 2;
int pinRST = 3;

volatile bool resetEsp = false;
volatile bool resetEnabled = true;

void setup() {
  pinMode(pinIR, INPUT);
  pinMode(pinLed, OUTPUT);
  pinMode(pinRSTen, INPUT);
  pinMode(pinRST, INPUT); // set as input so ESP can auto-reset itself over USB serial
  //digitalWrite(pinRST, HIGH); // let the Esp boot

  flash(4, 500);
  sbi(GIMSK, PCIE); // Turn on Pin Change interrupt
  sbi(PCMSK, PCINT0); // Which pins are affected by the interrupt
  sbi(PCMSK, PCINT2);
  sei();
}

void loop() {
  set_sleep_mode(SLEEP_MODE_PWR_DOWN); // Set sleep mode
  cli(); // Disable interrupts to avoid race condition

  if ( !resetEsp && !resetEnabled ) {
    // Only go to sleep if we have nothing to do right now.
    // Safe(*) code from the example in avr-libc:
    sleep_enable();
    sei();
    sleep_cpu(); // if this line is commented out, it all works perfectly.
    sleep_disable();
  } else {
    sei(); // Can go on processing IRQs.
  }

  if (resetEsp) {
    cli();
    resetEsp = false;
    resetEnabled = false;
    sei();
    pinMode(pinRST, OUTPUT); // needed to do this so auto-reset works when programming the ESP
                             // over serial
    digitalWrite(pinRST, LOW); // reset the ESP
    delayMicroseconds(240);
    digitalWrite(pinRST, HIGH); // let the ESP Boot
    pinMode(pinRST, INPUT);
    flash(10, 50); // show that we're resetting the ESP
  } else if (resetEnabled) {
    digitalWrite(pinLed, HIGH); // show that resets are enabled
    //flash(4, 100);
  } else {
    flash(1, 50);
  }
}

ISR(PCINT0_vect) {
  if (resetEnabled && !(PINB & (1 << PB0))) {
    resetEsp = true;
  }
  if ((PINB & (1 << PB2))) {
    resetEnabled = true;
  }
}

void flash(int num, int wait) {
  for (int i = 0; i < num; i++) {
    digitalWrite(pinLed, HIGH);
    delay(50);
    digitalWrite(pinLed, LOW);
    delay(wait);
  }
}

Also, jms, would you mind showing me exactly how to declare and use a single state byte? I tried googling for it but couldn't find the right keywords to come up with something useful. I don't think it's an enum, maybe a struct? Like you said, volatile uint8_t espIrRstState with three possible values IR_RST_DISABLED, IR_RST_ENABLED, IR_RST_TRIGGERED.

This thing is doing my head in!! I would love to get it working! Please help me so I can move on with my life!

Answer 1

You go to sleep unconditionally in every iteration of loop(). That's not what you want. The way your code is now, you'll miss every wake up event that happens while your normal program code runs.

Your code spends most of the time delaying inside flash(). And the controller goes to sleep after flash() is done. Always. This means that whenever a wake up event occurrs while the LED is flashing, it will not cause a wake up. That's just because the interrupt occurred and was serviced before you went to sleep.

Have a look at the example in the avr-libc docs here.

Your code should look like this:

void loop(){

  cli(); // Disable interrupts to avoid race condition.

  if ( !resetEsp ) {
    // Only go to sleep if we have nothing to do right now.
    // Safe(*) code from the example in avr-libc:
    sleep_enable();
    sei();
    sleep_cpu();
    sleep_disable();    
  } else {
    sei(); // Can go on processing IRQs.
  }

  if (resetEsp) {

    cli();

    resetEsp = false; // clear the flags
    resetEnabled = false;

    sei();

    digitalWrite(pinRST, LOW); // reset the ESP
    delayMicroseconds(240);
    digitalWrite(pinRST, HIGH); // let the ESP Boot    
    flash(10, 50); // flash the led fast to show we're waking the ESP

  } else if (resetEnabled) {
    flash(10, 500);  // mostly never get here
  } else {
    flash(2, 500);   // these are the flashes I see most of the time
  }

}

The point here is that we disable all interrupts while we check to see if we want to go to sleep. This way, we make sure that no interrupt can happen after we checked but before we actually sleep. Is is crucial, however, that we enable interrupts again just before going to sleep, or we will never be woken up again.

Also, @jms is right in stating that you have another potential race codition when resetting your flags, so include some cli/sei there too.

set_sleep_mode(SLEEP_MODE_PWR_DOWN); can go into setup(). And if you're not controlling spacecraft the same goes for sleep_enable();, while omitting sleep_disable(); completely.

---

(*) The code labelled "safe" above is not absolutely bullet-proof because, in theory, during optimization the compiler could decide to reorder some instructions so that sei() and sleep_cpu() could end up not being in direct sequence. To be absolutely safe, check the generated assembler code (gcc -save-temps), or just write the reqired instructions as inline assembler yourself. (Can be as simple as asm volatile (" sei \r\n sleep \r\n " :::);.)

Answer 2

I'm pretty sure that this is a classic concurrency problem which arises from you clearing the two flags after resetting the ESP.

ATtiny boots, initializes, resetEnabled set 
Main loop starts, ATtiny goes asleep.
IR pulse arrives
ATtiny wakes up, ISR executes, resetEsp set
Main loop code resets ESP                   ==>      ESP wakes up
Main loop code starts flashing LED                   ESP does stuff
ISR executes, doesn't do anything           <==      ESP enables IR reset
Main loop code finishes flashing LED                 ESP goes asleep
ResetEsp and resetEnabled cleared               
ATtiny goes asleep, resetEnabled is never set again  
IR pulse arrives, ISR executes, ESP reset skipped 

The ATtiny might sometimes work as intended if the ESP takes long enough to do its thing before triggering the interrupt.

The fix is simple, clear the flags before resetting the ESP.

void loop()
{
    system_sleep();
    if(resetEsp)
    {
        cli(); //prevent an interrupt from firing while 
               //..clearing the flags and messing them up
        resetEsp = false; // clear the flags
        resetEnabled = false;
        sei();
        digitalWrite(pinRST, LOW); // reset the ESP
        delayMicroseconds(240);
        digitalWrite(pinRST, HIGH); // let the ESP Boot    
        flash(10, 50); // flash the led fast to show we're waking the ESP
    }
    else
        flash(2, 500);   //reset by IR not enabled 
}

---

EDIT:

As @JimmyB discovered, there is a second oversight in both the code of the original question and my original answer. If ESP IR resets are enabled and an IR pulse arrives while the ATtiny is awake, the ATtiny will go to sleep without resetting the AVR and it will take a second, superfluous IR pulse to actually trigger a reset of the ESP.

To fix this, the resetEsp flag should be checked again (while interrupts are disabled) just before falling asleep, and the very last instruction before the sleep instruction should re-enable the interrupts. This way if an IR pulse arrives while the resetEsp flag is being checked again, the sleep mode transition will be canceled by the pending interrupt.

This code should fix that issue as well.

void loop()
{
    set_sleep_mode(SLEEP_MODE_PWR_DOWN);
    cli();
    if(!resetEsp)
    {
        sleep_enable();         //Set the sleep enable bit, allowing the sleep instruction to work.
        sei();                  //Enable interrupts one cycle before falling asleep. If an 
        sleep_cpu();            //..interrupt is pending, the ISR will abort the sleeping process
        sleep_disable();        //Clear the sleep enable bit
    }
    else
        sei();

    if(resetEsp)
    {
        cli();                  //prevent an interrupt from messing with the flags while 
                                //..main loop code is in the process of clearing them
        resetEsp = false;       // clear the flags
        resetEnabled = false;
        sei();
        digitalWrite(pinRST, LOW); // reset the ESP
        delayMicroseconds(240);
        digitalWrite(pinRST, HIGH); // let the ESP Boot    
        flash(10, 50);          // signal that the ESP has been reset
    }
    else
    {
        if(resetEnabled)
            flash(10, 500);     //signal that the ESP has enabled IR resets
        else
            flash(2, 500);      //signal that the IR pulse was ignored
    }
}  

---

EDIT 2:

Here's the full code of an implementation based on a single state variable. I also modified how the ESP reset line is driven: Now the reset line is normally left to float undriven, and brought briefly to ground when a reset is commanded.

#include <avr/sleep.h>
#include <cstdint.h>

//IO pin bit definitions
#define IR_IN               0
#define LED_OUT             1
#define RST_ENABLE_IN       2
#define RST_OUT             3

//State definitions
#define IR_RST_DISABLED     0
#define IR_RST_WAITING      1
#define IR_RST_TRIGGERED    2

//State variable
volatile uint8_t state;


void setup()
{
    DDRB = (0<<IR_IN) | 
           (1<<LED_OUT) | 
           (0<<RST_ENABLE_IN) | 
           (0<<RST_OUT); //set RST_OUT as input. This lets the reset line float undriven.

    PORTB |= (0<<RST_OUT);

    flashLed(4,500); // flash the led to show attiny has started
    
    state = IR_RST_WAITING; //start up with the IR reset enabled
    
    //enable IR_IN and RST_ENABLE_IN as pin change interrupt sources.
    PCMSK |= (1<<PCINT0) | (1<<PCINT2); 
    GIMSK |= (1<<PCIE);
    sei();
}

void loop()
{    
    switch(state)
    {
    case IR_RST_DISABLED:
    default:
        flashLed(2, 500);   //Signal that an IR pulse was received but ignored.
        break;
        
    case IR_RST_WAITING:
        flashLed(10, 500);  //Signal that IR Resets are now enabled
        break;
        
    case IR_RST_TRIGGERED:
        //Clear the state. No risk of race conditions, this takes just a single cycle. 
        state = IR_RST_DISABLED; 
        
        //Reboot the ESP
        DDRB |= (1<<RST_OUT);  //set RST_OUT as output. This drives the pin low, as the corresponding bit in PORTB is 0
        delayMicroseconds(240);
        DDRB &= ~(1<<RST_OUT); //set RST_OUT as input. This lets the reset line float undriven.

        flashLed(10, 50);   //Signal that the ESP was reset.
        break;
    }
    
    set_sleep_mode(SLEEP_MODE_PWR_DOWN);
    cli();               //disable interrupts so that the state won't change while we are falling asleep
    if(state != IR_RST_TRIGGERED)
    {
        sleep_enable();  //Set the sleep enable bit, allowing the sleep instruction to work.
        sei();           //Enable interrupts one cycle before falling asleep. If an interrupt source occurred after
        sleep_cpu();     //..disabling interrupts, the ISR will now trigger and abort the sleeping process
        sleep_disable(); //Clear the sleep enable bit, locking sleep functionality from accidential use
    }
    sei();               //enable interrupts back again and resume normal code execution.
}

ISR(PCINT0_vect)
{
    if(state == IR_RST_DISABLED)
    {
        if(PINB & (1 << RST_ENABLE_IN))
            state = IR_RST_WAITING; 
    }
    else
    {
        if(PINB & (1 << IR_IN))
            state = IR_RST_TRIGGERED; 
    }
}

void flashLed(int flashCount, int offTime)
{
    while (flashCount-- > 0) {
        PORTB |= (1<<LED_OUT);  //LED output high
        delay(50);
        PORTB &= ~(1<<LED_OUT); //LED output low
        delay(offTime);
    }
}

If you don't want the LED flashing functionality anymore, you can just replace the whole switch statement with a simple if:

if(state == IR_RST_TRIGGERED)
{
    //Clear the state. No risk of race conditions, this takes just a single cycle. 
    state = IR_RST_DISABLED; 
    
    //Reboot the ESP
    DDRB |= (1<<RST_OUT);  //set RST_OUT as output. This drives the pin low, as the corresponding bit in PORTB is 0
    delayMicroseconds(240);
    DDRB &= ~(1<<RST_OUT); //set RST_OUT as input. This lets the reset line float undriven.
}  

Removing the flashing functionality would also allow you to implement simple debugging with the LED: Toggle the LED state when a certain piece of code runs.
For example, you could toggle the LED every time the RST_ENABLE_IN pin ever changes state, allowing you determine if the reset enable line ever triggers the interrupt in the first place

ISR(PCINT0_vect)
{
    if(PINB & (1 << RST_ENABLE_IN))
        PORTB ^= (1 << LED_OUT); //XOR the LED output with itself, toggling its state.
    ...