Switch/case problem

I was doing just fine reshaping bits of code from here and there for block control on a model railway signalling system.

The sketch uses a sensor to set danger at the start of a block and another sensor to reset the block occupancy with a delay before proceeding to the second block.

All worked fine for both blocks until I tried to introduce an additional case that would sense a loop point being set against the main line requiring the block status to be overridden and a 'calling on' signal set instead.

This is a very simple system whereby a train moving past the sensors first sets a red, then exits the block setting a yellow which times out to green... and so on for the next block. This all functions, but the (what should be)overriding change of points is totally ignored.

An analog input has been used for the point sensor so that I can monitor and correctly set the sensing, but the case 'GLOOPSET' seems to be being ignored.

I am puzzled, so any help please guys?

working2aspectMar10.ino (6.35 KB)

Please follow the forum guidelines and post in code tags next time.

//PROGRESS LOG
// This is a copy of2aspectonedirectiontimerMarch72020
//  This now compiles 26 dec 2019.
// Only one block, but yellow is included.. Not sure what POWER does (Provides power to signal that faces the right way)
//Will butcher by eliminating yellow and bi-direction  4 March 20
// OMG!! It actually,,, B... well works!!!!
// Wii now add the second block... Done AM 5 March.. (g) It compliles and block f still functions
//Now to update breadboard! Whoops PM  Block G works, Block f doesn't, LEDs reversed?
//AMFriday 6th MARCH 2020 All functions correctly as far as basic block detection and occupancy goes
//March7,add timer in guise of case yellow and add yellow LEDs for diagnostic purposes only
//March 8 Didn't work compile error green1. Will comment out timer and see.
//March 8 PM works as far as the yellow, but rset of program for block g not yet done.
// March 9 AM tried adding extra 'if' to yellow sub routine.. No change, still hangs on yellow
//March 9th 1610 hrs SHAZAM!!!!  allseems to work as planned.. Have lefyblock f with 5 Secs and blockg with 2 secs
//March 10 this will be used for adding loop point function
//I note that pinmodes for analog IPs are missing.. will add and check Inputs
//March 11 Will add loop point function to block G
// PM March 11..  Loop mod not working
//Mar 12 Will try analog inputs for pointLever

//<code>
int fEntry = A0; //Analog sensors
int fExit = A1;
int gEntry = A2;
int gExit = A3;
int pointLever = A4; // temp jumper

int gantrytwoHome = 3;

// Outputs

int fYellow = 5;// 2 secs delay yellow LED
int fDanger = 8;//red LED
int fCaution = 7; //note caution is actually a green LED
int gYellow = 6; // 2 secs delay yellow LED
int gDanger = 9;//red LED
int gCaution = 10;//Green LED
int callingOn = 11; // activates signal to loop. temp blue LED



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

  pinMode(fEntry, INPUT);
  pinMode(fExit, INPUT);
  pinMode(gEntry, INPUT);
  pinMode(gExit, INPUT);
  pinMode(pointLever, INPUT);
  pinMode(gantrytwoHome, INPUT);

  pinMode(fDanger, OUTPUT);
  pinMode(fCaution, OUTPUT);
  pinMode(gDanger, OUTPUT);
  pinMode(gCaution, OUTPUT);
  pinMode(fYellow, OUTPUT);
  pinMode(gYellow, OUTPUT);
  pinMode(callingOn, OUTPUT);
}
enum BLOCKSTATESF
{
  ST_FUNOCCUPIED,
  ST_FOCCUPIED,
  ST_FYELLOW,
};

enum BLOCKSTATESG
{
  ST_GLOOPSET,
  ST_GUNOCCUPIED,
  ST_GOCCUPIED,
  ST_GYELLOW,


};

BLOCKSTATESF signalStatef = ST_FUNOCCUPIED;

BLOCKSTATESG signalStateg = ST_GUNOCCUPIED;

static unsigned long currentTime;

// This is the main loop containing the switch/ case commands




