Detecting when interrupts are enabled

I have a critical section of code that must not be interrupted so I am using

cli();
// Do something that must not be interrupted
sei();

Now I should only re-enable interrupts if they were enabled before the cli() so how do I detect that interrupts are enabled?

Read the appropriate interrupts register to see if they are enabled.

Now I should only re-enable interrupts if they were enabled before the cli() so how do I detect that interrupts are enabled?

Can't you remember if you disabled them? I would certainly think you should be able to.

Put this at the top of your sketch (may not be necessary)…

#include <util/atomic.h>

Use this pattern…

  ATOMIC_BLOCK( ATOMIC_RESTORESTATE )
  {
    // critical section goes here
  }

CrossRoads:
Read the appropriate interrupts register to see if they are enabled.

This was my first thought but I had to assume I was not the first person to need this functionality so there had to be a function or some other method that read the status of the interrupt enable bit without having to find out which register it was in and then read it.

PaulS:

Now I should only re-enable interrupts if they were enabled before the cli() so how do I detect that interrupts are enabled?

Can't you remember if you disabled them? I would certainly think you should be able to.

I'm not sure how to take this! Of course I know that I disabled the interrupts but I do no know if they were enabled before I came to the point when I needed to disable them.

Thanks for this - it looks to be exactly what I need but I shall have to look at the small print to make sure.

Edit : Within atomic.h one has the statemement

i. e. the interrupt status will be
restored to the same value it has been when entering the
respective block.

so it is exactly what I am looking for.

Of course I know that I disabled the interrupts but I do no know if they were enabled before I came to the point when I needed to disable them.

I don't understand this. Set a flag when you disable interrupts. Clear it when you enable them. If the flag is set, you know that interrupts are already disabled, so disabling and re-enabling them is not required.

On the other hand, it takes next to no time to disable and enable interrupts, so I don't understand the problem you are trying to solve.

PaulS:
I don't understand this. Set a flag when you disable interrupts. Clear it when you enable them. If the flag is set, you know that interrupts are already disabled, so disabling and re-enabling them is not required.

On the other hand, it takes next to no time to disable and enable interrupts, so I don't understand the problem you are trying to solve.

When one writes a library module one does not know whether or not the user of that library will have interrupts enabled at the entry to the code. It would be very fragile code that for a critical section of code one blindly turned off interrupts before and then back on again afterwards without worrying whether or not it was appropriate to turn the interrupts back on.

I have just wasted two days dealing with some Liquid Crystal library code that worked great most of the time but every now and again failed for no obvious reason. For a long time I assumed it was my problem but eventually had to assume that the problem was in one of the libraries and I narrowed it down to methods in the Liquid Crystal library that could not tolerate being interrupted. My initial fix was to edit the library to blindly turn off the interrupts when at a timing critical point and then back on again afterwards and this worked great for me but it is fragile solution at best and the ATOMIC_BLOCK solution is a very much better solution. Even this can have problems since it can add to interrupt latency but this does not seem to be a problem in my current application.

When I have thoroughly tested the changes and I am completely happy with them I shall publish them.

Another option, which is what the Arduino cores use:

uint8_t oldSREG = SREG;
// Clear interrupts
cli();

// Atomic, interrupt sensitive code codes here.
//
//
//

// Restore old interrupt state
SREG = oldSREG;

stowite:
It would be very fragile code that for a critical section of code one blindly turned off interrupts before and then back on again afterwards without worrying whether or not it was appropriate to turn the interrupts back on.

I have to say I’m with @PaulS on this one.

I think, for the most part, one does blindly turn off interrupts very briefly if it is essential to perform some non-atomic instruction.

And I especially don’t understand why you are in the least bit worried about turning on interrupts after you have turned them off yourself.

…R

Robin2:
And I especially don't understand why you are in the least bit worried about turning on interrupts after you have turned them off yourself.

...R

When one writes a library module one does not know whether or not the user of that library will have interrupts enabled at the entry to the code.

