One button seems to be sending signal to everything

I had code set up, with one button being pressed switching between two relays (as it will be controlling relays which makes a model train turn out switch)
I had a one button configuration working (push once the track switch goes to turn, press it again it goes to straight).

I have now written the code to have 4 buttons, as I am running 4 points, which means 8 outputs in total.

However, when I push button A0 it just seems to cycle all the outputs to relays. pushing the other buttons does nothing.

My four points are Mainline, Quarry, Dropoff and Shunt.
I dont know where the problem is occuring, as i am new to arduino programming, and have not done more than some breadboard testing with single button. You will see a stepper motor declared, but this isnt used in the code, but will be to control a turntable in the future.

I am using an Arduino Uno, buttons are anolog 0 thru to 3, and output is 0 thru to 7.

The idea was that button press calls code, and that way i can easily make adjustments in the void Mainline section opposed to making adjustments in the void loop (etc) as i might want to add indicator LEDs etc.
power on is momentary for signal to relays, as once switch is triggered it only needs momentary power then switched off to prevent burn out.

But as discussed, issue is pressing mainlinbutton seems to just cycle through all the outputs (can see them light up on relay board) if i remove the button, it just cycles through without stopping, until i plug the button back in.

button has 5v power going in, output goes between input, and resistor to ground.

#include <Stepper.h>

#define mainlinebutton A0
#define quarrybutton A1
#define dropoffbutton A2
#define shuntbutton A3
#define mainlinestraight 0
#define mainlinecurve 1
#define quarrypass 2
#define quarryin 3
#define dropoffpass 4
#define dropoffin 5
#define shuntpass 6
#define shuntin 7

const int stepsPerRevolution = 2038;

Stepper turntable(stepsPerRevolution, 8, 10, 9, 11);

int stepCount = 0;

void setup() {
  pinMode(mainlinebutton, INPUT);
  pinMode(mainlinestraight, OUTPUT);
  pinMode(mainlinecurve, OUTPUT);
  pinMode(quarrybutton, INPUT);
  pinMode(quarrypass, OUTPUT);
  pinMode(quarryin, OUTPUT);
  pinMode(dropoffbutton, INPUT);
  pinMode(dropoffpass, OUTPUT);
  pinMode(dropoffin, OUTPUT);
  pinMode(shuntbutton, INPUT);
  pinMode(shuntpass, OUTPUT);
  pinMode(shuntin, OUTPUT);
  digitalWrite(mainlinestraight, LOW);
  digitalWrite(mainlinecurve, LOW);
  digitalWrite(quarrypass, LOW);
  digitalWrite(quarryin, LOW);
  digitalWrite(dropoffpass, LOW);
  digitalWrite(dropoffin, LOW);
  digitalWrite(shuntpass, LOW);
  digitalWrite(shuntin, LOW);
}
enum MAINLINESWITCH
{
  ML_OFF1,
  ML_OFF2,
  ML_STRAIGHT,
  ML_CURVE,
};
enum QUARRYLINESWITCH
{
  QL_OFF1,
  QL_OFF2,
  QL_STRAIGHT,
  QL_CURVE,
};
enum DROPOFFLINESWITCH
{
  DO_OFF1,
  DO_OFF2,
  DO_STRAIGHT,
  DO_CURVE,
};
enum SHUNTLINESWITCH
{
  SL_OFF1,
  SL_OFF2,
  SL_STRAIGHT,
  SL_CURVE,
};

MAINLINESWITCH mainswitch=ML_OFF1;
QUARRYLINESWITCH quarryswitch=QL_OFF1;
DROPOFFLINESWITCH dropoffswitch=DO_OFF1;
SHUNTLINESWITCH shuntswitch=SL_OFF1;

