Two Inputs, Same Output to Relay Board Not Working

Forgive me if this is a super beginner issue, but that’s what I am…

I have a sketch written that takes inputs from 4 different buttons.
This part is working fine.
I also have several outputs also working fine (ie turning on different LEDs, running a servo, etc).

As I iterate the development I have reached the point where I want to assign a number of actions to the buttons, some of which have overlap.

For example, I want button A to close relay #1 for 3 seconds when pressed, but I want button D to close the same relay for the duration of the button press only.

When I took the code for closing the relay which works on button A and tried to do the exact same code for button D, the relay did not click but the light lit up.

So as I sit, if I press button A the relay clicks (and the little LED on the board lights up)
When I press button D the LED on the board lights, but the relay does not click.
If I comment out the code for A then D starts working.

Is there some issue with two digital inputs with the same output?

Hopefully I have explained this correctly.

// Set pin numbers and define servo for choke:
//includes
#include <Servo.h>
// Define Servo
Servo choke;
int pos = 0;
// Define Remote Buttons
const int AbuttonPin = 11;     // Remote input for A button
const int BbuttonPin = 9;      // Remote input for B button
const int CbuttonPin = 7;     // Remote input for C button
const int DbuttonPin = 12;     // Remote input for D button
// define digital outputs
const int starterOutputPin =  2;      // output pin to starter relay
const int BoutputPin =  8;      // the number of the LED pin

// set states:
int AbuttonState = 0;         // variable for reading the pushbutton status
int BbuttonState = 0;         // variable for reading the pushbutton status
int CbuttonState = 0;         // variable for reading the pushbutton status
int DbuttonState = 0;         // variable for reading the pushbutton status

void setup() {
// Attach servo control to pin
  choke.attach(6);

  // initialize the LED pin as an output:
  pinMode(starterOutputPin, OUTPUT);
  pinMode(BoutputPin, OUTPUT);

  // initialize the pushbutton pins as inputs:
  pinMode(AbuttonPin, INPUT);
  pinMode(BbuttonPin, INPUT);
  pinMode(CbuttonPin, INPUT);
  pinMode(DbuttonPin, INPUT);

}

void loop() {
// D Button Code -Manual Starter
  DbuttonState = digitalRead(DbuttonPin);

  // check if the Dpushbutton is pressed.
  // if it is, the DbuttonState is HIGH:
  if (DbuttonState == LOW) {
    // turn LED on:
    digitalWrite(starterOutputPin, HIGH);
  } else {
    // turn LED off:
    digitalWrite(starterOutputPin, LOW);
  }

// A Button Code
  AbuttonState = digitalRead(AbuttonPin);

  // check if the pushbutton is pressed.
  // if it is, the buttonState is HIGH:
  if (AbuttonState == LOW) {
    // turn LED on:
    digitalWrite(starterOutputPin, HIGH);
  } else {
    // turn LED off:
    digitalWrite(starterOutputPin, LOW);
  }

// B Button Code
    BbuttonState = digitalRead(BbuttonPin);

  // check if the pushbutton is pressed.
  // if it is, the buttonState is HIGH:
  if (BbuttonState == LOW) {
    // turn LED on:
    digitalWrite(BoutputPin, HIGH);
  } else {
    // turn LED off:
    digitalWrite(BoutputPin, LOW);
  }

// C Button Code
    CbuttonState = digitalRead(CbuttonPin);

  // check if the pushbutton is pressed.
  // if it is, the buttonState is HIGH:
  if (CbuttonState == LOW) {
    // Run servo:
    choke.write(90);
  } else {
    // stop servo:
    choke.write(0);
  }


  
}
DbuttonState = digitalRead(DbuttonPin);

  // check if the Dpushbutton is pressed.
  // if it is, the DbuttonState is HIGH:
  if (DbuttonState == LOW) {
    // turn LED on:
    digitalWrite(starterOutputPin, HIGH);
  } else {
    // turn LED off:
    digitalWrite(starterOutputPin, LOW);
    }

