I'm not sure if this is the best solution

I have two interrupts connected, one is an E_Stop button, the other is a Reset button. PORT L is setup as an output port. I’m using the watchdog timer to allow me to reboot the Arduino.

The E_Stop routine works quite well, when pressed the E_Stop ISR is called, writing ‘0’ to PORT L and waiting for the E_Stop button to be reset, then the Arduino reboots.

My question is with the Reset ISR, it writes ‘64’ to PORT L, which activates a reset relay that controls other devices, that reset relay needs to be activated for 10 seconds, before the Arduino reboots.

To do this the Reset ISR sets the Reset flag ‘true’ before returning to the main Loop, then because the Reset flag is ‘true’, the program exits the while loop and enters an exit loop that does nothing but update the watch dog timer for 8 seconds, then it enters another loop that does not reset the Watch Dog Timer, so the Arduino resets.

This process works, but I feel it is fraught with danger, when returning from the Reset ISR, before exiting the While loop, things may turn on as the program progresses. Is there a better way to guarantee that nothing happens during the reset process.

#include <Controllino.h>
#include <SPI.h>
#include <avr/wdt.h>
 
bool Reset = false;                            //flag for reset button
const long r = 8000;                           
unsigned long CurrentR = 0;
unsigned long PreviousR = 0;
const byte E_StopPin = CONTROLLINO_IN0;        //interrupt pin connected to E_Stop button
const byte R_SetPin = CONTROLLINO_IN1;         //interrupt pin connected to Reset button
int foo = 255;     

void setup() {
  wdt_disable();                              //watch dog timer disable
  DDRL = DDRL | B11111111;                    // Set  Port L as output
  wdt_enable(WDTO_2S);                        //watch dog timer enable with 1 second renew or reboot
 attachInterrupt(5, E_Stop,LOW);              //attach interrupt pin, name ISR to call, trigger on LOW
 attachInterrupt(4, R_Set,LOW);               //attach interrupt pin, name ISR to call, trigger on LOW
}


void loop() {                                //Main loop just runs rubbish for testing, counts down from 255 and back up repeatedly, showing as binary on Port L LEDs
  while (Reset != true){                     //loop runs as long as Reset flag remains false
    PORTL = foo;
while (foo >= 0){
     wdt_reset();                            //renew watch dog timer
    if (Reset == true){
      Restart();
    }
     delay(100);                             
    foo -= 1;
    PORTL = foo;
   }
while (foo <= 255){
  wdt_reset();                               //renew watch dog timer
     delay(100);
    foo += 1;
    PORTL = foo;
 }
}
 Restart();                                //if Reset flag is true Restart is called
  }

void Restart(){
  PORTL = 64;                              //set bit 6 of Port L HIGH, and all other bits LOW activating the reset relay
  PreviousR = millis();
  CurrentR = millis();
  while (CurrentR - PreviousR <= r){      //keep reset relay active for 8 seconds
  CurrentR = millis(); 
   wdt_reset(); 
  }                
  while(true){}                          //do nothing loop until WDT times out and Processor reboots      
  }



//RESET ISR
void R_Set(){
 PORTL = 64;                              //set bit 6 of Port L HIGH, and all other bits LOW activating the reset relay
 digitalWrite(CONTROLLINO_D3,HIGH);       //turn on Reset status LED 
int c = digitalRead(R_SetPin);               
  while (c == LOW){                       //This loop keeps output Port L LOW until Reset Button is released
  c = digitalRead(R_SetPin); 
  wdt_reset();                            //renew watch dog timer inside ISR to maintain R_Set LED 
  } 
  Reset = true;
  
 }


 

//E_Stop ISR
void E_Stop() { 
   PORTL = 0;                            //set Port L LOW first, to shut everything down
  digitalWrite(CONTROLLINO_D0,HIGH);     //turn on E_stop status LED  
 int a = digitalRead(E_StopPin);           
  while (a == LOW){                      //This loop keeps output Port L LOW until E_Stop Button is reset
  a = digitalRead(E_StopPin);
  wdt_reset();                           //renew watch dog timer inside ISR to maintain E_Stop Status LED 
  } 
  while(true){}                          //do nothing loop until WDT times out and Processor reboots
}

In principle, you are simply using two buttons to control two external devices with a timed sequence of actions. There is no need to force a restart of the Arduino for that and you are not shutting it down to conserve power or have I missed something ?

I’d start with the desired action(s) to be performed (a) when Stop is pressed and (b) when Reset is pressed, then work out the cleanest way of achieving that.

