I see that the interrupts() and nointerrupts() definitions were added to wiring.h for 0012. This is a step in the right direction but they suffer from a significant defect. The problem is that they don't nest properly. For example, if one uses these in a function that is called with interrupts disabled, interrupts will be unexpectedly enabled upon return.
I propose adding two new definitions to address this issue:
With these definitions, the example above could be rewritten as
begin_atomic
<code that needs to be executed atomically>
end_atomic
Some may comment that this syntax is dissimilar to normal C/C++ code given that the braces are hidden inside the definitions for begin_atomic and end_atomic. This can be partially remedied by simply adding some extra braces:
begin_atomic {
<code that needs to be executed atomically>
} end_atomic
An extra semicolon can be added after end_atomic, if desired, to make it look a little more "normal".
Here is an improved version of the atomic block definitions. These changes eliminate the code added to wiring.c and makes the SREG restoration code be generated in line, improving efficiency.
I don't think it really makes sense to add new functions for this. We might consider creating a global variable that calculates the "nesting" of interrupt disablement (i.e. is incremented by calls to noInterrupts() and decremented by calls to interrupts()). Then we would only re-enable interrupts when the counter hit 0. Thoughts?
I don't really see that disabling interrupts is a functionality a) needed very often, and b) rarely needed by the typical arduino user.
If this causes any resource utilization (code space due to linking, or RAM due to a global variable) I really don't see the benefit to including it in the core.
On the surface mellis's proposal sounds like a good idea, but on reflection I don't think it would solve the problem Don Kinzer refers to. Say you had a function that you wanted to be callable from an interrupt handler, and this function was bracketed with nointerrupts()/interrupts(). Even with this "nesting" counter mechanism, the call the interrupts() would (erroneously) re-enable interrupts before returning to the handler.
Is this really a "significant defect"? If you think of interrupts() and nointerrupts() simply as easier-to-remember aliases for cli and sei, then interrupts() is not really any more "defective" than sei().
If this causes any resource utilization [...] I really don't see the benefit to including it in the core.
intDisable() and intRestore() are just macros. If you don't use them you'll not see any effect of their presence in wiring.h.
Even with the "nesting" variable mechanism, the call the interrupts() would (erroneously) re-enable interrupts.
The point is that the use of interrupts() should be discouraged precisely because of its unconditional nature and there are many cases where you probably don't want interrupts unconditionally re-enabled.
Using intDisable() and intRestore() in a function instead of nointerrupts()/interrupts() makes it more robust and renders it callable from any other place in the code - even from within an ISR.
mikalhart: depth counting might not solve the problem of calling the function from an interrupt handler, but it would solve the problem of nested calls to functions that both need interrupts disabled. In any case, though, I agree that it's not any more defective than sli(), so maybe it's fine.
Don: I think the intDisable() and intRestore() macros are too confusing for Arduino users. By the time you understand what they're doing, you might as well just save SREG directly.
mellis:
Your argument for rejecting the intDisable() and intRestore() functions based on them being too hard to understand is IMO flawed. You are making that statement under the unspoken assumption that the Arduino environment is by design an entry level and learning platform. I agree with this latter assumption.
I think that an 'easy to pick up' and 'learning' platform should hide complex or difficult concepts from beginner. So it makes sense to either: leave interrupts related functions out altogether or point to them in an advanced howto/reference.
Making interrupts() and nointerrupts() available to the user is not good because: it requires the user to understand something about these 'interrupts things' and it implements disabling/enabling interrupts the wrong way (see comments about nesting).
If the concept of interrupts has to be exposed to the user, it should be done so that it is "correct". Better fix it now rather then have to stick with broken abstractions in the default library and reference.
What is easier for the user: learning interrupts() and nointerrupts() and understanding how to avoid their pitfalls or learning intDisable() and intRestore()? It is also easier to document intDisable() and intRestore() because you do not have to launch into explaining what could go wrong with nesting and how to avoid it.
I agree with the previous post. Moreover, I think that it is easier still to introduce the concept of atomicity (what it means, why it's important) and then show how the atomic block construct (described earlier in this thread) makes it easy to implement. This leaves the intracasies of interrupt()/nointerrupt() or, better still, intEnable()/intDisable() to be introduced as an advanced concept for those who need such special capability.