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:

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");

     }
 }


}

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

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')