N/South Traffic lights with two pedestrian buttons using millis()

[meach1]

I did this project to learn millis(). Two traffic lights with Two pedestrian lights and buttons. The traffic works well. There is a small timing issue in the main loop every 7 to 9 cycles. I added some resets to the lights to correct this issue. I would like to get feedback on this code of mine. It works very well, but know it can be better. I am very new to Arduino. I have about 2 months into this rabbit hole that I am loving. Please if anyone give me some improvements within the code. I would appreciate it.

int northLed[] = { 13, 12, 11 };  //13 red 12 yellow 11 green
int northWalking[] = { 10, 9 };   // 10 red 9 green
int southLed[] = { 2, 3, 4};    // 4 red 3 yellow 2 green
int southWalking[]{ 5, 6 };       // 5 green 4 red

int NLT, SLT;    // Led Counters
int nWRT, sWRT;  // pedestrian counters

unsigned long southcurrentTime;    // assigned to millis()
unsigned long southoldTime;        // assigned to millis()
const int southTimeEvent = 3000;   // time millis() for Red and Green lights 3sec
const int southGreenEvent = 1500;  // time millis() needed to exit yellow loop 1.5sec


bool SredLightOn = true;      // exit and start the loop with millis()
bool SyellowLightOn = false;  
bool SgreenLightOn = false;   


unsigned long northcurrentTime;
unsigned long northoldTime;
const int northTimeEvent = 3000;   // time millis() for Red and Green lights 3sec
const int northGreenEvent = 1500;  // time millis() needed to exit yellow loop 1.5sec

bool NredLightOn = true;      //  exit and start the loop with millis()
bool NyellowLightOn = false;  
bool NgreenLightOn = false;   


int buttonN = 8;  // north ligh button
int buttonS = 7;  //  south light button

bool runTime = true;           // allows for one side of the north or southLightContol to run oposite of each other.
int trafficRunTime= 9100;     // time needed to run through either north or southLightControl function.
unsigned long trafficOldTime;  // assigned to millis()


void setup() {
  //enable all pins
  for (NLT = 0; NLT <= 2; NLT++) {
    pinMode(northLed[NLT], OUTPUT);
  }
  for (SLT = 0; SLT <= 2; SLT++) {
    pinMode(southLed[SLT], OUTPUT);
  }
  for (nWRT = 0; nWRT <= 1; nWRT++) {
    pinMode(northWalking[nWRT], OUTPUT);
  }
  for (sWRT = 0; sWRT <= 1; sWRT++) {
    pinMode(southWalking[sWRT], OUTPUT);
  }
  // enable button pins
  pinMode(buttonN, INPUT_PULLUP);
  pinMode(buttonS, INPUT_PULLUP);
  // starts lights in the correct order traffic lights to red and the pedestrain to green
  digitalWrite(northLed[0], HIGH);
  digitalWrite(southLed[2], HIGH);
  digitalWrite(northWalking[1], HIGH);
  digitalWrite(southWalking[0], HIGH);

  Serial.begin(9600);
}

void loop() {
  int nWalkingRunTime = digitalRead(buttonN);  //pedestrain button read for while()
  int sWalkingRunTime = digitalRead(buttonS);


  unsigned long trafficCurrentTime = millis();  // starts time

  if (trafficCurrentTime - trafficOldTime >= trafficRunTime) {
    trafficOldTime = trafficCurrentTime;  // sets the if statement for main loop
    if (runTime == true) {                // used for turning off and on north/southLightControl
      runTime = false;
    } else {
      runTime = true;
    }
  }
  if (runTime == true) {
    for (NLT = 0; NLT <= 2; NLT++) {  // used to reset northLightControl
      digitalWrite(northLed[NLT], LOW);
    }
    for (nWRT = 0; nWRT <= 1; nWRT++) {  // used to reset North Pedestrian light
      digitalWrite(northWalking[nWRT], LOW);
    }
    digitalWrite(northLed[0], HIGH);      // part of North light reset
    digitalWrite(northWalking[1], HIGH);  // part of Pedestrian reset

    southLightControl();  // first main function loop

  } else {

    for (SLT = 0; SLT <= 2; SLT++) {  // used to reset southLightControl
      digitalWrite(southLed[SLT], LOW);
    }
    for (sWRT = 0; sWRT <= 1; sWRT++) {  // used to reset South Pedestrian light
      digitalWrite(southWalking[nWRT], LOW);
    }
    digitalWrite(southLed[2], HIGH);      // part of the South Light reset
    digitalWrite(southWalking[0], HIGH);  // part of the South Pedestrain Light reset

    northLightControl();  // second main function loop
  }

  while (nWalkingRunTime == 0 || sWalkingRunTime == 0) {  // pedestrain button delay() is used, no exit is needed safety first lol
    walkLight();

    return 0;  // exit loop(probably not needed I am still learning)
  }
}