It is possible for other code outside of the library to have turned off interrupts already before the particular routine the OP is creating has run. If this is true, turning on interrupts immediately after the critical section has been run would be inappropriate. You would want to restore them back to the original state, which might very well be off.

Robin2:

stowite:
It would be very fragile code that for a critical section of code one blindly turned off interrupts before and then back on again afterwards without worrying whether or not it was appropriate to turn the interrupts back on.

I have to say I'm with @PaulS on this one.

I think, for the most part, one does blindly turn off interrupts very briefly if it is essential to perform some non-atomic instruction.

And I especially don't understand why you are in the least bit worried about turning on interrupts after you have turned them off yourself.

...R

You are missing the point. If there is a section of code in which you don't want interrupts then if prior to turning off the interrupts they were not actually turned on then you do not want to turn them on at the end of the section! That is exactly what ATOMIC_BLOCK does and since I did not know about ATOMIC_BLOCK I was trying to re-invent it.

It is not a matter of blindly turning OFF interrupts but blindly turning ON interrupts.

This is done all the time. In assembly, it's done by pushing the flags register on the stack, disabling interrupts, doing your stuff, and then popping the flags register back off the stack. It's done automatically whenever there is an interrupt.

In C, using a local variable is like pushing it onto the stack. Like what Jiggy-Ninja posted.

TanHadron:
This is done all the time. In assembly, it's done by pushing the flags register on the stack, disabling interrupts, doing your stuff, and then popping the flags register back off the stack. It's done automatically whenever there is an interrupt.

In C, using a local variable is like pushing it onto the stack. Like what Jiggy-Ninja posted.

Except that the two actions (pushing onto the stack and then clearing the interrupt) need to be atomic and your way is not! You will get away with this non-atomic approach 99.999... % of the time but sometime it will jump up and bite you.

Except that the two actions (pushing onto the stack and then clearing the interrupt) need to be atomic

Why is that? If the interrupts were already disabled, there is no way the two actions could be separated by an interrupt. If the interrupts are already enabled, what is the difference between an interrupt firing before the push, or an interrupt firing between the push and the cli? I don't see how it could be a problem.

In any case, ATOMIC_BLOCK( ATOMIC_RESTORESTATE ) isn't atomic at that point, either. The compiler implements it by saving the SREG in a general purpose register. In my case, it was r24. Then it executes the cli.

	in r24,__SREG__
	cli

;        ...    My block of code that can't be interrupted

	out __SREG__,r24

TanHadron:

Except that the two actions (pushing onto the stack and then clearing the interrupt) need to be atomic

Why is that?

Having thought about this for a short while you are right - the two actions do not need to be atomic since there is no way that anything should be able to change the interrupt bit between the two operations. To my mind using ATOMIC_BLOCK is still the way to go.

I agree. I looked at the C code in the atomic.h file, and I don't understand how the heck it works. But I can't argue with the assembly code it spits out.

In a chunk of code which has interrupts turned off you should never call anything but the simplest/shortest functions/methods.

Any time you have interrupts turned off you risk other problems such as serial missing data and messing you the timers making a hash of servo/mills/micros/pwm etc. The time spent in ISR's or with interrupts turned off must always be kept as short as possible. So you don't run off and play with LCDs and the like.

Mark

holmes4:
In a chunk of code which has interrupts turned off you should never call anything but the simplest/shortest functions/methods.

Any time you have interrupts turned off you risk other problems such as serial missing data and messing you the timers making a hash of servo/mills/micros/pwm etc. The time spent in ISR's or with interrupts turned off must always be kept as short as possible. So you don't run off and play with LCDs and the like.

That's exactly why I think this whole question is chasing a will-o'-the-wisp.

In my code I shouldn't have to worry that some other guy did his coding badly - especially on a tiny thing like an Arduino.

I just can't envisage a situation where my code could find itself called by code from the middle of somebody else's CLI / SEI block.

And if you find yourself caught by a badly written Library the solution is to correct the library code.

...R