Variable wont work unless global

This simple time delay routine works fine as long as count is a global variable. If I change it to a local variable, the function breaks. What am I doing wrong?

I am using an MKRZero.

This works

int count;
#define iterations 0x12A0

void delaynoISR(int ms)
{
  do
  {
    count = iterations;
    do
    {
      count--;
    } while(count);
  ms--;
  } while(ms);
}

void setup()
{
  pinMode(2, OUTPUT);
}

void loop()
{
  delaynoISR(1000);
  digitalWrite(2, HIGH);
  delaynoISR(1000);
  digitalWrite(2, LOW);
}

This only gives me one pass

// int count;
#define iterations 0x12A0

void delaynoISR(int ms)
{
  do
  {
    int count = iterations;
    do
    {
      count--;
    } while(count);
  ms--;
  } while(ms);
}

void setup()
{
  pinMode(2, OUTPUT);
}

void loop()
{
  delaynoISR(1000);
  digitalWrite(2, HIGH);
  delaynoISR(1000);
  digitalWrite(2, LOW);
}

When you declare a variable inside a set of curly braces, it's scope is limited to within those braces, not outside
Try moving the declaration (int count) right after
void delaynoISR(int ms) {
It's scope will be the whole function, but not global.

1 Like

I tried that before, but it did not help.
To be thorough, I tried again. This code did not work:

// int count;
#define iterations 0x12A0

void delaynoISR(int ms)
{
int count;
  do
  {
    count = iterations;
    do
    {
      count--;
    } while(count);
  ms--;
  } while(ms);
}

void setup()
{
  pinMode(2, OUTPUT);
}

void loop()
{
  delaynoISR(1000);
  digitalWrite(2, HIGH);
  delaynoISR(1000);
  digitalWrite(2, LOW);
}

What does delaynoISR mean?

Why do you want to recreate another blocking delay?

The Arduino function delay() uses a timer, and will not work with interrupts disabled. This is just a clean, quick and dirty way for me to show hard faults, both inside and outside of ISRs. I use it to flash an LED to indicate a hard fault. The rate of flashing indicates which fault.

How about this code to make your IO-pin switch high/low?

unsigned long MyTestTimer = 0;                   // Timer-variables MUST be of type unsigned long
const byte    OnBoard_LED = 2;

void setup() {
  Serial.begin(115200);
  Serial.println("Setup-Start");
}

void loop() {
  BlinkHeartBeatLED(OnBoard_LED,1000);
}

// easy to use helper-function for non-blocking timing
boolean TimePeriodIsOver (unsigned long &startOfPeriod, unsigned long TimePeriod) {
  unsigned long currentMillis  = millis();
  if ( currentMillis - startOfPeriod >= TimePeriod ) {
    // more time than TimePeriod has elapsed since last time if-condition was true
    startOfPeriod = currentMillis; // a new period starts right here so set new starttime
    return true;
  }
  else return false;            // actual TimePeriod is NOT yet over
}


void BlinkHeartBeatLED(int IO_Pin, int BlinkPeriod) {
  static unsigned long MyBlinkTimer;
  pinMode(IO_Pin, OUTPUT);
  
  if ( TimePeriodIsOver(MyBlinkTimer,BlinkPeriod) ) {
    digitalWrite(IO_Pin,!digitalRead(IO_Pin) ); 
  }
}

best regards Stefan

1 Like

You should never disable interrupts for so long.

You should never use such code inside an ISR.

But this is only my humble opinion and experience.

1 Like

There's a very good chance that function could be completely optimized away by the compiler.

I like that code!

For the most part, I agree. But, if I encounter a hard fault, then the software is not working anyway. This is just for debugging.

Why?

My malfunctioning function is still called. It just does not iterate the way I expect that it should; it only makes one pass.

Because the whole thing only manipulates data that is unreachable for the rest of the program.

2 Likes

Then perhaps this is my problem. A .lst file would be very helpful right now. I don't think I can generate one with the Arduino IDE.

Put another way, the compiler can detect that the function has no observable effect on the execution flow of the program.

1 Like

This will be optimized away, since it has no external effect.


void loop()
{
    2108:       b510            push    {r4, lr}
  delaynoISR(1000);
  digitalWrite(2, HIGH);
    210a:       2101            movs    r1, #1
    210c:       2002            movs    r0, #2
    210e:       f000 fa8f       bl      2630 <digitalWrite>
  delaynoISR(1000);
  digitalWrite(2, LOW);
    2112:       2100            movs    r1, #0
    2114:       2002            movs    r0, #2
    2116:       f000 fa8b       bl      2630 <digitalWrite>
}
    211a:       bd10            pop     {r4, pc}
1 Like

Without a lst file I cannot be sure, but I am almost convinced that you are correct: the function is being optimized out of existence.

This change works:

int delaynoISR(int ms)
{
  static int count;
  do
  {
    count = iterations;

This change does not:

int delaynoISR(int ms)
{
  do
  {
    static int count = iterations;

Adding some GPIO interaction within the function also fixes it.

I also tried return(ms); and return(count); those did not work.

I am going to call this problem solved, and thank you for your time!

I wonder: is there a way to force the compiler to compile a given block of code?

It DID compile it. It just decided it wasn't useful and threw it away.
You can do

   volatile int count;

Which essentially says" "modifying count has undetectable-to-the-compiler side effects, so you should really do it, even if it seems silly."

1 Like

This works:

int delaynoISR(int ms)
{
  do
  {
    volatile int count = iterations;
    do

I suspect that this is cleaner than the way I did it using static.

How did you generate this listing?

void loop()
{
    2108:       b510            push    {r4, lr}
  delaynoISR(1000);
  digitalWrite(2, HIGH);
    210a:       2101            movs    r1, #1
    210c:       2002            movs    r0, #2
    210e:       f000 fa8f       bl      2630 <digitalWrite>
  delaynoISR(1000);
  digitalWrite(2, LOW);
    2112:       2100            movs    r1, #0
    2114:       2002            movs    r0, #2
    2116:       f000 fa8b       bl      2630 <digitalWrite>
}
    211a:       bd10            pop     {r4, pc}

Clean and dirty, nice!

a7

1 Like