void southLightControl() {

  southcurrentTime = millis();
  //start of the south light Red yellow and then green. bools are used to control the condition with millis().
  if (southcurrentTime - southoldTime >= southTimeEvent && SredLightOn == true) {
    southoldTime = southcurrentTime;

    for (SLT = 0; SLT <= 2; SLT++) {  // used to reset lights
      digitalWrite(southLed[SLT], LOW);
    }

    for (nWRT = 0; nWRT <= 1; nWRT++) {  //resets lights
      digitalWrite(northWalking[nWRT], LOW);
    }
    digitalWrite(northWalking[1], HIGH);  // resets North pedestrian light from Red to Green
    digitalWrite(northWalking[0], LOW);

    digitalWrite(southWalking[0], HIGH);  // turn pedestrian lights from Red to Green
    digitalWrite(southWalking[1], LOW);

    digitalWrite(southLed[2], HIGH);  // turn on Red light
    SyellowLightOn = true;            // controls the loop
    SredLightOn = false;
  }

  //Yellow Light
  if (southcurrentTime - southoldTime >= southTimeEvent && SyellowLightOn == true) {
    southoldTime = southcurrentTime;

    for (SLT = 0; SLT <= 2; SLT++) {  // used to reset lights
      digitalWrite(southLed[SLT], LOW);
    }
    digitalWrite(southWalking[1], HIGH);  //turn pedestrian from Green to Red
    digitalWrite(southWalking[0], LOW);

    digitalWrite(southLed[1], HIGH);  // turn on Yellow Light
    SyellowLightOn = false;           // controls the loop
    SgreenLightOn = true;
  }

  //Green Light
  if (southcurrentTime - southoldTime >= southGreenEvent && SgreenLightOn == true) {
    southoldTime = southcurrentTime;

    for (SLT = 0; SLT <= 2; SLT++) {  // used to reset lights
      digitalWrite(southLed[SLT], LOW);
    }
    digitalWrite(southWalking[1], HIGH);  //turn pedestrian from Green to Red help with the reset of the exit of the while()
    digitalWrite(southWalking[0], LOW);

    digitalWrite(southLed[0], HIGH);  // turn on Green Light
    SredLightOn = true;
    SgreenLightOn = false;

  }
}

void northLightControl() {

  northcurrentTime = millis();
  //start of the north light Red yellow and then green. bools are used to control the condition with millis()
  if (northcurrentTime - northoldTime >= northTimeEvent && NredLightOn == true) {
    northoldTime = northcurrentTime;

    for (NLT = 0; NLT <= 2; NLT++) {  //resets the lights
      digitalWrite(northLed[NLT], LOW);
    }

    for (sWRT = 0; sWRT <= 1; sWRT++) {  //resets the lights
      digitalWrite(southWalking[nWRT], LOW);
    }
    digitalWrite(southWalking[0], HIGH);  //resets the south Pedestrian from Red to Green
    digitalWrite(southWalking[1], LOW);

    digitalWrite(northWalking[1], HIGH);  // turns on Pedestrian from Red to Green
    digitalWrite(northWalking[0], LOW);

    digitalWrite(northLed[0], HIGH);  // turns on Red Light
    NyellowLightOn = true;            // controls the loop
    NredLightOn = false;
  }

  // Yellow Light
  if (northcurrentTime - northoldTime >= northTimeEvent && NyellowLightOn == true) {
    northoldTime = northcurrentTime;

    for (NLT = 0; NLT <= 2; NLT++) {  //resets the lights
      digitalWrite(northLed[NLT], LOW);
    }
    digitalWrite(northWalking[0], HIGH);  // turn on Pedestrian Green to Red
    digitalWrite(northWalking[1], LOW);

    digitalWrite(northLed[1], HIGH);  // turn on Yellow Light

    NgreenLightOn = true;  // controls the loop
    NyellowLightOn = false;
  }

  // Green Light
  if (northcurrentTime - northoldTime >= northGreenEvent && NgreenLightOn == true) {
    northoldTime = northcurrentTime;

    for (NLT = 0; NLT <= 2; NLT++) {  // reset lights
      digitalWrite(northLed[NLT], LOW);
    }
    digitalWrite(northWalking[0], HIGH);  //turn pedestrian from Green to Red help with the reset of the exit of the while()
    digitalWrite(northWalking[1], LOW);

    digitalWrite(northLed[2], HIGH);  // turns on Green Light
    NredLightOn = true;               // controls the loop
    NgreenLightOn = false;

  }
}

