Detecting when interrupts are enabled

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

Robin2:

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.

You are both missing the point of my original post! The third party library seems to be failing because it has very time critical code and even though my interrupt handlers have been pared down to the very minimum the fact that I am handling about 5000 per second seems to cause the problem with the library. I am not calling somebody else's code from within a CLI / SEI block and this this whole question is most definitely not chasing a will-o'-the-wisp. My solution to the problem is to modify the third party library so that it puts a CLI / SEI block round the critical code sections in the library; something that should have been done by the writers of the library.

The 5000 per second interrupt handler is just incrementing a counter and then once a second or so it sets a flag. I can't make it any shorter than that. My code happily deals with the interrupt latency caused by the CLI / SEI block so I am not affected by it. The third party library now works without problems though before publishing the changes I do need to try to minimise the library code that needs to be placed within the CLI / SEI block(s).

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

...R

Which is exactly what I am doing!!!!!!!!

@stowite we are not missing the point - the bug was NOT in the lib it was in your code when it called the part of the lib!

You do not call fuctions/methods from with ISR's or critical sections of code. The fault was with you. You tried to do to much with in the sei/cli block or ISR!

Mark

holmes4:
@stowite we are not missing the point - the bug was NOT in the lib it was in your code when it called the part of the lib!

You do not call fuctions/methods from with ISR's or critical sections of code. The fault was with you. You tried to do to much with in the sei/cli block or ISR!

Mark

One last time - I am not calling functions/methods from within ISR's or critical sections of code. I am only calling the Liquid Crustal library methods from outside of the ISR. My ISR do very very little just enough to set flags which are processed in the loop() method.

If the critical section is only called from outside of an ISR, cli/sei is fine.
If the critical section is only called from inside an ISR, nothing is needed
If the critical section can be called from both an ISR and from outside an ISR,
then you need the code that saves and restores SREG - look at the implementation
of digitalWrite().

Also if a critical section can be called from within another critical section, the save/restore
SREG method is needed.

Interrupts are enabled unless you are in an ISR or a critical section (though it is
possible to re-enable interrupts halfway through an ISR, this requires the ISR
to be coded as re-entrant, using critical sections).

stowite:
You are both missing the point of my original post! The third party library seems to be failing because it has very time critical code and even though my interrupt handlers have been pared down to the very minimum the fact that I am handling about 5000 per second seems to cause the problem with the library.

I don't see how we can be missing the point when you have never said what you are actually trying to do.

Post your code so that we have the same information in front of us as you have.

Maybe the code (library) that you would like to include in your sketch just isn't fast enough for 5000 per second?

...R