How to get rid of goto statement?

This is my sketch (given below) where I have to use goto statement to reach at the back. I have been trying to get rid of the goto statement, but I am not yet successful! Any help would be appreciated.

#include<avr/sleep.h>
int brightness_SEL = 0;

void setup()
{
  Serial.begin(9600);
  pinMode(2, INPUT_PULLUP);
  SMCR |= (1 << SM1); //Power-down Mode
  attachInterrupt(digitalPinToInterrupt(2), wakeISR, LOW);  //must be level mode
}

void loop()
{
L1: unsigned long currentMillis = millis();
  while (millis() - currentMillis < 2000)//test interval
  {
    if (brightness_SEL == 0)
    //if(vbattread <= 512 && brightness_SEL==0 && analogRead(Vusb_out) <= 899){}
    {
      sleepMCU();
      bitClear(SMCR, SE);
      Serial.println("Wake up from LED off.");
      brightness_SEL = 1;  //testing
      goto L1;  //my way; trying to get rid of goto statement
    }
  }
  //code to turn of all LED activities
  sleepMCU();
  bitClear(SMCR, SE);
  Serial.println("Wake up from 2-sec delay.");
}

void sleepMCU()
{
  bitSet(SMCR, SE); //sleep enable bit must be HIGH before sleep_cpu() instruction\
  sleep_cpu();  //asm: sleep; the MCU sleeps
}

void wakeISR()
{
}
#include<avr/sleep.h>
int brightness_SEL = 0;

void setup()
{
  Serial.begin(9600);
  pinMode(2, INPUT_PULLUP);
  SMCR |= (1 << SM1); //Power-down Mode
  attachInterrupt(digitalPinToInterrupt(2), wakeISR, LOW);  //must be level mode
}

void loop()
{
  unsigned long currentMillis = millis();
  while (millis() - currentMillis < 2000)//test interval
  {
    if (brightness_SEL == 0)
      //if(vbattread <= 512 && brightness_SEL==0 && analogRead(Vusb_out) <= 899){}
    {
      sleepMCU();
      bitClear(SMCR, SE);
      Serial.println("Wake up from LED off.");
      brightness_SEL = 1;  //testing
      currentMillis = millis();
    }
  }
  //code to turn of all LED activities
  sleepMCU();
  bitClear(SMCR, SE);
  Serial.println("Wake up from 2-sec delay.");
}

void sleepMCU()
{
  bitSet(SMCR, SE); //sleep enable bit must be HIGH before sleep_cpu() instruction\
  sleep_cpu();  //asm: sleep; the MCU sleeps
}

void wakeISR()
{
}

Essentially the same as


return;
2 Likes

@kolaha and @jremington
Interesting! Give me sometimes to validate your proposed solutions.

In your case it's just starting loop over. So just return and it will end loop.

Yes, but what about

L1: unsigned long currentMillis = millis();

I cant test it from under the umbrella, so in the meantime can someone say that this declaration works? If goto leads execution to L1, will currentMillis be updated, as it surely would be if the loop was execute again from the top?

It looks like an initialisation and I just don't know what happens in case it is run cross twice without having gone out of scope.

a7

I just don't know what happens

