Consider the Arduino code at the end of this post for full reference that makes LED13 blink in a 0.5Hz rhythm driven by timers. It is just a proof of concept, I know it can be improved for various aspects, but lets focus on the following issue. Although in the PoC below I only use bit0, the goal of my PoC is to find a robust way to access the full 32-bit integer outside the ISR. The source sniplet discussed is this:
void loop() {
digitalWrite( led , secondCounter & 1 );
}
The ISR increases the secondsCounter
every second and the led follows bit0 of that counter, making it blink.
The resulting disassembly listing for the loop looks like this:
000002ae <loop>:
2ae: 60 91 00 02 lds r22, 0x0200 ; Read secondCounter from memory
2b2: 70 91 01 02 lds r23, 0x0201 ; 4 bytes, 32-vit integer
2b6: 80 91 02 02 lds r24, 0x0202
2ba: 90 91 03 02 lds r25, 0x0203
2be: 61 70 andi r22, 0x01 ; 1
2c0: 77 27 eor r23, r23
2c2: 88 27 eor r24, r24
2c4: 99 27 eor r25, r25
2c6: 8d e0 ldi r24, 0x0D ; 13
2c8: 0c 94 0a 02 jmp 0x414 ; 0x414 <digitalWrite>
Notice that in the background Timer event interrupts are fired all the time and there is a possibility that an interrupt is serviced while r22-r25 are being read from memory. This may result in a corrupted integer value being read from memory, but it can easily be fixed by disabling interrupts while the variable is read from memory:
void loop() {
noInterrupts(); // Prevent secondCounter from being updated in ISR while being read from memory in main loop.
digitalWrite( led , secondCounter & 1 );
interrupts(); // Enable interrupts
}
Which results in:
000002ae <loop>:
2ae: f8 94 cli ; Disable interrupts
2b0: 60 91 00 02 lds r22, 0x0200
2b4: 70 91 01 02 lds r23, 0x0201
2b8: 80 91 02 02 lds r24, 0x0202
2bc: 90 91 03 02 lds r25, 0x0203
2c0: 61 70 andi r22, 0x01 ; 1
2c2: 77 27 eor r23, r23
2c4: 88 27 eor r24, r24
2c6: 99 27 eor r25, r25
2c8: 8d e0 ldi r24, 0x0D ; 13
2ca: 0e 94 0d 02 call 0x41a ; 0x41a <digitalWrite>
2ce: 78 94 sei ; Enable interrupts
2d0: 08 95 ret
But now the entire digitalWrite routine (which takes a relatively long time) is executed while interrupts are disabled and pending interrupts have to wait a long time before being serviced.
A solution seems to be the use of a dummy variable:
void loop() {
noInterrupts(); // Prevent secondCounter from being updated in ISR while being read from memory in main loop.
uint32_t seconds = secondCounter;
interrupts(); // Enable interrupts
digitalWrite( led , seconds & 1 );
}
Which results in nice and clean assembly:
000002ae <loop>:
2ae: f8 94 cli ; Disable interrupts
2b0: 60 91 00 02 lds r22, 0x0200 ; Read variable from memory
2b4: 70 91 01 02 lds r23, 0x0201
2b8: 80 91 02 02 lds r24, 0x0202
2bc: 90 91 03 02 lds r25, 0x0203
2c0: 78 94 sei ; Enable interrupts
2c2: 61 70 andi r22, 0x01 ; Do other stuff
2c4: 77 27 eor r23, r23
2c6: 88 27 eor r24, r24
2c8: 99 27 eor r25, r25
2ca: 8d e0 ldi r24, 0x0D ; 13
2cc: 0c 94 0c 02 jmp 0x418 ; 0x418 <digitalWrite>
Question: The use of an extra temporary variable seems cumbersome, is there a smarter and tidier solution to this?
Failed attempt: I tried creating a function like:
uint32_t atomic_int32( uint32 var ) {
noInterrupts();
uint32_t tmp = var;
interrupts();
return tmp;
}
Combined with:
digitalWrite( led , atomic_int32( secondCounter ) & 1 );
But the compiler 'optimizes' the reading of the variable from memory outside the cli
and sei
instructions. Actually there is nothing in between those instructions at all:
000002ae <loop>:
2ae: 80 91 00 02 lds r24, 0x0200 ; Read variable from memory
2b2: 90 91 01 02 lds r25, 0x0201
2b6: a0 91 02 02 lds r26, 0x0202
2ba: b0 91 03 02 lds r27, 0x0203
2be: f8 94 cli ; Disable interrupts
2c0: 78 94 sei ; Enable interrupts
2c2: 60 e0 ldi r22, 0x00 ; 0
2c4: 8d e0 ldi r24, 0x0D ; 13
2c6: 0c 94 09 02 jmp 0x412 ; 0x412 <digitalWrite>
Full code
/*
* (c) J.P. Hendrix
*
* http://blog.linformatronics.nl/213/electronics/timed-1-millisecond-interrupt-routine-for-arduino
*
* Timed interrupt using Timer2
* ISR is called every 1ms
*
* The LED on pin13 will blink in a 0.5Hz rhythm
*/
#define _BC(bit) ( 0 << ( bit ) )
#define _BS(bit) ( 1 << ( bit ) )
// Pin 13 has an LED connected on most Arduino boards.
const uint8_t led = 13;
volatile uint16_t millisecondCounter = 0;
volatile uint32_t secondCounter = 0;
void setup() {
// initialize the digital pin as an output.
pinMode( led , OUTPUT);
// Set up Timer2 for 1 ms interrupts
// - Arduino is clocked at 16MHz
// - The timer is clocked through a /128 prescaler, thus 125kHz (TCCR2)
// - The interrupt is generated when the counter hits 125 (OCR2A)
// at which moment the Timer is reset to 0, resulting in 1kHz intervals
TCCR2A = _BC( COM2A1 ) | _BC( COM2A0 ) | // Normal port operation, OC2A disconnected
_BC( COM2B1 ) | _BC( COM2B0 ) | // Normal port operation, OC2B disconnected
_BS( WGM21 ) | _BC( WGM20 ); // Clear timer on compare match
TCCR2B = _BC( FOC2A ) | _BC( FOC2B ) |
_BC( WGM22 ) | // Clear timer on compare match
_BS( CS22 ) | _BC( CS21 ) | _BS( CS20 ); // prescaler f = clk2 / 128
OCR2A = 125 - 1; // 16MHz / 128 = 125kHz => 125kHz/125 = 1kHz
TCNT2 = 0;
TIMSK2 = _BC( OCIE2B ) | _BS( OCIE2A ) | _BC( TOIE2 );// Enable compare match A interrupts
sei(); // Enable global interrupts
}
// Attach interrupt routine to the Timer Compare Interrupt
ISR( TIMER2_COMPA_vect ) {
millisecondCounter += 1;
if ( millisecondCounter == 1000 ) { // 1000 milliseconds equals 1 second
secondCounter += 1;
millisecondCounter = 0;
}
};
void loop() {
digitalWrite( led , secondCounter & 1 );
}
Best Answer
First of all, you're worrying about a non-issue. Since the operation only depends on the value of the LSB of the first byte of the 32-bit variable, whether or not the other bytes change while that particular stretch of code is executing has no effect whatsoever.
But in the general case, your function
atomic_int32()
fails because it is the caller that reads the variable associated with the formal argumentvar
before the function is called, while it is building the argument list on the stack. To fix this, and actually read the variable inside the function while the interrupts are off, you need to pass the address of the variable to the function, like this:Call it like this: