How to plan/structure interrupt-based code?

[Simon]

Looking at the OP requirements all the interrupt timing he really needs is around comms. ie interrupt says there is a message byte, empty register into correct array position and await new byte, in the mean time keep taking whatever external measurements are required and put the latest values into the data packet when a message is sent back.

[thinkfat]

When using interrupts, I typically have one interrupt that just drives a counter variable and derived from that counter I will iterate over a timer list with callback functions from my mainloop context. That serves my housekeeping needs and all tasks that just need to be run at a defined point of time in the future.

Peripheral interrupts are structured such that only the minimum necessary actions are taken. I treat interrupt handlers as atomic so they define the minimum response time of the system. Therefore, the time spent in atomic context should be as short as possible. For example, a value will be read from an ADC or a UART fifo and stored in a buffer, a flag will be set and a timer added so that a mainloop function can do the processing of the values.

I will also keep the amount of data structures touched from interrupt context small. That will limit the amount of headache considerably.

[Cerebus]

Looking at the OP requirements all the interrupt timing he really needs is around comms. ie interrupt says there is a message byte, empty register into correct array position and await new byte, in the mean time keep taking whatever external measurements are required and put the latest values into the data packet when a message is sent back.

I think you're missing the subtleties here, which is kind of the point of my long post. There's every opportunity with this kind of producer/consumer process to lose notifications (and therefore characters) unless you properly synchronise access to the array, the reader index for that array and the writer index for that array.

Exactly the same kind of race condition is possible here - consumer reads the read/write index into a register, interrupt (producer) updates it in memory, consumer uses the stale register value after the interrupt returns. You cannot avoid these errors without using interlocks such as read-modify-write instructions or higher level structures such as semaphores or mutexes based on them.

[Simon]

Of course you use interlocks. You need to know that the multi-byte message is complete. You will have flags for the fact that you have already looked at the message in the array and that a new reception has started. that remains in place until the entire new message is received at which point you take the data you want from the array and set the flag as message read.

[NorthGuy]

Code: [Select]

again:
          MOV    A, counter
          // danger point starts here
          MOV    B, compare_value
          CMP    A, B    // danger point ends with the finished execution of this instruction
          JE    do_task

          // a little bit of housekeeping code here, not shown

          JMP    again
do_task:
          //  task code follows on here