void walkLight() {

  // Pedestrain function
  for (NLT = 0; NLT <= 2; NLT++) {  //resets northLightControl
    digitalWrite(northLed[NLT], LOW);
  }
  for (SLT = 0; SLT <= 2; SLT++) {  // resets southLightControl
    digitalWrite(southLed[SLT], LOW);
  }
  digitalWrite(northLed[0], HIGH);  // turns on North Red Light part of reset
  digitalWrite(southLed[2], HIGH);  // turns on South Red Light

  digitalWrite(northWalking[0], LOW);  // Red Pedestrian Light
  digitalWrite(southWalking[1], LOW);

  delay(200);  // delay enought to see the reset

  digitalWrite(northWalking[1], HIGH);  //Turns on Pedestrain light to green
  digitalWrite(southWalking[0], HIGH);
  delay(1500);  // Pedestrian walking time, move it people
  // no need to digtialWrite Low due to resets in the code
}

[gilshultz]

Not sure this will help but the early traffic lights had the front and back connected together, the side was the same but the but has red side to green front , green side to red front, and the yellow was the same on all 4 sides.

[DaveX]

You might like to learn state machines. They help simplify the code from a number of interacting logical variables (SredLightOn, ...) into a structure of mutually exclusive states (enum {ALL_RED, NorthGreen, NorthYellow, SouthGreen, SouthYellow,...}) that helps manage lots of the conditional logic.

Yet another Finite State Machine introduction

Hello everyone, On this gray day, considering the number of questions that boil down to programming a finite automaton (or a state machine), I thought I would translate my small tutorial I also have in French to explain how to approach this type of code in a structured manner. Of course, there are libraries that do this more or less for you, but understanding how it works can often allow you to do without libraries that might weigh down your code. The general idea is to write a program that co…

or

State machines, a short tutorial

A common response to “I want my code to do A., then B., then C., etc.,” from the knowledgeable is “Use a state machine.” Many who are new to coding/programming have never heard of a state machine, although almost everyone is familiar with the concept. There are different ways to realize a state machine, like the very similar Sequential Function Chart, a construct used in Programmable Logic Controllers, but this discussion will deal only with the C++ switch/case construct - which is described bri…

[meach1]

I agree that is next. Through the course of this project I started to notice the state machine function. I do plan on learning/practicing that function. Thanks for the tip.

[noiasca]

As pointed out bei @gilshultz

@gilshultz

Not sure this will help but the early traffic lights had the front and back connected together, the side was the same but the but has red side to green front , green side to red front, and the yellow was the same on all 4 sides.

wouldn't it make more sense to make a

• North-South traffic light (and similar a South-Nord)
• a West-East traffic light (and a similar East-West) traffic light
• walking/ pedestrian lights with "request" buttons

If you want to stick to your North-South South-North naming, it might help if you could provide a diagram / bird view of how you consider your traffic lights to be placed.

[hammy]

Nice neat well laid out breadboard .

The cook book is very good , but not ideal when starting out .As a beginner you might like one of the SimonMonk books

[alto777]

@meach1

There is a small timing issue in the main loop every 7 to 9 cycles

As I understand it, there was a timing issue. Post the code that has that issue but otherwise seemed fine. If you still have that version.

Fixing things that surprise at the late stage of development by throwing some delay in there may work for a little flaw, or two, but by the time it's the way you are fixing more things that may actually be errors or design mistakes will quickly make trash of your sketch.

