Question

This question is about programming small microcontrollers without an OS. In particular, I'm interested in PICs at the moment, but the question is general.

I've seen several times the following pattern for keeping time:

Timer interrupt code (say the timer fires every second):

...
if (sec_counter > 0)
  sec_counter--;
...

Mainline code (non-interrupt):

sec_counter = 500; // 500 seconds

while (sec_counter)
{
  // .. do stuff
}

The mainline code may repeat, set the counter to various values (not just seconds) and so on.

It seems to me there's a race condition here when the assignment to sec_counter in the mainline code isn't atomic. For example, in PIC18 the assignment is translated to 4 ASM statements (loading each byte at the time and selecting the right byte from the memory bank before that). If the interrupt code comes in the middle of this, the final value may be corrupted.

Curiously, if the value assigned is less than 256, the assignment is atomic, so there's no problem.

Am I right about this problem? What patterns do you use to implement such behavior correctly? I see several options:

  • Disable interrupts before each assignment to sec_counter and enable after - this isn't pretty
  • Don't use an interrupt, but a separate timer which is started and then polled. This is clean, but uses up a whole timer (in the previous case the 1-sec firing timer can be used for other purposes as well).

Any other ideas?

Was it helpful?

Solution

The PIC architecture is as atomic as it gets. It ensures that all read-modify-write operations to a memory file are 'atomic'. Although it takes 4-clocks to perform the entire read-modify-write, all 4-clocks are consumed in a single instruction and the next instruction uses the next 4-clock cycle. It is the way that the pipeline works. In 8-clocks, two instructions are in the pipeline.

If the value is larger than 8-bit, it becomes an issue as the PIC is an 8-bit machine and larger operands are handled in multiple instructions. That will introduce atomic issues.

OTHER TIPS

Write the value then check that it is the value required would seem to be the simplest alternative.

do {
 sec_counter = value;
} while (sec_counter != value);

BTW you should make the variable volatile if using C.

If you need to read the value then you can read it twice.

do {
    value = sec_counter;
} while (value != sec_counter);

You definitely need to disable the interrupt before setting the counter. Ugly as it may be, it is necessary. It is a good practice to ALWAYS disable the interrupt before configuring hardware registers or software variables affecting the ISR method. If you are writing in C, you should consider all operations as non-atomic. If you find that you have to look at the generated assembly too many times, then it may be better to abandon C and program in assembly. In my experience, this is rarely the case.

Regarding the issue discussed, this is what I suggest:

ISR:
if (countDownFlag)
{
   sec_counter--;
}

and setting the counter:

// make sure the countdown isn't running
sec_counter = 500;
countDownFlag = true;

...
// Countdown finished
countDownFlag = false;

You need an extra variable and is better to wrap everything in a function:

void startCountDown(int startValue)
{
    sec_counter = 500;
    countDownFlag = true;
}

This way you abstract the starting method (and hide ugliness if needed). For example you can easily change it to start a hardware timer without affecting the callers of the method.

Because accesses to the sec_counter variable are not atomic, there's really no way to avoid disabling interrupts before accessing this variable in your mainline code and restoring interrupt state after the access if you want deterministic behavior. This would probably be a better choice than dedicating a HW timer for this task (unless you have a surplus of timers, in which case you might as well use one).

If you download Microchip's free TCP/IP Stack there are routines in there that use a timer overflow to keep track of elapsed time. Specifically "tick.c" and "tick.h". Just copy those files over to your project.

Inside those files you can see how they do it.

It's not so curious about the less than 256 moves being atomic - moving an 8 bit value is one opcode so that's as atomic as you get.

The best solution on such a microcontroller as the PIC is to disable interrupts before you change the timer value. You can even check the value of the interrupt flag when you change the variable in the main loop and handle it if you want. Make it a function that changes the value of the variable and you could even call it from the ISR as well.

Well, what does the comparison assembly code look like?

Taken to account that it counts downwards, and that it's just a zero compare, it should be safe if it first checks the MSB, then the LSB. There could be corruption, but it doesn't really matter if it comes in the middle between 0x100 and 0xff and the corrupted compare value is 0x1ff.

The way you are using your timer now, it won't count whole seconds anyway, because you might change it in the middle of a cycle. So, if you don't care about it. The best way, in my opinion, would be to read the value, and then just compare the difference. It takes a couple of OPs more, but has no multi-threading problems.(Since the timer has priority)

If you are more strict about the time value, I would automatically disable the timer once it counts down to 0, and clear the internal counter of the timer and activate once you need it.

Move the code portion that would be on the main() to a proper function, and have it conditionally called by the ISR.

Also, to avoid any sort of delaying or missing ticks, choose this timer ISR to be a high-prio interrupt (the PIC18 has two levels).

One approach is to have an interrupt keep a byte variable, and have something else which gets called at least once every 256 times the counter is hit; do something like:

// ub==unsigned char; ui==unsigned int; ul==unsigned long
ub now_ctr; // This one is hit by the interrupt
ub prev_ctr;
ul big_ctr;

void poll_counter(void)
{
  ub delta_ctr;

  delta_ctr = (ub)(now_ctr-prev_ctr);
  big_ctr += delta_ctr;
  prev_ctr += delta_ctr;
}

A slight variation, if you don't mind forcing the interrupt's counter to stay in sync with the LSB of your big counter:

ul big_ctr;
void poll_counter(void)
{
  big_ctr += (ub)(now_ctr - big_ctr);
}

No one addressed the issue of reading multibyte hardware registers (for example a timer. The timer could roll over and increment its second byte while you're reading it.

Say it's 0x0001ffff and you read it. You might get 0x0010ffff, or 0x00010000.

The 16 bit peripheral register is volatile to your code.

For any volatile "variables", I use the double read technique.

do {
       t = timer;
 } while (t != timer);
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top