Using Toggle switch to activate loop - Can this be improved?

I’ve wired a toggle switch to digital input, intending it as an on off switch for my little robot.

void loop (){
  switchstate = digitalRead(onoffpin);
if (switchstate == HIGH){
  
  sensestuff();
  
  if (motoron == false && (IRVoltageR > 2.5 && IRVoltageL > 2.5)){
    motorstart();
  }
  
  if (motoron == true && (IRVoltageR < 2.5 || IRVoltageL < 2.5)) {
    motorstop();  
  }
}
else {
  motorstop();
}

}

As you can see it governs a loop within the void loop. I wonder if this is inefficient, polling my on off switch pin every cycle. If there is a more elegant method to accomplish this I look forward to suggestions.

I haven’t used interrupts before but I think I can see how one might be used to end/pause the robot when the switch goes off. Not sure how it might work for starting the program in the first place.

If possible, I’d love to have this switch prevent void setup from running until the switch is on. (Other than splicing the switch into my barrel connector for the power supply.)

Thanks for reading!

If it works it works.

Suggest you look at a change in state rather than a level when it comes to switches.

See the change in state example in the IDE.

Also, for manually operated mechanical switches, scanning them once every 50ms should be adequate.

you might consider making it an if/else since only one case is possible (or are both possible)?

rather than make the bulk of loop() conditionally executed, you can

void loop (){
  if (LOW = digitalRead(onoffpin))  {
    motorstop();
    return;
  }
  
  sensestuff();
  
  if (motoron == false && (IRVoltageR > 2.5 && IRVoltageL > 2.5)){
    motorstart();
  }
  
  if (motoron == true && (IRVoltageR < 2.5 || IRVoltageL < 2.5)) {
    motorstop();  
  }
}

larryd:
If it works it works.

Suggest you look at a change in state rather than a level when it comes to switches.

See the change in state example in the IDE.

Also, for manually operated mechanical switches, scanning them once every 50ms should be adequate.

Thanks! Although doesn’t occur to me how to scan the switch every 50ms while the rest of the program runs as fast as possible.

An example that includes scanning the switch every 50ms.

 //Simple BWD BlinkWithoutDelay examples
//
//Timer variables used
unsigned long currentMillis;
unsigned long pin13Millis;
unsigned long pin12Millis;
unsigned long SwitchMillis;
 
//if these are not changed in the sketch, they can be const
unsigned long debounceMillis = 50UL;  //50ms
unsigned long ledOnTime      = 500UL; //500ms seconds
 
byte lastSwitchState = HIGH;
byte buttonState     = HIGH;
 
//enable/disable flags
boolean flag13 = true;
boolean flag12 = false;
 
const byte Switch = 2; //pushed = LOW
 
//**********************************************************************
 
void setup()
{
  Serial.begin(9600);
 
  digitalWrite(13,LOW);
  pinMode(13, OUTPUT);
  
  digitalWrite(12,HIGH);  //Turn LED on for 5 seconds
  pinMode(12, OUTPUT);
 
  pinMode(Switch, INPUT_PULLUP); //pushed = LOW
 
} //  >>>>>>>>>>>>>> E N D  O F  s e t u p ( ) <<<<<<<<<<<<<<<<<
 
