Hey there! First time posting.
Firstly: been gathering info on these forums for months now without ever signing in. Thanks to y'all!
Project: build a (RF) remote for turning on/off various things at home
Idea: have various buttons connected to Pin Change Interrupts (PCIs), while the atmega328 sleeps most of the time and wakes up only when a button is pressed (and sending whatever RF message at that point), then goes back to sleep
Ressources: I've been reading a lot about sleep, timers and interrupts on the excellent Gammon Forum (google it if you don't know it, I find it very helpful)
Problem Summary: for brown-out detection to be disabling (BODD) during sleep (which aims at saving power), sleep_cpu() must be executed right after the lines corresponding to BODD. However it is good practice to re-enable interrupts right before sleep_cpu(), so that no interrupt can prevent you from going to sleep (which could cause erroneous behavior). The solution is known when using the "deluxe" interrupts (with dedicated vectors) of the atmega328. Indeed, writing:
MCUCR = bit (BODS) | bit (BODSE);
MCUCR = bit (BODS);
interrupts (); // one cycle
sleep_cpu (); // one cycle
works like a charm: the first two lines do the BODD, the third line re-enables interrupts but guarantees that sleep_cpu() is executed (because that's how it works: interrupts() gives you a margin of one clock cycle (or is it more?) to execute the sleep_cpu()).
However, I'm using the PCIs and hence my re-enabling of the interrupts reads not like interrupts() (which does not work of course) but like:
PCICR |= bit (PCIE0) | bit (PCIE1) | bit (PCIE2);
As you can see I wrote it all in one line, hoping that if I replaced interrupts() by the above line, I would manage what I want. But it does not work. Writing:
MCUCR = bit (BODS) | bit (BODSE);
MCUCR = bit (BODS);
PCICR |= bit (PCIE0) | bit (PCIE1) | bit (PCIE2);
sleep_cpu (); // one cycle
does go to sleep with interrupts enabled but it fails to catch the BODD (I checked this by measuring the current). Writing the original four lines (see first four lines of code above) but trying to make use of the PCIs does go to sleep ... but of course the PCIs are not enabled and the processor cannot be woken up.
Full working code which I'd like to improve:
#include <avr/sleep.h>
const byte LED0 = 8;
const byte LED1 = A0;
const byte LED2 = 0;
const byte I0 = 12;
const byte I1 = A5;
const byte I2 = 7;
ISR (PCINT0_vect)
{
// handle pin change interrupt for D8 to D13 here
if (digitalRead(I0)==LOW)
{
pinMode (LED0, OUTPUT);
digitalWrite (LED0, !digitalRead(LED0));
}
}
ISR (PCINT1_vect)
{
// handle pin change interrupt for A0 to A5 here
if (digitalRead(I1)==LOW)
{
pinMode (LED1, OUTPUT);
digitalWrite (LED1, !digitalRead(LED1));
}
}
ISR (PCINT2_vect)
{
// handle pin change interrupt for D0 to D7 here
if (digitalRead(I2)==LOW)
{
pinMode (LED2, OUTPUT);
digitalWrite (LED2, !digitalRead(LED2));
}
}
void setup ()
{
// pin change interrupts (example for D12)
PCMSK0 |= bit (PCINT4); // want pin 12
PCIFR |= bit (PCIF0); // clear any outstanding interrupts
PCICR |= bit (PCIE0); // enable pin change interrupts for D8 to D13
// pin change interrupts (example for A5)
PCMSK1 |= bit (PCINT13); // want pin A5
PCIFR |= bit (PCIF1); // clear any outstanding interrupts
PCICR |= bit (PCIE1); // enable pin change interrupts for A0 to A5
// pin change interrupts (example for D7)
PCMSK2 |= bit (PCINT23); // want pin 7
PCIFR |= bit (PCIF2); // clear any outstanding interrupts
PCICR |= bit (PCIE2); // enable pin change interrupts for D0 to D7
pinMode (I0, INPUT); // interrupt pins should be set as inputs
pinMode (I1, INPUT); // interrupt pins should be set as inputs
pinMode (I2, INPUT); // interrupt pins should be set as inputs
digitalWrite (I0, HIGH); // enable pull-up
digitalWrite (I1, HIGH); // enable pull-up
digitalWrite (I2, HIGH); // enable pull-up
pinMode (LED0, OUTPUT);
pinMode (LED1, OUTPUT);
pinMode (LED2, OUTPUT);
digitalWrite (LED0, HIGH);
digitalWrite (LED1, HIGH);
digitalWrite (LED2, HIGH);
}
// Table of pins -> pin change names / masks
//D0 PCINT16 (PCMSK2 / PCIF2 / PCIE2)
//D1 PCINT17 (PCMSK2 / PCIF2 / PCIE2)
//D2 PCINT18 (PCMSK2 / PCIF2 / PCIE2)
//D3 PCINT19 (PCMSK2 / PCIF2 / PCIE2)
//D4 PCINT20 (PCMSK2 / PCIF2 / PCIE2)
//D5 PCINT21 (PCMSK2 / PCIF2 / PCIE2)
//D6 PCINT22 (PCMSK2 / PCIF2 / PCIE2)
//D7 PCINT23 (PCMSK2 / PCIF2 / PCIE2)
//D8 PCINT0 (PCMSK0 / PCIF0 / PCIE0)
//D9 PCINT1 (PCMSK0 / PCIF0 / PCIE0)
//D10 PCINT2 (PCMSK0 / PCIF0 / PCIE0)
//D11 PCINT3 (PCMSK0 / PCIF0 / PCIE0)
//D12 PCINT4 (PCMSK0 / PCIF0 / PCIE0)
//D13 PCINT5 (PCMSK0 / PCIF0 / PCIE0)
//A0 PCINT8 (PCMSK1 / PCIF1 / PCIE1)
//A1 PCINT9 (PCMSK1 / PCIF1 / PCIE1)
//A2 PCINT10 (PCMSK1 / PCIF1 / PCIE1)
//A3 PCINT11 (PCMSK1 / PCIF1 / PCIE1)
//A4 PCINT12 (PCMSK1 / PCIF1 / PCIE1)
//A5 PCINT13 (PCMSK1 / PCIF1 / PCIE1)
void loop ()
{
// This is where code resumes from after some ISR has been called from sleep
// Firstly, disable all interrupts so that we can safely and reliably do stuff
PCICR &= ~(1 << PCIE0); // disable pin change interrupts for D8 to D13
PCICR &= ~(1 << PCIE1); // disable pin change interrupts for A0 to A5
PCICR &= ~(1 << PCIE2); // disable pin change interrupts for D0 to D7
// Then, cancel sleep as a precaution
sleep_disable();
// Prepare sleep setup (always turn off ADC first, otherwise it may stay frozen in an active state)
ADCSRA = 0; // disabling ADC saves 340 uA
set_sleep_mode (SLEEP_MODE_PWR_DOWN);
sleep_enable();
// Just to be sure, specify again the interrupts like in void setup
// pin change interrupts (example for D12)
PCMSK0 |= bit (PCINT4); // want pin 12
PCIFR |= bit (PCIF0); // clear any outstanding interrupts
// pin change interrupts (example for A5)
PCMSK1 |= bit (PCINT13); // want pin A5
PCIFR |= bit (PCIF1); // clear any outstanding interrupts
// pin change interrupts (example for D7)
PCMSK2 |= bit (PCINT23); // want pin 7
PCIFR |= bit (PCIF2); // clear any outstanding interrupts
// Re-enable interrupts as close to the sleep as possible (to avoid problems)
PCICR |= bit (PCIE0) | bit (PCIE1) | bit (PCIE2);
// enabled pin change interrupts for D8 to D13
// enabled pin change interrupts for A0 to A5
// enabled pin change interrupts for D0 to D7
MCUCR = bit (BODS) | bit (BODSE);
MCUCR = bit (BODS); // The BODS bit is automatically cleared after three clock cycles
sleep_cpu (); // one cycle
}
Question: As you can see the re-enabling of the PCIs comes before the BODD, because I can't seem to manage otherwise. But what I would want is to have a code as secure as possible where NOTHING takes place between the re-enabling of the PCIs and the sleep_cpu() line -- not even BODD.
Fellas, time to shine! 
Thanks a bunch in advance!