Attachinterrupt and delayMicroseconds

Hello
in a skecht I use the attachinterrupt to activate the buzzer and led.
If in this I assign a value to a variable

void blinkZ2 ()
     {
     Serial.println ("Zone2");
     delayMicroseconds (1000000);
     AStatoZ2 = 1;
      }

must it necessarily be declared volatile (volatile int AStatoZ2 = 0)?

If I check it volatile it gives me the following error
ERROR MSG

../wiring/inc/spark_wiring_cloud.h:121:24: note: candidate: 'static bool CloudClass :: _ variable (const char *, const int32_t &)'
   121 | static inline bool _variable (const char * varKey, const int32_t & var)

due to these lines in the void loop:

if (AStatoZ2 == 1) {digitalWrite (LedZona2, HIGH);}
             char dataZ2 [5];
             snprintf (dataZ2, sizeof (dataZ2), "% i", AStatoZ2);
             Particle.publish ("Status Z2:", dataZ2, PRIVATE, NO_ACK);

Second question as highlighted use delayMicroseconds there is an alternative to this delay since in the reference they recommend not to have long delay values .
Best
Valentino

This is a massive (!!!) no-no in an ISR. Never, absolutely never halt the code flow in an ISR for more than a couple of micro seconds.
You should also avoid Serial.print() inside an ISR as these are relatively heavy calls.
It may be OK for testing, but should never be used in production code.

Yes, it must be declared as volatile to prevent the optimizer from optimizing away actual data acquisition from RAM in the rest of your code where it otherwise would replace RAM access with internal register access for the sake of speed.

You also have an unwarranted space between % and i in this

snprintf (dataZ2, sizeof (dataZ2), "% i", AStatoZ2);

(besides many other superfluous spaces that don't really help readability :wink: - typically the parameter list of a function belongs to the function and is not separate from it as an extra space between would suggest)

To use a volatile variable with Particle.variable() you may want to read this thread

BTW, the suggestion in my second post in that thread works as expected. No need to use the deprecated syntax that is marked as the solution.
However, be aware that casting away volatile may result in unexpected results when the interrupt routine is called while the Particle.variable() is accessed.

Hello @ScruffR
Thanks to this point, if there is no possibility to use the delays within the ISR, I think back to the flow, perhaps using the functions of the Timer Software (not in the attach interrupts).
Best Regards
Valentino

This is not accurate. I see nothing in the posted code to suggest that volatile is needed for the AStatoZ2 variable and doing so just complicates other cases (like usage with Particle.variable).

For clarity, the “nothing” is the fact that the variable is not modified in loop(). Otherwise, a volatile declaration would be called for IMO.

The volatile keyword does nothing for the following snippet (even with the added modification of the ISR variable). If access to the variable needs to be atomic between the access and the update then volatile doesn’t help. And if it doesn’t need to be atomic then it likewise isn’t needed.

if(shared_isr_var)
{
  shared_isr_var = 0;
  call_a_func();
}

volatile is necessary for something like a memory-mapped register. It can also be necessary for certain usages of normal variables (ex. a tight while on the ISR variable without exiting loop() or otherwise calling into a function which would serve as a memory barrier). But the vast majority of times I see volatile thrown out as a solution/necessity it is either unnecessary or incorrect and actively harmful.

2 Likes

True, I should have been less absolute and more specific. For that specific code it's not required.
However, I didn't want to open that can of worms to dive into when it is an when it isn't given the other elephants in the room :wink:

Since I didn't take that snippet as the end of the story I just deemed it "saver" to advise for the use of volatile.

I may be wrong, but my feeling is that the adverse effects of "overusing" volatile where it's not required versus not using it when it'd be needed appear less problematic.
Typically not using it when needed will not be detected at compile time but bite you in the butt after delivery and makes debugging such ethereal bugs a nightmare.
On the other hand using it when not warranted is mostly a nuisance at compile time and may impact performance but rarely causes erratic behaviour.

BTW, instead of the that questionable type-casting I suggested in the other thread, since Particle.variable() now also allows for functions to be passed in this may be a better way to tackle the volatile issue @Tino52 was grappling with

volatile int x;
...
  Particle.variable("nonvolatileX", []()->int {return x;});

In that case the reference docs should be donated an elaborate application note when to use and when to avoid volatile.
e.g. the reference docs here do "advise" for its use where it clearly isn't needed going by above rationale.


(although I suspect the reason behind that was the same as mine - better "save" than sorry :wink: )

1 Like

Yeah, it serves no purpose in that example which I'll follow up on. You may have noticed, incorrect usage of volatile is sort of a personal pet-peeve. I've been hit too many times by code that misuses volatile but misses an actual concurrency issue for ISR/thread.

Quick, easy, and not harmful by itself (other than slightly less optimized code). But almost never correct and can serve to mask actual issues behind a false sense of confidence. And painful to see people waste time trying to shove volatile variables into Particle.variable.

An actual case where it matters would be a while-loop spinning on some signal variable set by an ISR. Not declaring volatile here could definitely end up with some infinite loop shenanigans depending on compiler optimizations.

while(!my_isr_signal_var)
{
  // do nothing, just wait
}
1 Like

Hello
first I see that it created discussion with the post.
I have translated everything since my shortcomings into English interesting discussion for a layman like me, it is never too late to learn.
I have prepared a small test skecht on the example of the original and as suggested by @Scruff so it works.
volatile int AStatoZ1 = 0;
Particle.variable (“Z1”, * (int *) & AStatoZ1);
It is clear that I will also do tests without the declaration of volatile.
For the suggestions of the delay time I do some tests then if positive I add them to the post.
For now, thanks and to @joel, @ScruffR,@peekay123 for the suggestions.
Valentino

1 Like

Thanks Tino for the kind words :+1:

In case you were sticking with volatile (although not needed here) or when you need to expose a volatile variable as Particle.variable() I’d now rather go with this (less questionable) approach

Particle.variable("Z1", []()->int {return AStatoZ1;});

which is shorthand (lambda) for this

inline int makeNonVolatileZ1() {
  return AStatoZ1;
}
...
  Particle.variable("Z1", makeNonVolatielZ1);
2 Likes

This topic was automatically closed 182 days after the last reply. New replies are no longer allowed.