void loop()
{
  //save the current time
  currentMillis = millis();
 
  //*************************************
  //Heartbeat LED
  //Toggle LED on and off. Helps show if there is blocking code
  if (flag13 == true && currentMillis - pin13Millis >= ledOnTime)
  {
    pin13Millis = millis();            //re-initialize Timer
    digitalWrite(13,!digitalRead(13)); //toggle LED condition
  }
 
  //*************************************
  if (flag12 == true && currentMillis - pin12Millis >= 5*1000UL)
  {
    //Turn off pin 12
    digitalWrite(12,LOW); //Turn off LED
    flag12 = false;       //disable timing
  }
 
//*************************************
//is it time to check the switches?
if (currentMillis - SwitchMillis >= debounceMillis)
{
//code here runs every debounceMillis ms
SwitchMillis = millis(); //re-initialize Timer

//go and check the switches
checkSwitches();   
}
 
  //*********************************
  //put other non-blocking stuff here
  //*********************************
 
} //  >>>>>>>>>>>>>> E N D  O F  l o o p ( ) <<<<<<<<<<<<<<<<<
 
 
//======================================================================
//                      F U N C T I O N S
//======================================================================
 
 
//****************** c h e c k S w i t c h e s ( ) *********************
//switches are checked every debounceValue milli seconds
//no minimum switch press time is validated with this code (i.e. No glitch filter)
void checkSwitches() 
{
  //re-usable for all the switches 
  boolean thisState;   
 
  //check if Switch1 has changed state
  thisState = digitalRead(Switch1);
  if (thisState != lastSwitch1State)
  { 
    //update the Switch1 state
    lastSwitch1State = thisState; 
 
    //this switch position has changed so do some stuff
    //"LOW condition code"
    //has switch gone from HIGH to LOW?
    if(thisState == LOW)                         
    {
      //Do HIGH to LOW stuff here
    }
    else
    {
      //Do LOW to HIGH stuff here
    }
 
 
  } //END of Switch code
 
  //***************************************** 
  // similar code for other switches goes here
  //***************************************** 
 
} //END of checkSwitches()
 
//**********************************************************************
 
//======================================================================
//                      E N D  O F  C O D E
//======================================================================

Suggest you look at a change in state rather than a level when it comes to switches.

since this is simple on/off operation, there's no need to keep track of the switch state.

it's not like trying to detect a button press where the state of the button changes state between subsequent button presses

Turn that around and you have, there is no reason to look at a switch’s level.

his code reads the switch and does one one of two things

i think your suggesting and unnecessary level of control -- check for a switch state change, set a state variable to the new state of the switch and do one of two things base on the state of the variable

i don't see a benefit

Speed of operation is one reason. When the switch goes HIGH you ‘do something’. When the switch goes LOW you ‘do something else’.


If you do something ‘while’ something is HIGH you are repeating things that don’t need to be repeated.


But really, the OP ask how things can be improved. They were offered ‘scanning of the the switch’, ‘looking at switch changes rather than levels’. Why not let the OP decide if they want to learn new ways of doing something ?


Why does everything need to turn into a p*ssing contest ?

Apologies to the OP.

larryd: If you do something ‘while’ something is HIGH you are repeating things that don’t need to be repeated.

when HIGH, the code is monitoring the IRvoltages to determine when to turn the motor on and off.

the code isn't simply turning the motor on or off when the switch changes state.

it makes sense that the code doesn't need to repeatedly turn the motor off when the switch is LOW

larryd: Why not let the OP decide if they want to learn new ways of doing something ?

i don't think this is what he wants to do

The OP does: else { motorstop(); } Guessing that this needs to be done once.


But we need to see complete sketches not snippets. ;)

Thanks guys for the advice. And thank you larry for the extensive example. I think I will try to implement the less frequent pin check.

Re: the following debate, my purpose is just to turn on the robot at the start and be able to neutralize it once with the flip of a switch. I'll be connecting/disconnecting the power supply shortly before/after the switch flips.

gcjr:
you might consider making it an if/else since only one case is possible (or are both possible)?

rather than make the bulk of loop() conditionally executed, you can

void loop (){

if (LOW = digitalRead(onoffpin))  {
    motorstop();
    return;
  }
 
  sensestuff();
 
  if (motoron == false && (IRVoltageR > 2.5 && IRVoltageL > 2.5)){
    motorstart();
  }
 
  if (motoron == true && (IRVoltageR < 2.5 || IRVoltageL < 2.5)) {
    motorstop(); 
  }
}

Does the return exit void loop and effectively end the program? That’s good for turning off the bot, but could make turning it on difficult if I’m starting with the switch at LOW.