Trying to make a standalone signal system...

So ive been messing with the arduino for about 6 months now , with a couple of the different projects just to get an understanding of how everything works and i thought it was time to create something of my own.
I have made a model railroad signal system which contains 6 controlled signals, 6 block inputs, 6ish other inputs, 6 pushbuttons, and 4 turnout switch machines.

I believe i have all the components in my code but im just getting it in the wrong order.
Right now i have it setup to light all the leds on the signals at startup and then wait 6 seconds and sets all leds off except the reds. Also to initialize all 4 switch machines to their normal state.

Now one of the issues is that it automatically sets the signals to all the aspects and because redyel is the last one i coded it ended up on redyel. I want it so itll light all the leds wait 6 seconds and the set signals 1, 2, 5 and 6 to redred and signal 7 and 8 to yel. I want the program to stay on redred until 2 pushbuttons are pressed and the respective route is setup which also changes the signals accordingly. I have also coded out the flash for now just so i can try to minimize the troubleshooting.
i have serial outputs setup for all the needed inputs and outputs and I dont have enough experience yet to understand the problem. Any help would be appreciated.

Thanx in advance
Harrison

And due to the length of the code i had to attach it.
I have it hooked up to a mega 2560.
HP G62 running Windows 7
Each signal is only supposed to have one aspect. (grnred or yelred or redred, etc..)
It shows this:
SIG1 GRNRED
SIG1 YELRED
SIG1 REDRED
SIG1 REDGRN
SIG1 REDYEL
SIG2 grnred
SIG2 yelred
SIG2 redred
SIG2 redgrn
SIG2 redyel
SIG5 grn red
SIG5 yelred
SIG5 redred
SIG5 redgrn
SIG5 redyel
SIG6 grnred
SIG6 yelred
SIG6 redred
SIG6 redgrn
SIG6 redyel
SIG7 grn
SIG7 yel
SIG7 red
SIG7 yelgrn
SIG7 yellun
SIG8 grn
SIG8 yel
SIG8 red
SIG8 yelgrn
SIG8 yellun
sw1 normal
sw1 reverse
sw2 normal
sw2 reverse
sw3 normal
sw3 reverse
sw4 normal
sw4 reverse
SIG1 GRNRED
SIG1 YELRED
SIG1 REDRED
SIG1 REDGRN
SIG1 REDYEL
os1 clr
bk1 clr
bk2 clr
bk3 clr
bk4 clr
bk5 clr
bk6 clr

newwauksouthern.ino (34.4 KB)

Serial.begin(9600); // make sure this matches your speed set in JMRI

  pinMode(0, OUTPUT);
  pinMode(1, OUTPUT);

These three statements can NOT be used in the same code. Ever.