Perhaps there is no timing issue.

Also, I want little traffic light modules like that. Too cool.

a7

[meach1]

The trafficRunTime in the void loop(), The runtime per light cycle is 7500milli or 7.5seconds. If use that time loop in the trafficRunTime you can tell my resets trigger more often. From Serial monitor and trial and error. It seems 9100/9200 is a good spot for the resets do not trigger as often.

State machine would be a good solution. In my code, I am using flags/bools to keep the logic running correctly. I did try to use a flag method with the trafficRunTime but it did not work well.

[alto777]

@meach1

The trafficRunTime in the void loop(),

OK, so just changing or playing with this constant then

int trafficRunTime= 9100;     // time needed to run through either north or southLightControl function.

will exhibit the problem you've observed?

That I can work with, just confirm to energize me or correct me.

TIA

a7

[meach1]

Yes, that is correct. That part of the logic Ive been playing with different functions but at end the bool with const int works the best for this code.

[alto777]

I played with your code. Mostly just ripping it apart to see how it worked and what it does.

@meach1

State machine would be a good solution.

Indeed. I would go for a full-on state machine approach. Here is a function that is a half-way measure that shows using a switch/case to inform progress throught the south side cycle.

But there is so much natural symmetry in your sketch - the north control is the same as the south control, the prep for being in north mode is identical modulo a few details to the prep for being in north mode that it could be one function and so forth.

# define GREEN   81
# define YELLOW  82
# define RED     83

byte SState = RED;

void southLightControl() {

  southcurrentTime = millis();
  
// is it time to advance the process
  if (southcurrentTime - southoldTime < dwellTime) return;

// yes
  southoldTime = southcurrentTime;
  
  switch (SState) {
  case GREEN :
    SState = YELLOW;

// set lights for yellow here and

    dwellTime = kYellowTime;       // use some constants to say how long to stay Y, R or G
    break;

    case YELLOW :
      SState = RED;
 
// set lights for red here and 

    dwellTime = kRedTime;
    break;
      
    case  RED :
      SState = GREEN;

// set lights for green here and

     dwellTime = kGreenTime;
    break;
  }
}

I could not test that, so take it as just illustration purposes. I can't even say I know this is what southLightControl actually does, so.

a7

[b707]

@meach1

There is a small timing issue in the main loop every 7 to 9 cycles.

I think it's because some of your intervals are signed and some are unsigned.

@meach1

unsigned long southcurrentTime;    // assigned to millis()
unsigned long southoldTime;        // assigned to millis()
const int southTimeEvent = 3000;   // time millis() for Red and Green lights 3sec
const int southGreenEvent = 1500;  // time millis() needed to exit yellow loop 1.5sec

Comparing signed and unsigned values ​​leads to "undefined behaviour", which is what you're seeing in your "issues".
By the way, you should have seen compiler warnings about this if you have verbose compiler output enabled.

[alto777]

@b707

Comparing signed and unsigned values ​​leads to "undefined behaviour"

It would be more accurate to say it can lead to unexpected behaviour. It will make answers that you may not believe or understand, but it dose not have the capability to make you sketch malfunction in the mysterious and weird ways which is called "undefined behaviour".

It is different to "unspecified behaviour".

@meach1 - after more play and perusal, I think your problem that comes and goes or changes with trafficRunTime is due to a design fault that is easy to fix.

Switching control between southLightControl and northLightControl after some arbitrary time, viz:

  if (trafficCurrentTime - trafficOldTime >= trafficRunTime) {

means you are necessarily interrupting to switch in an unsynchronzied manner. You switch when time says to, and you have no idea or control over where in the middle of R->Y->G that direction control is.

So you have to fuss with that time constant so it looks good.

I think the hiccup comes when something doesn't quite make it in time.

Instead, control by one or the other function should be terminated after that direction has performed a number of complete cycles. maybe here just one.

So the trigger to switch hegemons is synchronized, it will always occur when the side being switched from has finiched a complete cycle.

Just make a counter, zero it when you switch, count the cycles one of those functions performs, and when you see that it has done, the rinse and repeat: swap directions, reset the count and let the other side do its thing. Fow a few, or like I say, just once.

HTH

a7