problem with code

Explanation of project:~
I am a complete novice to Arduino, and am using the TinkerCAD website to try and devise a code to control 3-aspect Model Rail Signals.

Each signal has a Red, Yellow, and, Green aspect.

When a signal shows Red, this should turn the signal behind (a.k.a. the previous signal) to Yellow, and have no effect on the signal behind that.

I have only included 3 signals in the code, in order that I can grasp the principles before increasing the number of signals.

The 3 signals can be considered to be equally spaced around a circular track, in such a manner as the Train will pass signal 1, then pass signal 2, then pass signal 3, and then pass signal 1 again, etc, etc, etc.

This is to control Trains in one direction only.

As I am using the TinkerCAD website, I have created a project using 3 pushbuttons as INPUTS, to simulate the Train being detected in the block section(s) ahead of each signal.

Description of problem:~
When I operate pushbutton 1 (to simulate that the Train has now passed signal-1 and is being detected in the block section ahead of that signal, signal-1 correctly displays a Red aspect, and signal-2 correctly displays a Yellow aspect, and signal-3 correctly remains unchanged at Green.

BUT>>>> the Green aspect in Signal 1 goes dim instead of OFF, and the Green aspect in signal-2 remains Green (full brightness).

A similar thing happens when I operate pushbutton 2, and also pushbutton 3; i.e. the signal corresponding to those pushbuttons correctly displays a Red aspect, and the one behind correctly displays a Yellow aspect, BUT>>>> the signal showing Red also has the Green dim, and the signal showing Yellow has the Green full brightness.

I am assuming from the way I have attempted to write the code that the Green aspects are getting conflicting instructions; i.e. the code appertaining to whichever pushbutton is operated to set the corresponding signal to Red is correctly telling the corresponding Green to go OFF, but another part of the code is telling the same Green to stay ON.

Unfortunately, as a complete novice, I am unable to think of a solution to this problem.

Plea:~
Could somebody please provide me with some guidance as to how I can solve this issue?

I have (hopefully) attached a screenshot of what happens when I operate the pushbutton, and I also include my (very rudimentary) code.

Thanks.

[code]
const int R1 = 2;        // choose pin for signal 1 RED aspect
const int Y1 = 3;       // choose pin for signal 1 YELLOW aspect
const int G1 = 4;       // choose pin for signal 1 GREEN aspect
const int R2 = 5;       // choose pin for signal 2 RED aspect
const int Y2 = 6;       // choose pin for signal 2 YELLOW aspect
const int G2 = 7;       // choose pin for signal 2 GREEN aspect
const int R3 = 8;       // choose pin for signal 3 RED aspect
const int Y3 = 9;       // choose pin for signal 3 YELLOW aspect
const int G3 = 10;        // choose pin for signal 3 GREEN aspect


const int inPin1 = 14;      // choose pin for pushbutton 1
int val1 = 0;         // variable for reading the pin status
const int inPin2 = 15;      // choose pin for pushbutton 2
int val2 = 0;         // variable for reading the pin status
const int inPin3 = 16;      // choose pin for pushbutton 3
int val3 = 0;         // variable for reading the pin status




void setup() {
  pinMode(R1, OUTPUT);      // set pin as output
  pinMode(Y1, OUTPUT);      // set pin as output
  pinMode(G1, OUTPUT);      // set pin as output
  pinMode(R2, OUTPUT);      // set pin as output
  pinMode(Y2, OUTPUT);      // set pin as output
  pinMode(G2, OUTPUT);      // set pin as output
  pinMode(R3, OUTPUT);      // set pin as output
  pinMode(Y3, OUTPUT);      // set pin as output
  pinMode(G3, OUTPUT);      // set pin as output
  pinMode(14, INPUT);       // set pin as input
  pinMode(15, INPUT);       // set pin as input
  pinMode(16, INPUT);       // set pin as input

  digitalWrite(R1, LOW);    // turn R1 OFF
  digitalWrite(Y1, LOW);    // turn Y1 OFF
  digitalWrite(G1, HIGH);   // turn G1 ON
  digitalWrite(R2, LOW);    // turn R2 OFF
  digitalWrite(Y2, LOW);    // turn Y2 OFF
  digitalWrite(G2, HIGH);   // turn G2 ON
  digitalWrite(R3, LOW);    // turn R3 OFF
  digitalWrite(Y3, LOW);    // turn Y3 OFF
  digitalWrite(G3, HIGH);   // turn G3 ON
}




void loop() {


  val1 = digitalRead(inPin1); // read input value 1
  if (val1 == LOW) {          // check if input 1 is LOW (button 1 pressed)
    digitalWrite(R1, HIGH);   // turn R1 ON
    digitalWrite(Y1, LOW);    // turn Y1 OFF
    digitalWrite(G1, LOW);    // turn G1 OFF
    digitalWrite(Y2, HIGH);   // turn Y2 ON
    digitalWrite(G2, LOW);    // turn G2 OFF

  } else {
    digitalWrite(R1, LOW);    // turn R1 OFF
    digitalWrite(G1, HIGH);   // turn G1 ON
    digitalWrite(Y2, LOW);    // turn Y2 OFF
    digitalWrite(G2, HIGH);   // turn G2 ON


  }



  val2 = digitalRead(inPin2);  // read input value 2
  if (val2 == LOW) {          // check if input 2 is LOW (button 1 pressed)
    digitalWrite(R2, HIGH);   // turn R2 ON
    digitalWrite(Y2, LOW);    // turn Y2 OFF
    digitalWrite(G2, LOW);    // turn G2 OFF
    digitalWrite(Y3, HIGH);   // turn Y3 ON
    digitalWrite(G3, LOW);    // turn G3 OFF

  } else {
    digitalWrite(R2, LOW);    // turn R2 OFF
    digitalWrite(G2, HIGH);   // turn G2 ON
    digitalWrite(Y3, LOW);    // turn Y3 OFF
    digitalWrite(G3, HIGH);   // turn G3 ON

  }



  val3 = digitalRead(inPin3);  // read input value 3
  if (val3 == LOW) {          // check if input 3 is LOW (button 1 pressed)
    digitalWrite(R3, HIGH);   // turn R3 ON
    digitalWrite(Y3, LOW);    // turn Y3 OFF
    digitalWrite(G3, LOW);    // turn G3 OFF
    digitalWrite(Y1, HIGH);   // turn Y1 ON
    digitalWrite(G1, LOW);    // turn G1 OFF

  } else {
    digitalWrite(R3, LOW);    // turn R3 OFF
    digitalWrite(G3, HIGH);   // turn G3 ON
    digitalWrite(Y1, LOW);    // turn Y1 OFF
    digitalWrite(G1, HIGH);   // turn G1 ON

  }

}

[/code]

An LED going dim instead of completely off is probably because the pin that is controlling it is being turned on and off rapidly.

With pushbuttons that you are using to debug your code, you need to debounce them.

ieee488,
thanks for quick response.

I am getting this problem even when I keep the pushbutton pressed for "a long time".

Does this still indicate a "debouncing" issue?

If no buttons are pressed, the three else clauses will execute.

The else for val1 sets g2 LOW, turning the LED off.

Very shortly afterwards, the else for val2 runs which sets g2 HIGH.

That's the cause of the dimming you observe.

ieee448,
it's not a debounce issue because I just changed the pushbuttons for switches and get the same problem.

wildbill:
If no buttons are pressed, the three else clauses will execute.
The else for val1 sets g2 LOW, turning the LED off.
Very shortly afterwards, the else for val2 runs which sets g2 HIGH.
That's the cause of the dimming you observe.

wildbill,
that is what I thought.

So how do I get around this?

Rather than your if else structures controlling the lighting, consider basing your lights on what block the locomotive is in.

So if val1 is pressed, set a block variable to two. Have an if that says what you do when block two is occupied. No else. Once the train moves to another block, it's if should set all the lights to the appropriate states. No more dimming.

I think this will work for your current simulation. When there are multiple locomotives in play, you will need a more complex model.

There are a couple of folks here knowledgeable about Arduino controlled rail system - hopefully they can help.

wildbill,
in practise the Red signals will only be lit by the presence of the Train having passed that signal, and entered the block ahead, BUT>>>> any Red which is lit must change the previous signal to Yellow.

This must surely be one of the simplest of tasks, but as a complete novice with little more experience than "hello world", I just cannot resolve this.

And I really must resolve this before I can move forward and add more signals.

Just get rid of the else clauses. When a button is pressed, set every single light in the layout appropriately.

wildbill,
I know very little about Arduino codes.

You may know lots (although you are certainly not showing it).

This is suposed to be a forum where people like me, can ask people like you, for help.

If you can, and will, then help, I would be most grateful; and it would no doubt benefit any others who may read this post looking for solutions to their problems.

If you cannot, and/or, will not, then please do not add to this post.

Is it essential that random words be underlined or bold?

@wildbill had given you advice - have you taken it?
Tried it?

What did it do?
Did it meet your expectations?

(Pins 14, 15, and 16 have perfectly functional, built-in pull up resistors - No need to provide your own.
And, when variable names start getting numeric suffixes, it's time to look at arrays)

I know very little about Arduino codes.

But for all we know, you could be a demon COBOL or Lisp programmer.

TheMemberFormelyKnownAsAWOL,
I have no idea what COBOL or Lisp is.

As for wildbill's "advice", one of his suggestions was...
"When a button is pressed, set every single light in the layout appropriately."

I don't exactly call that advice; nor is it feasible, or practical, and indeed, my problem relates to a simple setup with just 3 signals. If someone could help me with this, then I could gain more confidence in using the Arduino.

AS for...
"consider basing your lights on what block the locomotive is in.
So if val1 is pressed, set a block variable to two. Have an if that says what you do when block two is occupied. No else. Once the train moves to another block, it's if should set all the lights to the appropriate states.".

I have absolutely no idea what that means, or, how to implement it.

I have come on here looking for help, and it seems from this thread (and many others) there are a vast number of people who like to "tease" us less able folks with "advice" about "try this" and "try that" without actually explaining to us how to "try this" and "try that".

The Arduino is clearly a hugely popular, and, very capable device, and it is such a shame that those with the most knowledge seem so reluctant to help the rest of us.

Moving on, I see you have almost 8,000 posts, so I presume you know a thing or two about a thing or two, and if you could assist me to overcome the problems I am having I would be most grateful.

deltech:
As for wildbill's "advice", one of his suggestions was...
"When a button is pressed, set every single light in the layout appropriately."

I don't exactly call that advice; nor is it feasible, or practical, and indeed, my problem relates to a simple setup with just 3 signals. If someone could help me with this, then I could gain more confidence in using the Arduino.

I don't know why you feel it's not feasible or practical - it seems like an eminently safety-concious, defensive programming technique to me.
It would take a tiny fraction of a second, and you, as a slow-reacting mammal wouldn't notice the delay.

At this stage, if I were you, I'd be looking at ways of factoring the code, using function(s) to control a single set of lights, probably represented by an array, or an array of struct.

Your current approach using simple variables is a dead-end as far as future expansion is concerned, and the sooner you drop it, the sooner you'll be able to see workable solutions.

The IDE has many useful worked examples built-in ("Files/Examples"), and some could be adapted to your purposes.

Ok, man. Let's put this the other way around. Your problem is:

When I operate pushbutton 1 (to simulate that the Train has now passed signal-1 and is being detected in the block section ahead of that signal, signal-1 correctly displays a Red aspect, and signal-2 correctly displays a Yellow aspect, and signal-3 correctly remains unchanged at Green.

BUT>>>> the Green aspect in Signal 1 goes dim instead of OFF, and the Green aspect in signal-2 remains Green (full brightness).

When you push button 1, signal 1 displays a dim green glow and it shouldn't.

Fine.

Let's copy your code and pull out everything except the stuff that's relevant to signal 1 green.

const int G1 = 4;       // choose pin for signal 1 GREEN aspect
const int inPin1 = 14;      // choose pin for pushbutton 1
int val1 = 0;         // variable for reading the pin status
const int inPin2 = 15;      // choose pin for pushbutton 2
int val2 = 0;         // variable for reading the pin status
const int inPin3 = 16;      // choose pin for pushbutton 3
int val3 = 0;         // variable for reading the pin status

void setup() {
  pinMode(G1, OUTPUT);      // set pin as output
  pinMode(14, INPUT);       // set pin as input
  pinMode(15, INPUT);       // set pin as input
  pinMode(16, INPUT);       // set pin as input
  digitalWrite(G1, HIGH);   // turn G1 ON
}

void loop() {
  val1 = digitalRead(inPin1); // read input value 1
  if (val1 == LOW) {          // check if input 1 is LOW (button 1 pressed)
    digitalWrite(G1, LOW);    // turn G1 OFF
  } else {
    digitalWrite(G1, HIGH);   // turn G1 ON
  }

  val2 = digitalRead(inPin2);  // read input value 2
  if (val2 == LOW) {          // check if input 2 is LOW (button 1 pressed)
  } else {
  }

  val3 = digitalRead(inPin3);  // read input value 3
  if (val3 == LOW) {          // check if input 3 is LOW (button 1 pressed)
    digitalWrite(G1, LOW);    // turn G1 OFF
  } else {
    digitalWrite(G1, HIGH);   // turn G1 ON
  }
}

Now, this loop() function gets called several hundred thousand times a second. What happens when inPin1 is low and the other two pins are high? Let's pull out all the code except the stuff relevant to that scenario:

void loop() {
  val1 = digitalRead(inPin1); // read input value 1
  if (val1 == LOW) {          // check if input 1 is LOW (button 1 pressed)
    digitalWrite(G1, LOW);    // turn G1 OFF
  } else {
  }

  val3 = digitalRead(inPin3);  // read input value 3
  if (val3 == LOW) {          // check if input 3 is LOW (button 1 pressed)
  } else {
    digitalWrite(G1, HIGH);   // turn G1 ON
  }
}

Each pass through loop(), your code checks inPin1 and unlights g1, then it checks inPin3 and lights g1. That's why g1 appears dim: it's being lit and unlit repeatedly very fast.

The MemberFormerlyKnowAsAWOL,
thank you for replying again.

The problem to a numpty like me (and any other numpty) is when people suggest "factoring the code, using function(s), arrays" is us numptys do not know what that means.

Anyway, moving on, I have attemtped a different approach to the code, where there will still be 3 signals in a virtual circular track, but this time I have considered there to be "Blocks" of track between each signal, so 3 blocks in my situation.

If the Block ahead of any signal is clear, and the following block is also clear, the signal should show Green.

If the Block ahead of any signal is clear, but the next Block is occupied, then the signal should show Yellow.

If the block ahead of any signal is occupied, then the signal should show Red.

With this approach, no signal is controlled by any other signal; simply by what block(s) are clear/occupied, but using the && function.

I include the code below, but again, it does not work.

In fact, when I run the code all 3 colours are displayed on all 3 signals!!!

I am beginning to tear out what little hair I have left.

[code]
const int R1 = 2;        // choose pin for signal 1 RED aspect
const int Y1 = 3;       // choose pin for signal 1 YELLOW aspect
const int G1 = 4;       // choose pin for signal 1 GREEN aspect
const int R2 = 5;       // choose pin for signal 2 RED aspect
const int Y2 = 6;       // choose pin for signal 2 YELLOW aspect
const int G2 = 7;       // choose pin for signal 2 GREEN aspect
const int R3 = 8;       // choose pin for signal 3 RED aspect
const int Y3 = 9;       // choose pin for signal 3 YELLOW aspect
const int G3 = 10;        // choose pin for signal 3 GREEN aspect

const int inPin1 = 14;      // choose pin for pushbutton 1
int val1 = 0;         // variable for reading the pin status
const int inPin2 = 15;      // choose pin for pushbutton 2
int val2 = 1;         // variable for reading the pin status
const int inPin3 = 16;      // choose pin for pushbutton 3
int val3 = 2;         // variable for reading the pin status


void setup()
{
  pinMode(R1, OUTPUT);         // set pin as output
  pinMode(Y1, OUTPUT);         // set pin as output
  pinMode(G1, OUTPUT);         // set pin as output
  pinMode(R2, OUTPUT);         // set pin as output
  pinMode(Y2, OUTPUT);         // set pin as output
  pinMode(G2, OUTPUT);         // set pin as output
  pinMode(R3, OUTPUT);         // set pin as output
  pinMode(Y3, OUTPUT);         // set pin as output
  pinMode(G3, OUTPUT);         // set pin as output
  pinMode(14, INPUT);          // set pin as input
  pinMode(15, INPUT);          // set pin as input
  pinMode(16, INPUT);          // set pin as input

  // start with all signal showing Green

  digitalWrite(R1, LOW);      // turn R1 OFF
  digitalWrite(Y1, LOW);      // turn Y1 OFF
  digitalWrite(G1, HIGH);     // turn G1 ON
  digitalWrite(R2, LOW);      // turn R2 OFF
  digitalWrite(Y2, LOW);      // turn Y2 OFF
  digitalWrite(G2, HIGH);     // turn G2 ON
  digitalWrite(R3, LOW);      // turn R3 OFF
  digitalWrite(Y3, LOW);      // turn Y3 OFF
  digitalWrite(G3, HIGH);     // turn G3 ON

}


void loop() {

  val1 = digitalRead(inPin1); // read input value 1
  val2 = digitalRead(inPin2); // read input value 2
  val3 = digitalRead(inPin3); // read input value 3


  if (val1 == HIGH && val2 == HIGH);
  {
    digitalWrite(G1, HIGH); // turns on G1
  }

  if (val1 == HIGH && val2 == LOW);
  {
    digitalWrite(Y1, HIGH); // turns on Y1
  }

  if (val1 == LOW);
  {
    digitalWrite(R1, HIGH); // turns on R1
  }


  // REPEAT FOR NEXT SIGNAL

  if (val2 == HIGH && val3 == HIGH);
  {
    digitalWrite(G2, HIGH); // turns on G2
  }

  if (val2 == HIGH && val3 == LOW);
  {
    digitalWrite(Y2, HIGH); // turns on Y2
  }

  if (val2 == LOW);
  {
    digitalWrite(R2, HIGH); // turns on R2
  }


  // REPEAT FOR NEXT SIGNAL

  if (val3 == HIGH && val1 == HIGH);
  {
    digitalWrite(G3, HIGH); // turns on G3
  }

  if (val3 == HIGH && val1 == LOW);
  {
    digitalWrite(Y3, HIGH); // turns on Y3
  }

  if (val3 == LOW);
  {
    digitalWrite(R3, HIGH); // turns on R3
  }

}


// END

[/code]

  if (val1 == HIGH && val2 == HIGH);

In all the example code you've worked through, you have never, ever seen a semicolon at the end of an "if" like that.

And if you think the logic through you'll see that nowhere in loop() does any LED ever get switched OFF. So once one has been switched ON it's on forever.

Steve

PaulMurrayCbr,
thanks for reply, which was submitted whilst I was typing a response to TheMemberFormerlyKnownAsAWOL.

Yes, I gathered from the very first time I ran the code, that the Greens are getting conflicting instructions to be on and off at the same time, so I have taken the advice of abandoning that approach.

I have now taken a different approach, breaking the track into Blocks, and using the && function to determine which colour should illuminate on any given signal, but unfortunately it will not work either, as explained in my previous response, and I hope that someone can give me some guidance with this second problem.

TheMemberFormerlyKnownAsAWOL, and slipstick,
thank you for the replies. They have been very helpful, and I will now attempt to address the points you both highlighted.

I believe you are missing a rule. When a signal is turned to red, you turn the previous one to yellow - what you appear to have missed is that you also need to change the one previous to that to green, other wise you leave a signal hanging on yellow. Presumably you are working on the principle a signal should only be one colour?