If the interrupt happens in the region covered by the two 'danger point' comments, the value of 'counter' as set by the interrupt routine, and the value of 'counter' as used by the main loop are different - the main loop doesn't 'see' the updated counter because it has already loaded the now out-of-date value into a register. So your main loop will not execute 'do_task' for at least one more main loop wait cycle, or will miss the trigger entirely if there's another counter incrementing interrupt happens before the loop has time to run around again (say because there's some housekeeping code at the top of the loop as here, or the increment interrupts are very frequent). So you end up with timing jitter, or worse, you miss your event entirely.

I cannot agree with your assesment. If the interrupt happens right after the line marked "danger point ends with the finished execution of this instruction" (or in any other place for that matter), these bad things will happen anyway - you get jitter because you may wait for almost a full cycle to pass, and you're doomed if the counter increments twice during the loop. There's no defined "danger" zone and the pseudo-assembler code is irrelevant to the problem.

Hopefully, if you put things in the main loop, you understand that these things must be able to tolerate jitter. Also, you probably will combat double ticks with:

Code: [Select]

while ((counter - compare_value) < 0)    // Wait for the interrupt routine to set 'counter' to our trigger value
instead of

Code: [Select]

while (counter != compare_value)    // Wait for the interrupt routine to set 'counter' to our trigger value
There are other things to look at, such as making the counter "volatile", making sure the counter variable in not too short, or taking precautions if it the counter variable is too long and requires non-atomic load, or mis-understanding differences between signed and unsigned comparisons. The possibility of such mistakes doesn't mean that Simon's approach is invalid or too naive.

[Cerebus]

Code: [Select]

again:
          MOV    A, counter
          // danger point starts here
          MOV    B, compare_value
          CMP    A, B    // danger point ends with the finished execution of this instruction
          JE    do_task

          // a little bit of housekeeping code here, not shown

          JMP    again
do_task:
          //  task code follows on here

If the interrupt happens in the region covered by the two 'danger point' comments, the value of 'counter' as set by the interrupt routine, and the value of 'counter' as used by the main loop are different - the main loop doesn't 'see' the updated counter because it has already loaded the now out-of-date value into a register. So your main loop will not execute 'do_task' for at least one more main loop wait cycle, or will miss the trigger entirely if there's another counter incrementing interrupt happens before the loop has time to run around again (say because there's some housekeeping code at the top of the loop as here, or the increment interrupts are very frequent). So you end up with timing jitter, or worse, you miss your event entirely.

I cannot agree with your assesment. If the interrupt happens right after the line marked "danger point ends with the finished execution of this instruction" (or in any other place for that matter), these bad things will happen anyway - you get jitter because you may wait for almost a full cycle to pass, and you're doomed if the counter increments twice during the loop. There's no defined "danger" zone and the pseudo-assembler code is irrelevant to the problem.

Hopefully, if you put things in the main loop, you understand that these things must be able to tolerate jitter. Also, you probably will combat double ticks with:

Code: [Select]

while ((counter - compare_value) < 0)    // Wait for the interrupt routine to set 'counter' to our trigger value
instead of

Code: [Select]

while (counter != compare_value)    // Wait for the interrupt routine to set 'counter' to our trigger value
There are other things to look at, such as making the counter "volatile", making sure the counter variable in not too short, or taking precautions if it the counter variable is too long and requires non-atomic load, or mis-understanding differences between signed and unsigned comparisons. The possibility of such mistakes doesn't mean that Simon's approach is invalid or too naive.

Somebody didn't look at the original code very carefully, to have missed the 'volatile' there. But 'volatile' won't magically protect you from interrupts happening between instructions in mainline code. 'volatile' simply says to the compiler "Beware, this memory location may change at times that you, the compiler, don't expect because it is also accessed by code that is outside your data flow analysis.".

If you want a step-by-step, cycle-by-cycle analysis of the vulnerabilities here then you're not going to get it. This was a simple throwaway example of where a race condition between mainline code and interrupt code can catch you out, not a worked example of how to do it right, but 'look it can go wrong here'.

If you think the pseudo-assembler is irrelevant, you don't understand the problem - specifically that a register load that is made before an interrupt, which changes the loaded value and then used, stale, after the interrupt returns. By the way, the simple fix for this specific, simplistic, example is a disable_interrupts()/enable_interrupts() pair around the critical section.

[NorthGuy]

If you think the pseudo-assembler is irrelevant, you don't understand the problem - specifically that a register load that is made before an interrupt, which changes the loaded value and then used, stale, after the interrupt returns. By the way, the simple fix for this specific, simplistic, example is a disable_interrupts()/enable_interrupts() pair around the critical section.

See for yourself:

Assume the counter changes from "compare_value-1" to "compare_value" during the interrupt:

Code: [Select]

          --- if interrupt happens here we go to do_task immediately
          MOV    A, counter
          --- if interrupt happens here we go for another loop
          // danger point starts here
          MOV    B, compare_value
          --- if interrupt happens here we go for another loop
          CMP    A, B    // danger point ends with the finished execution of this instruction
          --- if interrupt happens here we go for another loop
          JE    do_task
          --- if interrupt happens here we go for another loop
          <other instruction here>
          --- if interrupt happens here we go for another loop

As you can see, there's a point where the behaviour changes. If interrupt happens earlier (meaning that the time has come to do the task) the code does the task. If interrupt happens later (meaning this is not yet time), the code goes for another loop. There's no harm done either way. There's no danger zone, and absolutely nothing changes when the code leaves the imaginary danger zone.

If you add interrupt enable/disable to remove the imaginary danger zone, the behaviour doesn't change a bit:

Code: [Select]

          --- if interrupt happens here we go to do_task immediately
          DI
          --- interrupt cannot happen here
          MOV    A, counter
          --- interrupt cannot happen here
          MOV    B, compare_value
          --- interrupt cannot happen here
          CMP    A, B    // danger point ends with the finished execution of this instruction
          --- interrupt cannot happen here
          EI
          --- the interrupt will happen here if the interrupt condition arose when it was disabled
          --- if interrupt happens here we go for another loop
          JE    do_task
          --- if interrupt happens here we go for another loop
          <other instruction here>
          --- if interrupt happens here we go for another loop


[emece67]

.

[Cerebus]

If you think the pseudo-assembler is irrelevant, you don't understand the problem - specifically that a register load that is made before an interrupt, which changes the loaded value and then used, stale, after the interrupt returns. By the way, the simple fix for this specific, simplistic, example is a disable_interrupts()/enable_interrupts() pair around the critical section.

See for yourself:

Assume the counter changes from "compare_value-1" to "compare_value" during the interrupt:

Code: [Select]

          --- if interrupt happens here we go to do_task immediately
          MOV    A, counter
          --- if interrupt happens here we go for another loop
          // danger point starts here
          MOV    B, compare_value
          --- if interrupt happens here we go for another loop
          CMP    A, B    // danger point ends with the finished execution of this instruction
          --- if interrupt happens here we go for another loop
          JE    do_task
          --- if interrupt happens here we go for another loop
          <other instruction here>
          --- if interrupt happens here we go for another loop

As you can see, there's a point where the behaviour changes. If interrupt happens earlier (meaning that the time has come to do the task) the code does the task. If interrupt happens later (meaning this is not yet time), the code goes for another loop. There's no harm done either way. There's no danger zone, and absolutely nothing changes when the code leaves the imaginary danger zone.

If you add interrupt enable/disable to remove the imaginary danger zone, the behaviour doesn't change a bit:

Code: [Select]

          --- if interrupt happens here we go to do_task immediately
          DI
          --- interrupt cannot happen here
          MOV    A, counter
          --- interrupt cannot happen here
          MOV    B, compare_value
          --- interrupt cannot happen here
          CMP    A, B    // danger point ends with the finished execution of this instruction
          --- interrupt cannot happen here
          EI
          --- the interrupt will happen here if the interrupt condition arose when it was disabled
          --- if interrupt happens here we go for another loop
          JE    do_task
          --- if interrupt happens here we go for another loop
          <other instruction here>
          --- if interrupt happens here we go for another loop


You've merely proved what I asserted in the original post:

So your main loop will not execute 'do_task' for at least one more main loop wait cycle, or will miss the trigger entirely if there's another counter incrementing interrupt happens before the loop has time to run around again (say because there's some housekeeping code at the top of the loop as here, or the increment interrupts are very frequent). So you end up with timing jitter, or worse, you miss your event entirely.

I don't really see how that helps, but thanks for proving what I asserted.

Yes, the hazard here, if the task and housekeeping code is simple, is in the mainline code executing late or exhibiting jitter, or in the extreme case, if the housekeeping or task is lengthy, missing an interrupt. This all may be trivial or it may be catastrophic depending on the application, on what's in the task or the housekeeping code. It is a simple contrived example, reduced to the bare essentials. The choice of an equals comparison was deliberate to make it as simple an example as possible and make it clear how a single missed interrupt could cause the code to fail - it is deliberately fragile for epistemological purposes. The whole thing exists merely to demonstrate that there is a hazard there that while a shared value is held in a register that it can be outdated before it is used.

The point is to illustrate that critical sections exist and they are not immediately obvious or intuitive, nothing more, nothing less.

The overall point stands, code with parallel execution/interrupts is tricky, with non-obvious hazards, and a sensible person will go and learn how to use the standard tools for handling this.

[NorthGuy]

You've merely proved what I asserted in the original post:

No, you need to give me more credit than that. I have proved that isolating your supposedly critical section with disable/enable interrupts doesn't change the behaviour in a slightest. Which means that this wasn't a critical section at all, and there was no "danger point" in the pseudo-assembler code you have posted.

[Cerebus]

You've merely proved what I asserted in the original post:

No, you need to give me more credit than that. I have proved that isolating your supposedly critical section with disable/enable interrupts doesn't change the behaviour in a slightest. Which means that this wasn't a critical section at all, and there was no "danger point" in the pseudo-assembler code you have posted.

You haven't because you haven't been listening, but I'm quite happy to leave you with the belief that you did. Hopefully those I was trying to help have, or will, see the point.

[emece67]

.

[thinkfat]

You're all making this much more of a problem than it actually is. For such simple requirements with only one mainloop and no threads, all you need is one counter variable, one timer interrupt and a simple timer list sorted by earliest expiration time. Keep in mind that the counter will only monotonically increase and the only requirement for a lock free implementation is that read and write access to the counter is atomic. Any 32bit CPU can do that.

The modulo trick is superfluous. Subtracting two unsigned values and comparing 'smaller or equal to zero' is enough to catch a late timer.

[Kalvin]

For advanced users:
If the system requirements allow, it is also possible to place all the application logic into the timer interrupt handler (foreground process). Thus, the system will run at the timer interrupt rate. The application logic could be then implemented as s simple, time-triggered, co-operative state machine. The main() function would just initialize the system, start the timer and enter infinite loop doing nothing (putting the CPU into a sleep, waiting for the next timer interrupt). The timer interrupt will wake the CPU and the application will be run one step at a time at each timer interrupt. After the execution of the interrupt handler has completed, the CPU will enter back into the sleep. Quite simple, deterministic approach.

Of course, it is possible to perform some non-critical background processing in the main() function, too. In this case, it is necessary to implement some guards (like enable/disable interrupts) when transferring data between the foreground process and the background process. Enabling/disabling the interrupts may introduce some jitter to the system response, so that need to be analyzer carefully if some jitter in the system response cannot be tolerated.

It may not be possible to use standard printf() for the debugging purposes in the foreground process AND the background process use the printf(), though. However, if the background process is not used, or the background process does not use printf(), the application code running in the interrupt handler should be able to use printf(). Please check your implementation of the printf().

Here is a good reference for the time-triggered, co-operative systems: ‘Patterns for Time-Triggered Embedded Systems’ by Michael J. Pont
https://www.safetty.net/publications/pttes
The book is freely available. Although the book uses the 8051 as an example, the principles are equally applicable to all microcontrollers from Attiny, Arduino to SMT32.

An addendum to this book is also freely available with an updated task scheduler using LPC1769 (ARM Cortex-M3 based MCU):
https://www.researchgate.net/publication/311614300_The_Engineering_of_Reliable_Embedded_Systems

Edit: Modified the wording regarding the addendum and Cortex M3, as suggested by Simon and emece67 below.
Edit2: Fixed a typo.

[uer166]

For advanced users:
If the system requirements allow, it is also possible to place all the application login into the timer interrupt handler (foreground process). Thus, the system will run at the timer interrupt rate. The application logic could be then implemented as s simple, time-triggered, co-operative state machine. The main() function would just initialize the system, start the timer and enter infinite loop doing nothing (putting the CPU into a sleep, waiting for the next timer interrupt). The timer interrupt will wake the CPU and the application will be run one step at a time at each timer interrupt. After the execution of the interrupt handler has completed, the CPU will enter back into the sleep. Quite simple, deterministic approach.

Of course, it is possible to perform some non-critical background processing in the main() function, too. In this case, it is necessary to implement some guards (like enable/disable interrupts) when transferring data between the foreground process and the background process. Enabling/disabling the interrupts may introduce some jitter to the system response, so that need to be analyzer carefully if some jitter in the system response cannot be tolerated.

It may not be possible to use standard printf() for the debugging purposes in the foreground process AND the background process use the printf(), though. However, if the background process is not used, or the background process does not use printf(), the application code running in the interrupt handler should be able to use printf(). Please check your implementation of the printf().

Here is a good reference for the time-triggered, co-operative systems: ‘Patterns for Time-Triggered Embedded Systems’ by Michael J. Pont
https://www.safetty.net/publications/pttes
The book is freely available. Although the book uses the 8051 as an example, the principles are equally applicable to all microcontrollers from Attiny, Arduino to SMT32.

A newer version of the book is also available with an updated task scheduler:
https://www.researchgate.net/publication/311614300_The_Engineering_of_Reliable_Embedded_Systems

And the hybrid scheduler described in that book can get around some of the limitations of the cooperative one, with some tradeoff regarding safety guarantees.

[emece67]

.

[Simon]

And the updated version of the book uses the LPC1769, an ARM Cortex-M3 based MCU, instead of the venerable 8051.

I'd call it an addendum. The "new" book is a mere 400 pages, the old one 1'000. Maybe he just rewrote the examples.

[Kalvin]

Thanks Simon and emece67, I edited & updated my wording in my original post.

[Siwastaja]

Kalvin's strategy is very good, and I wouldn't call it "advanced"; quite the opposite, it's easy to do.

My take is, ignore the general advice of "keep ISRs short and make them set flags; process flags in main()". This may work in some cases, but is generally undeterministic, hard to analyze, and forces much more shared data between tasks than would be strictly necessary.

For example, if you follow the typical advice and program an UART interrupt, set a flag in ISR, and poll for that flag in the main loop, you could have just polled the peripheral interrupt flag to begin with, without writing or enabling the ISR in the first place. Or, you could use DMA to stream the data to memory, directly, and process it at a convenient time. But if you need to do it when the event happens, why not do it in the ISR, completely? Only if you have two separable actions, a timing-critical one, and a irrelevant-timing heavy one, then it makes sense to set flags and continue processing in main(). But this isn't any kind of default case. And even then, it would be more deterministic and easier to follow to demote the interrupt priority by creating a lower-priority software interrupt that continues processing.

Interrupt priorities and pre-empting interrupts make it easy to run time- or event-triggered tasks right whenever needed.

It's a good idea to run all code that needs any sort of realtime deterministic behavior in the interrupts. And contrary to the usual poor advice, there is no reason to make the interrupt handlers extremely short. Say you get a new ADC datapoint, and need to do some math upon it, to then decide whether to turn an IO on or off. Do all that in the ISR. Maybe your ISR takes worst case 1ms but you need to react to a comparator event in 10µs? Write a higher-priority ISR for the comparator event!

Maybe you have a state machine, run in ISR, triggered by SPI transfers from a sensor, and get a full data packet after the 17th state? At that state, generate a lower-priority software interrupt which takes over processing the full dataset. Now your longer processing function is out of the way, but you still have better determinism and safety against code changes compared to if you just set a flag and looked at it at some random place in your already long main loop.

If you go this way, the main() ends up doing the initialization and then either remains in empty while(1) loop, or does some procedural logic where timing and jitter is completely irrelevant, as long as it gets enough CPU time. Which is quite easy to calculate from ISR repetition rates and worst-case processing times.

[mac.6]

mbed is easy to use, although Flash hungry (just checked with a STM32L432kc, 16 KB for blinky with the RT part removed, 25 KB with it). Also, it is too much "high" level and complex for my taste, I feel I'm loosing control with it (I still use it sometimes, though).

But switching to another (supported) target is quite easy, that's true.

Just for the sake of comparison: ChibiOS on an STM32G474, full RTOS, variable frequency blinky in a thread, frequency set from main thread using the timer library for a quadrature encoder, SPI, an 8 digit seven segment driver atop that, serial port driver, streams library, printf library, quite a few 'debug' printfs - 13kb. (Just kicking the tyres on ChibiOS to see if it's worth adopting/persevering with.)

ChibiOS is nice but tend to growth into a full framework suite and is handicapped by it's dual GPL/proprietary licensing scheme (worst thing to do imo).

[NorthGuy]

Say you get a new ADC datapoint, and need to do some math upon it, to then decide whether to turn an IO on or off. Do all that in the ISR.

Moreover, you can piggy-back even more processing to that ISR. Typically, ADC readings come at regular intervals, so after you process your ADC reading and do everything what you need to do to react on it, you're still inside ISR. At this point you can be certain that you have some time until the next ADC reading comes. So, while still within ISR, you can do some things which should be done regularly, such as advance your "time" counter, or do some other housekeeping. You only need to be careful not to take too much time. You need to be done by the time the next interrupt happens, and you need to leave some time to let the main code run.

The things piggy-backed to a regular interrupt will have less jitter than they would have if you placed them in a lower-priority ISR or in main. If you don't have interrupt priorities, piggy-backing improves timing considerably.

[Siwastaja]

Yes, an ADC would be easy to configure to either free-run with pre-determined sampling and conversion times, or conversion triggered from a timer peripheral, resulting in stable and predictable timebase interrupt (give or take a few clock cycles of jitter if internal signals pass clock domains). Two flies with one stone.

You can further divide the ADC interrupt rate down by incrementing a counter and only acting every n interrupts. Process data, and if(++count > 123) { do_more(); count = 0;} Even fancier, if you have five different housekeeping tasks to do with regular intervals and doing them all at once would take too long before the next interrupt, just use modulo-5 of the count to decide which housekeeping function to call.

A practical example would be an ADC converting at 10ksmps, and the interrupt handler having ten different "slots" for 1kHz timebase stuff, each of which must be finished in significantly less than 0.1ms.

It's very wise to add some kind of rudimentary monitoring; for example reset a counter at the start of ISR, read at the end of ISR, store maximum value. Assert and fail with error message if it ever exceeds say 50% of the available time.

You can obviously bring a full-fledged RTOS to do the same but if you are constrained by processing power, an RTOS won't magically make your CPU faster.

[NorthGuy]

You can further divide the ADC interrupt rate down by incrementing a counter and only acting every n interrupts. Process data, and if(++count > 123) { do_more(); count = 0;} Even fancier, if you have five different housekeeping tasks to do with regular intervals and doing them all at once would take too long before the next interrupt, just use modulo-5 of the count to decide which housekeeping function to call.

USB 2.0 works that way. They have frames (1 ms each for Full Speed). Each frame starts from the Start of Frame (SOF) packet, which you can regard as the beginning of the interrupt.

The devices may create so called "interrupt end-points". For each interrupt end-point the device requests that it must be transacted every n-th frame. The controller packs these requests and schedules the interrupt end-points to be transacted according to the requests of their respective devices.

Once the interrupt end-points are processed, the controller transacts all other end-points (such as bulk end-points), and does so until the next SOF. That's the similar to how the "main" loop is executed in MCUs - the timing is vague. Some of the bulk end-points may not get a chance to get transacted, so they get postponed until the next frame. Or vice versa, if there's enough time in the current frame, some end-points may be transacted many times in a row.

Hence, the strange names - "interrupt" and "bulk" end-points, which is puzzling for a person who first starts working with USB.

[voltsandjolts]

Apologies in advance if this is slightly off topic, but there was some discussion earlier on RTOS.
This is quite an interesting read about space qualified VxWorks, RTEMS and Sylix.

https://arstechnica.com/features/2020/10/the-space-operating-systems-booting-up-where-no-one-has-gone-before/

[Howardlong]

Apologies in advance if this is slightly off topic, but there was some discussion earlier on RTOS.
This is quite an interesting read about space qualified VxWorks, RTEMS and Sylix.

https://arstechnica.com/features/2020/10/the-space-operating-systems-booting-up-where-no-one-has-gone-before/

Unlike many other areas of engineering, in aerospace, there's often bias towards systems with proven heritage as a means to reduce risk, and even when something new is placed in a payload, it's often complemented with similar functionality bearing heritage. This also helps mitigate against common mode failure.