The point is, I "DO" want to restart the Arduino. As pointed out in the code, it is only running a simple program loop for testing, the purpose at the moment is to restart the Arduino as I stated.

I "DO" want to restart the Arduino.

Why ?

rameses32:
The point is, I "DO" want to restart the Arduino. As pointed out in the code, it is only running a simple program loop for testing, the purpose at the moment is to restart the Arduino as I stated.

Then there surely is a better way of doing it. Which arduino are you using ?
It is quite usual to put the Arduino in a power down state to conserve battery power and then wake it with an interrupt. It is quite unusual as part of the program logic to force a crash and restart to get the Arduino back to a known state. But maybe you have a good reason and I'd be curious to hear it.

rameses32:
I have two interrupts connected, one is an E_Stop button,

That suggests to me an Emergency Stop and if there is any risk of injury to a person (or damage to equipment) the emergency stop should not be entrusted to a microprocessor.

And I agree with others who have said that if the Arduino needs to reset there is a problem with the program design.

...R

The safety side of the E-Stop and other functions are handled externally by properly configured control circuitry, and do not rely on the CONTROLLINO in any way. I am simply configuring the software side of the E-Stop function. Having read the comments I am looking into a different approach that does not require rebooting the Arduino.

Robin2:
That suggests to me an Emergency Stop and if there is any risk of injury to a person (or damage to equipment) the emergency stop should not be entrusted to a microprocessor.

And I agree with others who have said that if the Arduino needs to reset there is a problem with the program design.

...R

This is a good example of "So bad, it's not even wrong."

You have a high-level programming language, comprised of C++ and the Arduino standard functions. Then you've added another layer on top of that with Controllino. But you never use Controllino except to use two of its pin name aliases, which is pretty well covered by the Arduino aliases in my opinion.

Then you are using a low-level technique of direct port manipulation. With all of those high-level layers also trying to control the output pins, you are going to cause yourself problems.

You also quickly forget the pin names and use magic numbers anyway...

const byte E_StopPin = CONTROLLINO_IN0;        //interrupt pin connected to E_Stop button

const byte R_SetPin = CONTROLLINO_IN1;        //interrupt pin connected to Reset button
...
  attachInterrupt(5, E_Stop, LOW);            //attach interrupt pin, name ISR to call, trigger on LOW
  attachInterrupt(4, R_Set, LOW);              //attach interrupt pin, name ISR to call, trigger on LOW

Look at the digitalPinToInterrupt() function. That will save using those magic numbers.

Then you're using interrupts for buttons pressed by humans. That should never be necessary. Interrupts are for special time-sensitive functions like shaft encoders and serial data. Responding to a button-push in a millisecond is not time-sensitive in the same way. If you're 'forced' to use interrupts then you're doing something else wrong. Namely, over-use of the delay() function. Re-write without using delay().*

Relying on the watchdog timer for normal functioning of your program? That's a valid technique for extreme low-power applications, like when you're trying to get it to run more than a year on a single coin-cell battery. Or trying to use an ATTINY with not enough pins for the job. But if you aren't dealing with those kinds of problems then the watchdog should not be part of the normal operation. If you need to "reset" then just call setup() again. There's nothing single-use about that function - it can be called from any part of the program.

*Well, actually, your "reset delay" is a good candidate for using the delay() function. At that time, you want the Arduino doing absolutely nothing else except holding that value on the output pins and you don't care if there's any other inputs during that time.

Nice Complisult, come up with it by yourself?

MorganS:
This is a good example of “So bad, it’s not even wrong.”

I changed most of the sketch, taking into account all of the suggestions made. Thank you everybody for your help, it is appreciated. The sketch works exactly as I was hoping, my concern now is that I am running most of the code in a function and in that function I am checking to see if the reset or reboot flags have been set before writing to the output port. Just wondering if there was a better way to go about this?

#include <Controllino.h>
#include <SPI.h>
bool ReBoot = false;                           //reboot condition flag
bool Reset = false;                            //flag for reset button                           
int foo = 255;
     
/*-------------------------------------------------------------------------------------------------------------------------------
                                                 Setup 
 ------------------------------------------------------------------------------------------------------------------------------*/
 
void setup() { 
  delay(1000);
  DDRL = DDRL | B11111111;                    // Set  Port L as output
  PORTL = 0;                                  //clear output port
 attachInterrupt(digitalPinToInterrupt(CONTROLLINO_IN0), E_Stop,LOW);              //attach interrupt pin, name ISR to call, trigger on LOW
 attachInterrupt(digitalPinToInterrupt(CONTROLLINO_IN1), R_Set,LOW);                //attach interrupt pin, name ISR to call, trigger on LOW 
}

