Digital Input using a button doesn't work properly when using it to control a relay

Question

I'm trying to control a relay both via Bluetooth and using a switch. The problem is that, when the LED is off, I can turn it on with the switch but then if I turn off the switch the relay will stay on. Is there something wrong with my code? Thanks in advance. The switch is installed as follows: Switch

Here is my code: Switch is connected to pin 3 and the relay is connected to pin 2.

//bluetooth hc-06
int ledPin = 2; // usamos un pin de salida al LED
int state = 0; // Variable lectrura serial
int interruptor=3;
void setup() {
 pinMode(ledPin, OUTPUT); //Declara pin de Salida
 pinMode(interruptor, INPUT);
 digitalWrite(ledPin, LOW); //Normalmente Apagado
 Serial1.begin(9600);
 }
void loop() {
 int valor=digitalRead(interruptor);
 if(Serial1.available() > 0){
 state = Serial1.read();
 } // esta parte del código es para solo 1 Carácter o Unidad. 
 if (state == '0'&& valor== LOW) {
 digitalWrite(ledPin, LOW);
 Serial1.println("LED: off");
 }
 else
 if (state == '1' || valor== HIGH) {
 digitalWrite(ledPin, HIGH);
 Serial1.println("LED: on");
 }
}

New code:

//bluetooth hc-06
int ledPin = 2; // usamos un pin de salida al LED
int state = 1; // Variable lectrura serial
int interruptor=3;
void setup() {
 pinMode(ledPin, OUTPUT); //Declara pin de Salida
 pinMode(interruptor, INPUT);
 digitalWrite(ledPin, LOW); //Normalmente Apagado
 Serial1.begin(9600);
 }
void loop() {
 int valor=digitalRead(interruptor);
 //si el modulo ha mandado dato, guardarlo en estado.
 if(Serial1.available() > 0){
 state = Serial1.read();
 } // esta parte del código es para solo 1 Carácter o Unidad. 
 // si el estado es 0 ese sería Apagado "OFF"
 if (valor== LOW) {
 digitalWrite(ledPin, LOW);
 Serial1.println("LED: off");
 }
 // de lo contrario si el estado es 1 ese sería Encendido "ON"
 else
 if (valor== HIGH) {
 if (state== 1){ 
 digitalWrite(ledPin, HIGH);
 Serial1.println("LED: on");
 }
 else if (state== 0){
 digitalWrite(ledPin, LOW);
 Serial1.println("LED: off");
 }
 }
}

• Daniel Navarro: The circuit is working properly when using bluetooth commands, in case that helps.
• chrisl: Should the switch work independently from the bluetooth command on the relay? Currently you can only turn it on with state='1' AND switch on, and you can only turn it off with state='0' AND switch off. It is not completely clear, what the desired behavior is. Please explain further, what the code should do.
• Daniel Navarro: Okay, I modified my code so I can only control via bluetooth when the switch is ON, however it is still not working...
• Jim Mack: You might be better off with boolean state = true, then using state = (bool)Serial1.read() and 'if (state)...' There's nothing shown here that guarantees Serial1.read()` will return a value of 1
• Delta_G: "I'm trying to control a relay both via Bluetooth and using a switch." You should explain this further. How do you want it to act? Does one override the other?

Answer 1

Well, there are a couple of problems.

In the original SW, you are checking for state = '0' and state = '1', but the initial value of state is 0 (not '0'). For this reason both tests always return false.

In addition, avoid setting conditions when not necessary. else already mean "all the other options", so if you have one variable that can be only A or B, don't write if A ... else if B ...; the second condition is necessary (and meaningful) only if the condition is actually something you care of (so you want to exclude condition C).

I started modifying your sketch from the new version, and here are the things I modified:

• Variables were set to type uint8_t; int is not efficient on 8-bit platforms, so use 8-bit variables whenever you can
• Instead of using multiple triggers, I used a single variable (ledState) to calculate the state in each loop. This way it is much easier to track the modifications
• simplified the input management: now it turns on the led when the input is high, and does nothing (leaves it off) when it is low
• simplified the bluetooth management. With a '1' it turns on the led, and all the other chars set it off; if you need only a '0' to turn it off you can easily modify the parentheses to ignore other chars

This is the code I wrote. NOTE: I did not test it, so there may be residual bugs. Let me know if it works as you expect or want some additional clarifications:

uint8_t ledPin = 2;
uint8_t interruptor = 3;
uint8_t bluetoothState = '0';
void setup() {
 pinMode(ledPin, OUTPUT);
 pinMode(interruptor, INPUT);
 digitalWrite(ledPin, LOW);
 Serial1.begin(9600);
}
void loop() {
 uint8_t ledState = LOW;
 
 if (digitalRead(interruptor)) {
 ledState = HIGH;
 Serial1.println("LED: on (switch)");
 }
 
 if(Serial1.available() > 0){
 bluetoothState = Serial1.read();
 }
 
 if (bluetoothState == '1') {
 ledState = HIGH;
 Serial1.println("LED: on (BT)");
 }
 
 digitalWrite(ledPin, ledState);
 if (ledState == LOW) {
 Serial1.println("LED: off");
 }
}

Things still to do (IMHO):

• make a robust button handler (for instance using the debounce library)
• get rid of the serial debug outputs
• make a better serial protocol

• Dave Newton: Hopefully the problem was resolved in the intervening 2.5 yrs ;)
• frarugi87: O_O why was it in the homepage then? I assumed it was something like 10 minutes old LOL.. sorry for the necroposting
• Dave Newton: :shrug: It's a low-traffic site so stuff gets bumped for visibility ;) And it's fine anyway--it'll help somebody, if not the OP :)
• Nick Gammon: 3.5 years now. :)
• Nick Gammon: I assumed it was something like 10 minutes old ... yes, SE does do odd things like making posts look relevant when they are not.

Answer 2

You have to invalidate the state variable when Serial is not available, otherwise you will produce strange behaviours.

if (Serial1.available() > 0) {
 state = Serial1.read();
} else {
 state = -1; // not valid
}

Also, serial values will be characters, not integers, so compare with character values, e.g.,

if (state == '1')