{ 
 //SIG1*********************************
 if (sig1 == grnred){

Why is that open curly brace there? Random curly braces do not make your code more readable.

Why are you comparing the value in sig1 to the value in grnred? You have done nothing to change the values the compiler assigned to either one, so they are both still 0.

After that, I gave up. Most of the code in setup() is useless. You REALLY need to learn about functions. There should be functions for each of the conditions, when they make sense, like sig1 is equal to grnred. Call that function when the condition is true. Test that the function works, and then you can forget about how the function works, and concentrate on the logic in loop().

After you create a loop() function, that is.

I had a very brief look at your code. There is far too much of it to be able to give any advice without spending an hour or two trying to understand it. I'm too lazy for that.

I wrote a demo sketch in the first post in this Thread which illustrates how several different things can be controlled. I also wrote a short sketch here which is an attempt at building a signal interlocking system.

You need to learn how to use arrays and FOR loops rather than line after line of nearly identical code doing things like pinMode(). Not only are all those repetitive lines confusing, they are an open invitation for typing errors.

You need to learn how to divide up your code into short functions that each deal with a single action. This will make it easier to follow the logic of the overall sketch and to debug problems in a particular piece without being confused by other code.

I suggest you start with a very short sketch that just has one of each element. Design it with a view to extending it for multile elements and get it to work before extending it to a second and third element.

...R

this is the help i was looking for. i will look into all that. appreciate the help

thanx
harrison

i posted on here awhile ago about my project and took some of the suggestions and heavily modified my code and for some reason its still doing the same thing. It tries to set all the switches both normal and reverse and every signal all of its available indications. Im not quite sure what Im missing. My idea is to check sensors then if the sensors are reading low to wait until pushbuttons are pressed in a certain order. After the pushbuttons are pressed it initials the route lining switches and setting leds their assigned colors. Ive divided everything into functions as suggested and no difference except faster program speed. Any help is appreciated. Thanx in advance.
And apparently my code is too long still. So i have to upload it.

ElkRiver.ino (24.5 KB)

A link to the previous discussion would be more than welcome, so people know what has been proposed.

I have suggested to the Moderator that this be merged with your previous Thread so that we have easy access to the whole story.

The original Thread at least had a useful title.

I will have a look at your code shortly.

...R

merged ...

I was just about to post the link.

You snooze, you lose! Moving at speed of the internet highway here - dirt slow at times it seems ...

You didn't lose as much as I did - I had a long reply composed and it has disappeared (and I can't blame anyone but myself).

I must make a habit of writing my replies in GEdit and only copying them in when I am done

Anyway ...

Your code is still impossible to follow.

You have so much repetitive stuff that should be in arrays (such as route14, route15 etc).

And you have a ton of detail in loop() that it would take hours to figure out. If I was writing your project my loop() would be something like this

void loop() {
   readDigitalInputs();
   readAnalogInputs();
   analyzeInputs();
   activateSignals();
   activateSwitches(); // I presume these are the same as points
}

You seem to have inputs and outputs all over the place without meaningful names on any of them. For example if (digitalRead(A3) == HIGH) { and digitalWrite(42, HIGH);

You have some functions (such as void blocks() { ) whose names mean little to me, but more importantly, there is so much code around that I can't easily see where they are called.

Sorry, but while it is like this I'm not able to see what might be causing your problems (and I suspect you are faced with the same problem).

...R

You didn't lose as much as I did - I had a long reply composed and it has disappeared (and I can't blame anyone but myself).

Sorry about that, must got caught up in the merge.
I've done the same - long replies now, Ctrl-A, Ctrl-C to highlight & copy before posting in case the post fails/network times out/gets merged from under me ...

Robin2:
Sorry, but while it is like this I'm not able to see what might be causing your problems (and I suspect you are faced with the same problem).

After spending a bit of time looking at the 1,000+ lines of code, the OP really needs to step back and consider how a single signal should work (and just the input(s) that it needs).

Once you get that working, read up on arrays. The amount of code to control 6 signals with 6 inputs shouldn't be that much more than the code for a single signal and input.

Oh yes... you should also read up on scope. You're wasting code in setup doing digitalReads and then just tossing the values (since they are local to setup). Plus you use constructs like:

sig1 = redred;

But you've never defined what "redred" is, so sig1 is just as undefined as before.

Hope this helps,

Brad
KF7FER

EDIT: Should have been more clear - I realize the quote is Robin's and the "You" is meant to be the OP

Alright so still need to look at arrays and scopes. I have taken a glance at arrays but have a hard time thinking up of examples to use it in my code. That's the kind of help I'm looking for. And what I was doing with sig1 = redred is trying to give the variable sig1 a value of redred. And in the signals function it makes the writes to the mega to make the signal redred.
Aka.. Digitalwrite(2,1);
Digitalwrite(3,0);
Digitalwrite(4,0);
Replying from my phone so the code may not be perfect.

I guess what I'm trying to say is that I have a hard time in referencing scopes and things like that to the model railroad without examples. I found a signal system sketch a long time ago and can't seem to find it again.

I make all signals global scope (define in pre-setup code) - lets any code access any variable.
Same with arrays - make them global, let any code access from anywhere.
For example, you could have an array that tracks the left/right state of all switches, or an array with the on/off or red/green state of all LEDs or similar.

Is there a site where I can goto for specific examples that anyone knows of? I'm new to arrays and just got the basic programming knowledge

byte pinArray[] = {2,3,4,5,},

for (x=0; x<4; x=x+1){
pinMode([x], OUTPUT); // set pins 2,3,4,5 as output pins
}

hbingham86:
Is there a site where I can goto for specific examples that anyone knows of? I'm new to arrays and just got the basic programming knowledge

An array is like a card index file.
Each entry (card) can hold some information.
The first entry (card) is number 0 so, if you want the info in the nth entry (card), you look at entry (card) n -1.
All the 'cards' in an array have to hold the information in the same format (E.g. a number (int, long, byte, etc.), a single character, a character string, etc.)
As with card indices, arrays are a convenient way to hold (and retrieve) data.
I'll not confuse you with multi-dimensional arrays here, just let you know they're possible.

I'll try to demonstrate arrays with some stuff taken from your code (forgive me if it's a poor example as I don't undertand how you use the variables.

You have

int sig1;
int sig2;
int sig3;
int sig4;
int sig5;
int sig6;
int sig7;
int sig8;

That would be easier to code if it was like this

byte signals[9];

and then you could refer to a specific signal as signals[6] = HIGH;

NB I have changed int to byte to save memory and I have "wasted" 3 unused array elements (0,1,2) so that you can refer to the signals by the numbers you are familiar with.

The beauty of arrays is when you need to do something to all the members you just need one piece of code - for example

for (byte n = 2, n < 9; n++) {
   signals[n] = LOW;
}

And, for example, it should be possible to deal all your Blocks in 5 or 6 lines of code.

...R