void loop() {

  if (analogRead(mainlinebutton==HIGH)) {           // if button pressed then 
    switch(mainswitch)             //   process the button press
      {
        case ML_OFF1:               // if current state is ML_OFF1 then set the switch to straight
        mainlstraight();           //calls mainlstraight code
        break;
    
       case ML_OFF2:               // if current state is ML_OFF2 then set the switch to turn
        mainlswitch();               // calls mainlswitch code
        break;
     }
     delay(200);
  }
  if (analogRead(quarrybutton==HIGH))
  {
    switch(quarryswitch)
    {
      case QL_OFF1:
      quarrylstraight();
      break;

      case QL_OFF2:
      quarrylswitch();
      break;
    }
    delay(200);
  }
  if (analogRead(dropoffbutton==HIGH))
  {
    switch(dropoffswitch)
    {
      case DO_OFF1:
      dropofflstraight();
      break;

      case DO_OFF2:
      dropofflswitch();
      break;
    }
    delay(200);
  }
  if (analogRead(shuntbutton==HIGH))
  {
    switch(shuntswitch)
    {
      case SL_OFF1:
      shuntlstraight();
      break;

      case SL_OFF2:
      shuntlswitch();
      break;
    }
    delay(200);
  }
 }

void mainlswitch(){
  digitalWrite(mainlinecurve, HIGH); //activates mainlinecurve relay
  delay(200);
  digitalWrite(mainlinecurve,LOW); //turns off mainlinecurve relay
  mainswitch=ML_OFF1; //chages to ML_OFF1
}
void mainlstraight(){
  digitalWrite(mainlinestraight, HIGH); 
  delay(200);
  digitalWrite(mainlinestraight,LOW); 
  mainswitch=ML_OFF2; 
}
void quarrylswitch(){
  digitalWrite(quarryin, HIGH); 
  delay(200);
  digitalWrite(quarryin,LOW); 
  quarryswitch=QL_OFF1; 
}

void quarrylstraight(){
  digitalWrite(quarrypass, HIGH); 
  delay(200);
  digitalWrite(quarrypass,LOW); 
  quarryswitch=QL_OFF2; 
}
void dropofflswitch(){
  digitalWrite(dropoffpass, HIGH); 
  delay(200);
  digitalWrite(dropoffpass,LOW); 
  dropoffswitch=DO_OFF1; 
}

void dropofflstraight(){
  digitalWrite(dropoffin, HIGH); 
  delay(200);
  digitalWrite(dropoffin,LOW); 
  dropoffswitch=DO_OFF2; 
}
void shuntlswitch(){
  digitalWrite(shuntpass, HIGH); 
  delay(200);
  digitalWrite(shuntpass,LOW); 
  shuntswitch=SL_OFF1; 
}

void shuntlstraight(){
  digitalWrite(shuntin, HIGH); 
  delay(200);
  digitalWrite(shuntin,LOW); 
  shuntswitch=SL_OFF2; 
}

Welcome to the forum

Thanks for using code tags but you had put them round your problem description as well as the code which made it hard to read. I have edited your post so that the code tags are only round the code itself

1 Like
  if (analogRead(mainlinebutton == HIGH))           // if button pressed then

A couple of things wrong here

  • why use analogRead() to read a digital input ?
  • remind yourself of the syntax for reading the state of a pin then check the line above

the argument to analogRead() is a conditional expression that is either 0 or 1 and being used as the pin # to read.

