Can arduino interrupts occur in a middle of if statement?

Question

I got an issues from few days, got some code which reset millis time on successful i2c communication, if not it restart an relay after specified period of time. Here is part of the code:

 if ((unsigned long)(i2c1Millis1 - i2cpreviousMillis1) >= i2cset1 )  // check if not restarted soon
        {

i2c1Millis1 are updated every run of the loop and on successful i2c communication.

i2cpreviousMillis1 is updated on successful i2c request (coming from interrupt)

I manage to debug when false restart occur and looks like i2c1Millis1 is lower than i2cpreviousMillis1

So question is: can an interrupt occur while those two millis are compared when value of i2c1Millis1 is taken for compare and then come interrupt where i2cpreviousMillis1 is updated to current millis and it become greater and trigger the condition?

(I already modified my code i2cpreviousMillis1 is updated with i2c1Millis1 value on successful communication to prevent that, but it is not working, from time to time it made false restarts).

Answer 1

Yes. And it's even worse. Because your variables are longs, they are 4 bytes long, and most Arduinos are only 8-bit processors, the code only checks one byte at a time. So for example, after checking the first byte, an interrupt could occur, changing the 2nd, 3rd, and 4th byte. So you end up comparing the first byte of the old value, and other bytes of the new value.

To prevent this you could disable interrupt for a very short time, and make a copy the variable.

cli();//disable interrupts
unsigned long temp = i2c1Millis1;
sei();//enable interrupts

Or use the util/atomic.h macros

#include <util/atomic.h>

...

ATOMIC_BLOCK(ATOMIC_FORCEON)
{
  unsigned long temp = i2c1Millis1;
}

Answer 2

Yes, an interrupt can occur at any time and can change the values during a calculation. And since you work with unsigned long variables (which are 4 bytes as I remember), it is also possible, that the calculation uses the old value for the first bytes and the new value for the other bytes, meaning that the used value for calcuation is totally garbage.

To prevent that your checks must either be atomic (meaning consisting only of 1 base instruction, which cannot be interrupted, e.g. handling only 1 byte) (that is not possible here), or you must prevent the calculation from being interrupted. For this you can turn off all or only the corresponding interrupt. To turn off all interrupts you can use noInterrupts() and interrupts() to turn them on again.

But it is questionable, why you need to update this time directly in the ISR. Instead you could only set an atomic flag (1 byte) and check this flag in the loop. If the flag was set, reset it and save the millis value. If you loop runs fast enough (which seems the case, since you already use it to set a millis timestamp) that should be sufficient

Both ways can be used and it depends on you, if it's ok to turn of interrupts for a short time, since you might miss an I2C interrupt this way.

Answer 3

Yes. An interrupt can occur pretty much anywhere.

Interrupts are processed between each assembly language instruction. That if will be made up of many many ASM instructions, so there are many many places where an interrupt can occur.

To avoid it you use what are called critical sections where you disable interrupts before performing some operations, then enable them again afterwards.

It is recommended to keep critical sections as short as possible to avoid breaking your interrupts. For this reason most critical sections generally involve copying values to or from local variables before then doing operations on them.

noInterrupts();
uint32_t localPreviousMillis = i2cpreviousMillis1;
interrupts();

if ((unsigned long)(i2c1Millis1 - localPreviousMillis) >= i2cset1 )  // check if not restarted soon

On an 8-bit system even assigning a simple value like that can be interrupted. A 32-bit value is made up of four 8-bit values, and an interrupt can occur between the copying of any of those 4 bytes.