/*-------------------------------------------------------------------------------------------------------------------------------
                                                 Loop
 ------------------------------------------------------------------------------------------------------------------------------*/
void loop() {                               
    if (ReBoot == true || Reset == true){     //call reboot function if ReBoot or reset flags have been set
      Reboot();
    }  
  Bar();                                     //everything is fine, go do some stuff, have a ball, meet new friends, live your life
}


/*-------------------------------------------------------------------------------------------------------------------------------
                                                 Bar 
 ------------------------------------------------------------------------------------------------------------------------------*/
void Bar(){                               //most of the program runs here because I want the ability to start over if E-Stop or Reset buttons are activated
     Restart();                           //check reset flag, if false, returns and program continues
      if (ReBoot == true){                //check if reboot needed
         return;                          //time to make like a shepherd and get the flock out of here
       }
    PORTL = foo;                          //output to control port
  while (foo >= 0){                       //count down in binary on Port L for testing
       delay(100);                        //I know, I know delay is bad mojo, only here for testing
       foo -= 1;                          //foo just lost one life and that 256 sided dice sucks to find numbers on
       Restart();                         //check reset flag, if false, returns and program continues
        if (ReBoot == true){              //check if reboot needed
         return;                          //tap thoes heels Dorthey, cause you goin home
      }
    PORTL = foo;                          //output to control port
   }
  while (foo <= 255){                     //count up in binary on Port L for testing
      delay(100);                         //I know, I know delay is bad mojo, only here for testing
      foo += 1;                           //foo just gained a life, way to go foo
      Restart();                          //check reset flag, if false, returns and program continues
       if (ReBoot == true){               //check if reboot needed
        return;                           //why are you still here, get out!!!
     }
    PORTL = foo;                          //output to control port
  }
}




/*-------------------------------------------------------------------------------------------------------------------------------
                                                 R_Set ISR
 ------------------------------------------------------------------------------------------------------------------------------*/

void R_Set(){
  PORTL = 64;                              //set bit 6 of Port L HIGH, and all other bits LOW activating the reset relay
  digitalWrite(CONTROLLINO_D3,HIGH);       //turn on Reset status LED 
  int c = digitalRead(CONTROLLINO_IN1);               
   while (c == LOW){                       //This loop keeps output Port L LOW until Reset Button is released
     c = digitalRead(CONTROLLINO_IN1); 
    } 
  Reset = true;
}

 /*-------------------------------------------------------------------------------------------------------------------------------
                                              Restart 
 ------------------------------------------------------------------------------------------------------------------------------*/
void Restart(){
  if (Reset == true){                        //checks the reset flag
    PORTL = 64;                              //set bit 6 of Port L HIGH, and all other bits LOW activating the reset relay
    delay(8000);                             //do nothing at all for 8 seconds
   ReBoot = true;                            //set the reboot flag             
  }
}

/*-------------------------------------------------------------------------------------------------------------------------------
                                                 E_Stop ISR
 ------------------------------------------------------------------------------------------------------------------------------*/

void E_Stop() { 
   PORTL = 0;                             //set Port L LOW first, to shut everything down
   digitalWrite(CONTROLLINO_D0,HIGH);     //turn on E_stop status LED  
   int a = digitalRead(CONTROLLINO_IN0);           
    while (a == LOW){                     //This loop keeps output Port L LOW until E_Stop Button is reset
      a = digitalRead(CONTROLLINO_IN0); 
    }  
  ReBoot = true;                          //set the reboot flag
}


/*-------------------------------------------------------------------------------------------------------------------------------
                                                Reboot 
 ------------------------------------------------------------------------------------------------------------------------------*/

void Reboot(){                        //this function resets all global variables, clears any flags and turns off any Indicator LEDs
 ReBoot = false;
 digitalWrite(CONTROLLINO_D3,LOW);
 digitalWrite(CONTROLLINO_D0,LOW);
 Reset = false;
 foo = 255;  
}

Generally speaking, you do not restart the arduino. You write a sketch that does what you need it to do, including going back to a home or zero state under certain conditions.

I didn't look at the earlier code but I have had a quick look at the code in Reply #9. I reckon @MorganS was very restrained.

You should NOT be writing code that requires an Arduino to reset. RESET should only be needed if your code stops working - and it obviously can't be triggered by non-working code. You can use the WatchdogTimer to check for non-functioning code - but the grand plan should be to ensure the WDT is never needed.

...R
Planning and Implementing a Program