Do you think this would be a problem (assuming that the compiler doesn't remove the call)?

void loop() {
unsigned long currentMillis = millis();
unsigned long currentMillis = millis();
...
void setup() {
  int x = 5;
  int x = 5;
  Serial.begin(9600);
  Serial.print(x);

}
void loop(){}
error: redeclaration of 'int x'

I'm guessing it would be an error. No, I'm not guessing, I'm assuming it would be the same error as @Delta_G's.

But I don't see that it helps answer the question, is the variable re-initialised or not?

Still not where I could just run a test.

a7

Even if you test it and see one particular answer, it dones't mean that the behavior is defined. WHat does the standard say?

All that being said, I think it is apparent in the OP's code that the INTENT was to get a new value for currentMillis. Otherwise they could have just done nothing and stayed in the while loop. Simply deleting the goto line would solve that. So while we may argue what actually happened with this code, I think the INTENT of the code is clear.

And in that case the return statement is the way to go.

So, according to this example (the second goto), a jump to a point before the lifetime of an object starts destroys the object in question. It seems to me that this means the following is also permitted.

l1:
  int x {10};
  Serial.println(x++);  // Prints `10` over and over again.
  goto l1;

EDIT: With your edit I understand.

1. I have done the following experiment which has established that the solutions provided by @kolaha in post #2 and @jremington in post #3 are the valid replacement of goto L1 instruction.

2. In my experiment, the MCU will go into sleep after glowing L (built-in LED of UNO) for 10-sec; however, if Battery voltage (which I have simulated using a switch at DPin-4) goes LOW before the elapse of this 10-sec time, the MCU will also go into sleep.

3. This is the Flow Chart (Fig-1) of the process, which I have not optimized for the shake of clarity.


Figure-1:

4. This is the test sketch which I have also not optimized for the shake of clarity.

#include<avr/sleep.h>

void setup()
{
  Serial.begin(9600);
  SMCR |= (1 << SM1); //Power-down Mode
  pinMode(13, OUTPUT);  //LED at DPin-13
  pinMode(2, INPUT_PULLUP);  //MCU wake up trigger
  pinMode(4, INPUT_PULLUP);  //Battery status at DPin-4
  digitalWrite(13, HIGH);  //LED is ON
  attachInterrupt(digitalPinToInterrupt(2), wakeUpIsr, LOW);
}

void loop()
{
 L1: unsigned long currentMillis = millis();
  while (millis() - currentMillis < 10000) //test interval
  {
    if (digitalRead(4) == LOW)
    {
      digitalWrite(13, LOW);  //LED is Off
      sleepMCU();  //MCU sleeps
      Serial.println("WakeupBatt.");    //MCU is wake up
      bitClear(SMCR, SE); 
      while (digitalRead(4) != HIGH) //checking if Battery volt ok
      {
        ;
      }
      digitalWrite(13, HIGH); //LED is On
      currentMillis = millis();//return;//goto L1; all three codes work
    }
  }
  digitalWrite(13, LOW);  //LED is Off
  sleepMCU();  //MCU sleeps
  Serial.println("Wakeup10sec.");   //MCU is wake up
  bitClear(SMCR, SE);  
  while (digitalRead(4) != HIGH) //checking if Battery volt ok
  {
    ;
  }
  digitalWrite(13, HIGH); //LED is ON
}

void sleepMCU()
{
  bitSet(SMCR, SE);
  sleep_cpu();
}

void wakeUpIsr()
{

}

5. Serial Monitor Output:

Wakeup10sec.
Wakeup10sec.
WakeupBatt.
Wakeup10sec.
WakeupBatt.

6. Thanks to you all for helping me to convert my ignorance into knowledge.

7. Question:
The return statement is used at the end of a sub-routine for reverting to the main line program (the calling program). In my sketch, there is no sub-routine that has been called upon -- then how does return bring the control at the beginning of the loop() function?

8. Observation:
I have some reservations with the solution of @kolaha in post#2; where, the same instruction (currentMillis = millis()) is being executed twice to arrive at the beginning of the loop() function. If I delete this instruction: currentMillis = millis(), then the timing is not 10-sec (the test interval); rather it is erratic.

Of course there is. The hidden function main() calls loop() repeatedly, which is why "return" works.

1 Like

So what is wrong with using a GOTO?

Same as falling into a black hole - spaghetti-fication

That means that the return statement brings the control next to the exit point of the main() function. After that main() function calls upon the loop() function and program execution begins at label L1 (sketch of post #14). Also, at the end of loop() function the control goes back to the main() function.

If the above is the case, then the solution of post #2 @kolaha needs more clarification as to how his solution brings the control at label L1.

Arduino main function code

#include <Arduino.h>

int main(void)
{
    init();

#if defined(USBCON)
    USBDevice.attach();
#endif
    
    setup();
    
    for (;;) {
        loop();
        if (serialEventRun) serialEventRun();
    }
        
    return 0;
}
1 Like

is not.
if (brightness_SEL == 0)
ends and
while (millis() - currentMillis < 2000)
will continue, but after
Serial.println("Wake up from 2-sec delay.");
loop() starts over and
unsigned long currentMillis = millis();
is executed.