void loop() {   // this is the main loop

  //Get current time

  currentTime = millis();

  int valA1 = analogRead(fEntry);
  int valA2 = analogRead(fExit);
  int valA3 = analogRead(gEntry);
  int valA4 = analogRead(gExit);
  int valA5 = analogRead(pointLever);
  int valA6 = digitalRead(gantrytwoHome);

  // Serial.println(valA1);
  // Serial.println(valA2);
  // Serial.println(valA3);
  // Serial.println(valA4);
  Serial.println(valA5);
  delay(200);



  switch (signalStatef)
  {
    case ST_FUNOCCUPIED:
      signalgreen(valA1, valA2);
      break;
    case ST_FOCCUPIED:
      signalred(valA1, valA2);
      break;
    case ST_FYELLOW:
      signalyellow(valA1, valA2);
      break;
  }



  switch (signalStateg)
  {
    case ST_GLOOPSET:
      pointset(valA5);
      break;
    case ST_GUNOCCUPIED:
      signalgreen1(valA3, valA4);
      break;
    case ST_GOCCUPIED:
      signalred1(valA3, valA4);
      break;
    case ST_GYELLOW:
      signalyellow1(valA3, valA4);
      break;

  }
}


// These are the sub routines associated with each case..

//Block F


void signalgreen(int valA1, int valA2) {   // Sets green (so called caution) LED if neither sensor  is detecting.
  digitalWrite(fCaution, HIGH);
  digitalWrite(fDanger, LOW);
  digitalWrite(fYellow, LOW);


  if (valA1 < 500 && valA2 > 500) {  // Sets Occupied state if fEntry detects but not fExit
    signalStatef = ST_FOCCUPIED;


  }
}


void signalred(int valA1, int valA2) { // OCCUPIED turns green led off but red on
  digitalWrite(fCaution, LOW);
  digitalWrite(fDanger, HIGH);
  digitalWrite(fYellow, LOW);


  if (valA1 > 500 && valA2 < 500) {   // train is leaving block
    signalStatef = ST_FYELLOW;
  }
}

void signalyellow(int valA1, int valA2) { //sets delay

  const unsigned long duration = 5000;
  static unsigned long  startTime = 0;
  if (startTime == 0) {

    digitalWrite(fCaution, LOW);
    digitalWrite(fDanger, LOW);
    digitalWrite(fYellow, HIGH);
    startTime = currentTime;
    return;

  }

  if (currentTime - startTime >= duration) {
    startTime = 0;


    if (valA1 > 500 && valA2 > 500) {  // train has left block
      signalStatef = ST_FUNOCCUPIED;
    }


  }
}


//good up to here!
//Block G

void signalgreen1(int valA3, int valA4) {   // Sets green LED if neither sensor  is detecting.
  digitalWrite(gCaution, HIGH);
  digitalWrite(gDanger, LOW);
  digitalWrite(gYellow, LOW);

  if (valA3 < 500 && valA4 > 500) {  // Sets Occupied state if gEntry detects but not fExit
    signalStateg = ST_GOCCUPIED;
  }

}



void signalred1(int valA3, int valA4) { // OCCUPIED turns all other leds off but red on
  digitalWrite(gCaution, LOW);
  digitalWrite(gDanger, HIGH);
  digitalWrite(gYellow, LOW);

  if (valA3 > 500 && valA4 < 500) {   //  train is leaving block
    signalStateg = ST_GYELLOW;

  }
}
void signalyellow1(int valA3, int valA4) { //sets delay
  const unsigned long duration = 2000;
  static unsigned long  startTime = 0;
  if (startTime == 0) {

    digitalWrite(gCaution, LOW);
    digitalWrite(gDanger, LOW);
    digitalWrite(gYellow, HIGH);
    startTime = currentTime;
    return;
  }
  if (currentTime - startTime >= duration) {
    startTime = 0;


    if (valA3 > 500 && valA4 > 500) {  // train has left block
      signalStateg = ST_GUNOCCUPIED;
    }
  }

}

void pointset(int valA5) {
  digitalWrite(gCaution, LOW);
  digitalWrite(gDanger, HIGH);
  digitalWrite(gYellow, LOW);
  digitalWrite(callingOn, HIGH); // Servo needs low to activate

  if (valA5 >= 900) { /// Loop point is switched to loop not main
    signalStateg = ST_GLOOPSET;
    //    delay(6000);
    digitalWrite(callingOn, HIGH);

  }

}

// </code>

Regarding the analog point sensor, have you tried writing a simple test sketch that only does that, so you can rule out programming errors in the greater program, or hardware problems?