Is this what you intend, that the LED is off when the button is pressed? It seems counterintuitive to me.

Anyway, i's a good practice to not have the same output controlled by different parts of the program - especially here since the logic 'falls through' to successively check each switch. Separating the outputs gives rise to a 'last one controlling wins' situation. Separating the output statements can also complicate troubleshooting. You can beat your head against a piece of code that appears to be malfunctioning but is actually correct while the real culprit is in another function you forgot about. If two or more conditions are to control an output combine them in a single statement with the or '||' function. As in:

 // combined with OR
  if (Dbuttonstate == LOW || Abuttonstate == LOW) {
    digitalWrite(starterOutputPin, HIGH);
  }
  else {
    // turn LED off:
    digitalWrite(starterOutputPin, LOW);

This sort of program will be much easier if you separate the input and the output into separate functions. At the moment you have them all mixed up in loop().

Create a function that reads the buttons and updates the value of variables that represent the state of the LEDs (for example). Create another function that operates the LEDs depending on the value of the appropriate variable. Note that several buttons could affect one variable or one button could affect several variables. The only thing common between the two parts of the program are those variables. The input side has no idea what the variable is used for. And the output side has no idea where the value came from.

It may (or may may not) also be useful to have a function that manages the timing by changing the variable when some time has elapsed.

Have a look at how it is done in Several Things at a Time and in Planning and Implementing a Program

...R

DougP,

That is another weird thing that I have found;
When I wrote it originally, i checked if the button was high and then wrote high to the output and put the low in the else statement. It functioned the reverse of what I expected, so I reversed them like you see above. I expected that when the button is pressed it would be 'high' and not pressed would be 'low'.

I don't want to use an OR for the buttons however because despite this simple state my development is in at this point, the future goal is for the different buttons to do some of the same things in sequence, not everything the same.

To put it in a practical example, I am building a remote starter for a generator at my off grid cottage. No one wants to go out and start the generator in the winter just to make toast and certainly no one wants to go turn it off when the toast is done and its time to eat breakfast by the wood stove (I have some other plans for this arduino as well, but this is the start).
I have a 4 button remote control.

I want the;
A button to actuate a servo for the choke, crank the engine (via relay board) for a defined amount of time and then after a few seconds turn the choke back.
B button will open the kill switch via the relay board.
in case my timings don't work out due to temperature or some other reason, I want to manually operate the choke and starter not as part of a timed sequence.
C button will actuate the choke only on one button press, back on the next.
D button will crank the starter for as long as I hold the remote button down.

So where I am at is that I have a relay board, the servo, remote and receiver.
I have mocked it up with two LED's on the relay board, one normally closed to represent the kill switch, one normally open to represent the starter. I also have the servo.

I started by setting up the buttone and outputs assigning each function to a button.
Everything works as expected until I tried to do the same function in two different if statements in the loop.

My next step was going to be to make the A button do the three steps, so I thought I would write the D button first by copying the code and updating the variables for the button, etc.

That's where I ran into this issue. When I press A the relay clicks and the little LED on the relay board lights up nice and bright. When I press D however, the LED on the relay board lights up dimly and the coil does not click. If I comment out the A button code, the D works fine......

I'm sure I'm missing something simple.

Robin, I will look at those.

Thanks all

cphinney:
That's where I ran into this issue. When I press A the relay clicks and the little LED on the relay board lights up nice and bright. When I press D however, the LED on the relay board lights up dimly and the coil does not click. If I comment out the A button code, the D works fine......

Try this experiment - reverse the order of the statements in the A button code and the D button code. That is, do all the A stuff, then all the D stuff. What happens?

So...... I put the code for each button into functions and called the functions from the loop.....
It worked as expected.

Now I just have to get the proper servo (accidentally bought a continuous rotation one) and I think I can make this thing work.

Cheers.