should be

    if (HIGH == analogRead(mainlinebutton))  {

it's also conventional to wire button switches between the pin and ground, configure the pin with an internal pull resistor, INPUT_PULLUP, and test for LOW

 if (HIGH == analogRead(mainlinebutton))

analogRead(). Why ?

As for putting the test conditions in that order, personally I dislike it intensely because the code does not read as an English sentence (more like Yoda) and does not match the action of physically operating the switch. I understand the reason for doing it but prefer not to

i just corrected the statement

i believe your point is it should be digitalRead () !

consider the following which has less redundant code making it easier to read and instead of copy&paste code for each new turnout, a single new line is added to sw []

this assume button switches are wired between the pin and ground


struct Switch {
    const byte   PinBut;
    const byte   PinRelayA;
    const byte   PinRelayB;
    const char  *label;

    byte         butState;
    byte         swState;
};

Switch sw [] = {
    { A0,  0, 1, "main" },
    { A1,  2, 3, "quarry" },
    { A2,  4, 5, "dropoff" },
    { A3,  6, 7, "shunt" },
};

#define  N_SW   (sizeof(sw)/sizeof(Switch))

byte butState [N_SW];

enum { Off = LOW, On = HIGH };
enum { A, B };

// -----------------------------------------------------------------------------
void pulse (
    byte pin )
{
    digitalWrite (pin, On);
    delay (200);
    digitalWrite (pin, Off);
}

// -----------------------------------------------------------------------------
void loop ()
{
    for (unsigned n = 0; n < N_SW; n++)  {
        byte   but = digitalRead (sw[n].PinBut);

        if (sw[n].butState != but)  {
            sw[n].butState = but;
            delay (10);             // debounce
        
            if (LOW == but)  {      // button pressed
                if (A == sw[n].swState)  {
                    pulse (sw[n].PinRelayB);
                    sw[n].swState = B;
                }
                else  {
                    pulse (sw[n].PinRelayA);
                    sw[n].swState = A;
                }

                Serial.print   (sw[n].swState);
                Serial.print   (" ");
                Serial.println (sw[n].label);
            }
        }
    }
}

// -----------------------------------------------------------------------------
void setup ()
{
    Serial.begin (9600);

    for (unsigned n = 0; n < N_SW; n++)  {
        pinMode (sw [n].PinBut, INPUT_PULLUP);
        sw [n].butState = digitalRead (sw [n].PinRelayA);

        digitalWrite (sw [n].PinRelayA, Off);
        digitalWrite (sw [n].PinRelayB, Off);
        pinMode (sw [n].PinRelayA, OUTPUT);
        pinMode (sw [n].PinRelayB, OUTPUT);
    }
}
1 Like

I originally had it as if (analogRead(mainlinebutton > 500)) as it is running on A0 input, but found the result of the code was exactly the same.

Once everything is completed I won't have any digital pins avaliable, as 8 are used for running relay for point controllers, and then 4 for stepper motor. (and 2 left for an LED indicator)

You can use the A* pins as digital pins
If, however, you insist on using analogRead() on those pins then do it correctly

Thanks for that gcjr, I will have to read up on the switch function to better understand the code, but it all accomplishes what I was wanting. Only issue I am facing is that output 0 and 1, when booting they activate but never switch off. button A1, A2, A3 all seem to work switching between their desired outputs.
when pressing A0 i can see the light flash on the ardunio board but relay just stays on (0 & 1) (which has been on since boot).

any ideas on what could be causing it? Cheers.

Are you saying I can just treat the A0-A5 pins as digital pins? as in use digitalRead, instead of analogRead?

here is the serial output: showing everything is working, but relay 0 and 1 are stuck on HIGH
1 main
0 main
1 main
0 main
1 main
1 quarry
0 quarry
1 quarry
0 quarry
1 dropoff
0 dropoff
1 dropoff
0 dropoff
1 shunt
0 shunt
1 shunt
0 shunt
0 main
1 main
0 main
1 main

are the other relays turning on/off properly?

Yes.

AND you need to get the syntax correct for reading the pins... currently ")" is in the wrong place.

Should be...
if(analogRead(mainlinebutton) > 500)

Do you see the difference?

... but now we're using digitalRead, you'll need to check for HIGH or LOW.

if(digitalRead(mainlinebutton) == LOW)

i don't seem what i may be missing and retested using the LEDs on my board

could you try the following which includes prints of the pin being pulsed and it's final value using digitalRead().

struct Switch {
    const byte   PinBut;
    const byte   PinRelayA;
    const byte   PinRelayB;
    const char  *label;

    byte         butState;
    byte         swState;
};

Switch sw [] = {
    { A0,  0, 1, "main" },
#undef MyHW
#ifdef MyHW
    { A1, 10,11, "quarry" },
    { A2, 12,13, "dropoff" },
#else
    { A1,  2, 3, "quarry" },
    { A2,  4, 5, "dropoff" },
#endif
    { A3,  6, 7, "shunt" },
};

#define  N_SW   (sizeof(sw)/sizeof(Switch))

byte butState [N_SW];

enum { Off = LOW, On = HIGH };
enum { A, B };

// -----------------------------------------------------------------------------
void pulse (
    byte pin )
{
    digitalWrite (pin, On);
    delay (200);
    digitalWrite (pin, Off);

    Serial.print   (" pulse: pin ");
    Serial.print   (pin);
    Serial.print   (" ");
    Serial.println (digitalRead (pin));
}

// -----------------------------------------------------------------------------
void loop ()
{
    for (unsigned n = 0; n < N_SW; n++)  {
        byte   but = digitalRead (sw[n].PinBut);

        if (sw[n].butState != but)  {
            sw[n].butState = but;
            delay (10);             // debounce
        
            if (LOW == but)  {      // button pressed
                if (A == sw[n].swState)  {
                    pulse (sw[n].PinRelayB);
                    sw[n].swState = B;
                }
                else  {
                    pulse (sw[n].PinRelayA);
                    sw[n].swState = A;
                }

                Serial.print   (sw[n].swState);
                Serial.print   (" ");
                Serial.println (sw[n].label);
            }
        }
    }
}

// -----------------------------------------------------------------------------
void setup ()
{
    Serial.begin (9600);

    for (unsigned n = 0; n < N_SW; n++)  {
        pinMode (sw [n].PinBut, INPUT_PULLUP);
        sw [n].butState = digitalRead (sw [n].PinRelayA);

        digitalWrite (sw [n].PinRelayA, Off);
        digitalWrite (sw [n].PinRelayB, Off);
        pinMode (sw [n].PinRelayA, OUTPUT);
        pinMode (sw [n].PinRelayB, OUTPUT);
    }
}

Same issue occurred, however I just changed my output pins for Main (currently 0,1) to 8,9, and it has fixed the relay issue. I don't know what TX-RX is on I/O 0,1, could that be what was causing the issue?

pulse: pin 1 0
1 main
pulse: pin 0 1
0 main
pulse: pin 1 0
1 main
pulse: pin 3 0
1 quarry
pulse: pin 2 0
0 quarry
pulse: pin 5 0
1 dropoff
pulse: pin 4 0
0 dropoff
pulse: pin 0 1
0 main
pulse: pin 1 0
1 main
pulse: pin 0 1
0 main
pulse: pin 5 0
1 dropoff
pulse: pin 4 0
0 dropoff

this is where i changed it main to read from pin 8 9, sending to relay int 1, 2 (8 channel Keyes relayboard)
pulse: pin 9 0
1 main
pulse: pin 8 0
0 main
pulse: pin 3 0
1 quarry
pulse: pin 2 0
0 quarry
pulse: pin 5 0
1 dropoff
pulse: pin 4 0
0 dropoff
pulse: pin 9 0
1 main
pulse: pin 8 0
0 main
pulse: pin 3 0
1 quarry
pulse: pin 2 0
0 quarry
pulse: pin 5 0
1 dropoff
pulse: pin 4 0
0 dropoff

Yes

Pins 0 and 1 are used by the hardware Serial interface (hence Rx and Tx) and unless you really know what you are doing and don't use that interface in the sketch then don't use those pins

Please post your full sketch that works

The code that worked was @gcjr Greg's code. i just had to move off pins 0 and 1 for it to work properly. The next task I have is working out what greg's code does, so I can add an LED indicator to Mainline (only) so I know if it will cross or straight. (as those points are the only risk of train crash)

The build is a model train line that will fit in a coffee table, and I have made a control board which will be to activate lights, switches, rotate the turn table, that way my three year old can hopefully have alot of fun with it and it be quite interactive.

I have definitely bitten off more than i expected and got most of my code ideas from railroad youtube videos featuring arduinos.

struct Switch {
    const byte   PinBut;
    const byte   PinRelayA;
    const byte   PinRelayB;
    const char  *label;

    byte         butState;
    byte         swState;
};

Switch sw [] = {
    { A0,  0, 1, "main" },
#undef MyHW
#ifdef MyHW
    { A1, 10,11, "quarry" },
    { A2, 12,13, "dropoff" },
#else
    { A1,  2, 3, "quarry" },
    { A2,  4, 5, "dropoff" },
#endif
    { A3,  6, 7, "shunt" },
};

#define  N_SW   (sizeof(sw)/sizeof(Switch))

byte butState [N_SW];

enum { Off = LOW, On = HIGH };
enum { A, B };

// -----------------------------------------------------------------------------
void pulse (
    byte pin )
{
    digitalWrite (pin, On);
    delay (200);
    digitalWrite (pin, Off);

    Serial.print   (" pulse: pin ");
    Serial.print   (pin);
    Serial.print   (" ");
    Serial.println (digitalRead (pin));
}

// -----------------------------------------------------------------------------
void loop ()
{
    for (unsigned n = 0; n < N_SW; n++)  {
        byte   but = digitalRead (sw[n].PinBut);

        if (sw[n].butState != but)  {
            sw[n].butState = but;
            delay (10);             // debounce
        
            if (LOW == but)  {      // button pressed
                if (A == sw[n].swState)  {
                    pulse (sw[n].PinRelayB);
                    sw[n].swState = B;
                }
                else  {
                    pulse (sw[n].PinRelayA);
                    sw[n].swState = A;
                }

                Serial.print   (sw[n].swState);
                Serial.print   (" ");
                Serial.println (sw[n].label);
            }
        }
    }
}

// -----------------------------------------------------------------------------
void setup ()
{
    Serial.begin (9600);

    for (unsigned n = 0; n < N_SW; n++)  {
        pinMode (sw [n].PinBut, INPUT_PULLUP);
        sw [n].butState = digitalRead (sw [n].PinRelayA);

        digitalWrite (sw [n].PinRelayA, Off);
        digitalWrite (sw [n].PinRelayB, Off);
        pinMode (sw [n].PinRelayA, OUTPUT);
        pinMode (sw [n].PinRelayB, OUTPUT);
    }
}

i think both bob and i are model RRs

i added some comments


// define a struct describing the pins controlling the turnout
struct Switch {
    const byte   PinBut;        // button pin
    const byte   PinRelayA;     // normal route pin
    const byte   PinRelayB;     // reverse route pin
    const char  *label;         // description for debugging

    byte         butState;      // button state used to detect press
    byte         swState;       // switch state
};

// describe turnouts
Switch sw [] = {
    { A0,  8, 9, "main" },
    { A1,  2, 3, "quarry" },
    { A2,  4, 5, "dropoff" },
    { A3,  6, 7, "shunt" },
};

// determine # of turnouts being decribed
#define  N_SW   (sizeof(sw)/sizeof(Switch))

// define meaningful names for relay output value and switch state
enum { Off = LOW, On = HIGH };
enum { A, B };

// -----------------------------------------------------------------------------
// pulse pin on then off for 200 msec
void pulse (
    byte pin )
{
    digitalWrite (pin, On);
    delay (200);
    digitalWrite (pin, Off);

    // report pin status
    Serial.print   (" pulse: pin ");
    Serial.print   (pin);
    Serial.print   (" ");
    Serial.println (digitalRead (pin));
}

// -----------------------------------------------------------------------------
void loop ()
{
    // index thru each turnout descriptor
    for (unsigned n = 0; n < N_SW; n++)  {
        // read button
        byte   but = digitalRead (sw[n].PinBut);

        // check for change of button state
        if (sw[n].butState != but)  {
            sw[n].butState = but;
            delay (10);             // debounce
        
            // check if button pressed
            if (LOW == but)  {
                // determine switch state and pulse opposite relay
                if (A == sw[n].swState)  {
                    pulse (sw[n].PinRelayB);
                    sw[n].swState = B;
                }
                else  {
                    pulse (sw[n].PinRelayA);
                    sw[n].swState = A;
                }

                // report new switch state
                Serial.print   (sw[n].swState);
                Serial.print   (" ");
                Serial.println (sw[n].label);
            }
        }
    }
}

// -----------------------------------------------------------------------------
void setup ()
{
    Serial.begin (9600);

    // iterate thru each turnout descriptor
    for (unsigned n = 0; n < N_SW; n++)  {
        // configure button as output and initialize state
        pinMode (sw [n].PinBut, INPUT_PULLUP);
        sw [n].butState = digitalRead (sw [n].PinRelayA);

        // configure both relay outputs Off and as outputs
        digitalWrite (sw [n].PinRelayA, Off);
        digitalWrite (sw [n].PinRelayB, Off);
        pinMode (sw [n].PinRelayA, OUTPUT);
        pinMode (sw [n].PinRelayB, OUTPUT);
    }
}

a common novice mistake (and i still saw professionals do this) is to copy&paste code rather than create a sub-function that can perform that function for different cases.

the excessive amount of code becomes hard to read and when changes are needed, they need to be made in multiple locations. hopefully this code demonstrates that not much code is needed to do things and in this case, additional turnouts can easily be added

i would suggest adding another field to the struct

    const byte   PinRelayB;     // reverse route pin
    const char  *label;         // description for debugging
    const byte   PinLED;        // led pin

define a pin in the array if there is an LED (we now know pin 0 can't be used to a value o zero means no LED)

    { A1, 12, 13, "quarry",  10 },       // pin #s for my hardware

add code to change state of LED when turnout position changes. (needs to be done for both cases: A & B)

                if (A == sw[n].swState)  {
                    pulse (sw[n].PinRelayB);
                    sw[n].swState = B;
                    if (sw[n].PinLED)
                        digitalWrite (sw[n].PinLED, On);
                }

and configure the LED pin as an OUTPUT in setup()

        digitalWrite (sw [n].PinLED, Off);
        pinMode (sw [n].PinLED, OUTPUT);

Hi Greg, the code is completed with the LED indicator. at the moment it is just on the controller board, an RGB LED, that is green for straight, and red for crossing. but once the actual train layout is underway it will also be able to control the Signal on the board which will be cool.

Completed code below, thanks again for all your help!!

//Model Train Point Motor Controller with LED signal indicator for Mainline. controls 4 points in total, and requires a relay (8 channel)
// define a struct describing the pins controlling the turnout
struct Switch {
    const byte   PinBut; //button pin
    const byte   PinRelayA; //normal route pin
    const byte   PinRelayB; //switch route pin
    const char  *label; //for debugging
    const byte   PinLED1;//signal indicators for board
    const byte   PinLED2;

    byte         butState; //button state to detect press
    byte         swState; //switch state
};

//describes turnouts, allocates pins according to structure
Switch sw [] = {
    { A0,  8, 9, "main", 10, 11 },
    { A1,  2, 3, "quarry" },
    { A2,  4, 5, "dropoff" },
    { A3,  6, 7, "shunt" },
};
// determine # of turnouts being decribed
#define  N_SW   (sizeof(sw)/sizeof(Switch))

byte butState [N_SW];
// define meaningful names for relay output value and switch state
enum { Off = LOW, On = HIGH };
enum { A, B };

// -----------------------------------------------------------------------------
// pulse pin on then off for 200 msec. Adjust this if you find it isnt enough time to trigger point move.
void pulse (
    byte pin )
{
    digitalWrite (pin, On);
    delay (200);
    digitalWrite (pin, Off);
}

// -----------------------------------------------------------------------------
void loop ()
{
  // index thru each turnout descriptor
    for (unsigned n = 0; n < N_SW; n++)  {
        byte   but = digitalRead (sw[n].PinBut); // read button

        if (sw[n].butState != but) // check for change of button state
        {
            sw[n].butState = but;
            delay (10);             // debounce
        
            if (LOW == but)  {      // button pressed
                if (A == sw[n].swState)  {
                    pulse (sw[n].PinRelayB);
                    sw[n].swState = B;
                    if (sw[n].PinLED1)
                      digitalWrite (sw[n].PinLED1, On);
                      digitalWrite (sw[n].PinLED2, Off);
                }
                else  {
                    pulse (sw[n].PinRelayA);
                    digitalWrite (sw[n].PinLED1, Off);
                    digitalWrite (sw[n].PinLED2, On);
                    sw[n].swState = A;
                }

                Serial.print   (sw[n].swState);
                Serial.print   (" ");
                Serial.println (sw[n].label);
            }
        }
    }
}

// -----------------------------------------------------------------------------
void setup ()
{
    Serial.begin (9600);
// iterate thru each turnout descriptor
    for (unsigned n = 0; n < N_SW; n++)  {
        pinMode (sw [n].PinBut, INPUT_PULLUP);// configure button as output and initialize state
        sw [n].butState = digitalRead (sw [n].PinRelayA);
// configure both relay outputs Off and as outputs
        digitalWrite (sw [n].PinRelayA, Off);
        digitalWrite (sw [n].PinRelayB, Off);
        digitalWrite (sw [n].PinLED1, Off);
        digitalWrite (sw [n].PinLED2, Off);
        pinMode (sw [n].PinRelayA, OUTPUT);
        pinMode (sw [n].PinRelayB, OUTPUT);
        pinMode (sw [n].PinLED1, OUTPUT);
        pinMode (sw [n].PinLED2, OUTPUT);
    }
}