Done the analog prints.. That is why I made the input analog and all this functions as it should. I set the level to roughly the middle in the if statement, so that should work .. I did all the usual fiddles to ensure that there is no issue with the hardware. Your comment about the chicken and egg situation of the GLOOPSET status seems to be nearer the mark though...

It seems very simple, but..... looks like a one liner somewhere should fix it

dickmoger:
Done the analog prints..

Great! Can you please advise us as to the results? Did you try printing the captured analog value 'valA5'?

Sure.. With the jumper between +5v and port A4 (Val A5, yes I know that's silly) I read 950 on the serial monitor.
Jumper out it reads 530 ish
I have tried various values between with no effec.

dickmoger:
Sure.. With the jumper between +5v and port A4 (Val A5, yes I know that's silly) I read 950 on the serial monitor.
Jumper out it reads 530 ish
I have tried various values between with no effec.

That sounds like just a port test. What happens when it is connected to your actual hardware? I think it's time to ask for a wiring diagram...

Wiring is pretty basic and to test it I downloaded a quick sketch based on the basic button sketch and simply changed the port assignments to light the blue (callingOn) LED when the jumper between input port A4 was lifted and replaced.. This worked fine. I haven't checked this recently though and you seem to be suggesting that the problem smells of hardware rather than software? If so, I am delighted, as this would be a first!

I will go back and recheck because ageing eyesight does tend to produce errors.

If I need to come back, what is the process that you prefer for providing a wiring diagram?

The theoretical will reveal little as it is so basic:

Analog inputs go straight from the TCRT sensor analog O/Ps to the appropriate analog i/p port and all outputs go direct to the end of a 150 Ohm R whose other end goes to the + leg of a LED, the neg of which goes to the neg buss on the breadboard. Only LEDs are being used as outputs at this stage and the only non sensor i/p is the jumper between port A4 and the V+ (Which I have verified by the button sketch).

Again this (starter kit) breadboard is so small that I am not sure that a photo will be all that clear.

In the meantime, I will go back and double-check the hardware.

Thank you again for your interest...

Have double checked everything and even reversed all the values. but to no avail. I am enhancing a version of the button sketch to use as a future hardware diagnostic which checks all the i/ps and O/ps in turn, but otherwise, I am satisfied that there is no hardware error.
So it's back to my programming..... Any ideas guys?

dickmoger:
Wiring is pretty basic and to test it I downloaded a quick sketch based on the basic button sketch and simply changed the port assignments to light the blue (callingOn) LED when the jumper between input port A4 was lifted and replaced.. This worked fine.

Is that jumper test also the test that you use to check the operation of the complete program? Or do you have it actually connected to some kind of sensor when the stated problem is observed? What is the high and low reading of the sensor from an analogRead() when the sensor is connected?

Verbal descriptions really don't cut it when it comes to electronics. Please post a real diagram. It will help to clarify the operation of the software, even if there is no hardware problem.

Also, please elaborate on, "'GLOOPSET' seems to be being ignored". Seems? What is the significance of that variable? Why do you think it is being ignored? You do mean 'ST_GLOOPSET' right?

You have a problem with an unreachable state. Here you call pointset() only if signalStateg == ST_GLOOPSET:

  switch (signalStateg)
  {
    case ST_GLOOPSET:
      pointset(valA5);
      break;

but the only place other than initialization, that ST_GLOOPSET is modified, is in pointset():

void pointset(int valA5) {
 [...]
  if (valA5 >= 900) { /// Loop point is switched to loop not main
    signalStateg = ST_GLOOPSET;

pointset() is never called from any other place in the program.

I do not wish to delve into your program logic at all, but I'm sure this is not what you intended.

You have declared 'valA5' twice in different scope. Using the same identifier for different variables in different scope is a bad programming habit.

Can you post the current version of your code?

Not sure where it went, there was a post made a few days ago about the problem with signalStateg never being set to ST_GLOOPSET except in the pointset() function.

Sounds like my 'chicken and egg' suspicion!

Will check and come back..

Thanks so far..

Yeah I accept that the Vals have got a bit out of hand as the ambition has outgrown the original scope

When you do come back, please bring a system diagram with you. I started to dig into your code, and found it extremely hard to follow. You had some good ideas in the beginning, but your abstractions got lost... for example, you rightly declare well named pin number variables for I/O:

const uint8_t fEntry = A0; //Analog sensors

(note the change I made from 'int' to 'const uint8_t'. You should change all of them to that)
fEntry is fairly obvious in meaning, except that it doesn't say that it is a pin. However later you obfuscate it by assigning it thus:

 int valA1 = analogRead(fEntry);

Now, not only is the value associated with the "Entry" function not named as such, It is horrendously misleading because it is reminiscent of pin A1, it is actually on pin A0.
Then you further muddy the waters by passing the value to a function that declares its own variable 'valA1':

 switch (signalStatef)
  {
    case ST_UNOCCUPIED:
      signalgreen(valA1, valA2);

[...code...]
void signalgreen(int valA1, int valA2) {   // Sets green (so called caution) LED if neither sensor  is detecting.
  digitalWrite(fCaution, HIGH);
  digitalWrite(fDanger, LOW);
  digitalWrite(fYellow, LOW);

  if (valA1 < 500 && valA2 > 500) {  // Sets Occupied state if fEntry detects but not fExit

I seriously doubt that you need two different enums for signal states so I recommend unifying them:

enum BLOCKSTATES
{
  ST_UNOCCUPIED,
  ST_OCCUPIED,
  ST_YELLOW,
  ST_LOOPSET,
};

BLOCKSTATES signalStatef = ST_UNOCCUPIED;
BLOCKSTATES signalStateg = ST_UNOCCUPIED;

So far I have only addressed nomenclature and a bit of structure. Actually, the architecture and logic of the program have serious problems. I will not go into that right now because it is not feasible to address them until a clear picture of how the Arduino fits into the system (i.e. wiring diagram) and a program that is logically organized, properly factored and modularized, and well documented, is available in this thread.

OK.. So here is the wiring diagram.

Imagine the train enters from the top left and descends past sensors fEntry, fExit and the gEntry and finally gExit.

fClear (green) is lit until a train is sensed at fEntry which then sets fDanger (red) This remains lit until fExit detects the train and sets the timer and the fYellow LED. When this has expired, the block reverts to clear status (ST_FUNOCCUPIED).... And so on for blockG . This all works fine, but there is a loop point between the F and G blocks, so I need to override the status of BlockG (with ST_GLOOPSET), set the main line signal to danger but set the loop signal to clear (Blue LED 'callingOn'...) This last doesn't work as the function of the switch is completely ignored.

Following comments here, I have tried to turn it into a 'toggle' by adding a new state ST_GLOOPUNSET.

In theory, when I plug in my jumper to simulate the point switch, The callingOn blue LED should light as well as the gDanger LED. Alas nothing changes and the block process continues as if nothing has happened.

Not sure how to download all this, but will give it a go... It may take a couple of goes.

working2aspectMar15.ino (6.89 KB)

dickmoger:
OK.. So here is the wiring diagram.

Imagine [...]

I don't mean to be cruel, but verbal descriptions (one member here calls them Shakespeare diagrams) are not diagrams. There is a reason why they are not used much in electronics - they require that you build a complex mental picture in your mind. Please provide an actual diagram if you want people to understand. It's really standard.

Also what you have provided is not a wiring diagram - it's a functional system description (describing the application objects and behaviour). Not a single wire connection has been mentioned here.

In the meantime, I will attempt to digest your analysis.

What is a "loop point" and "loop signal"?

Your new sketch should fit inline in code tags since it's only ~7kb. Please go back and edit your last post to make that happen.

Trying to get it to you, but got a 5 minute posting block! something just for newbies apparently?

Missed where to find the symbols in the process..

Am being blocked by the system.. It restricts newbies to so called 5 minute intervals..

Will try for the fourth time

OK .. I used quick reply before and found its limitations.. Five minutes for beginners and also no obvious code symbols..

Check the stickies... if you follow the links all the way, there are images showing the necessary steps. Thank you for making the effort, truly. And for the patience with the forum restrictions...

Phew.. there is a conspiracy in cyberspace that seems to be preventing progress here.. Mostly my finger trouble, but Word decided to add Docx to my diagram!

OK Umpteenth time

2aspectsketchwiring.doc (39 KB)