Two way Traffic Light System Design

tumbleweed

Joined Jun 27, 2023
19
There are two issues with that code...

First, the variables red_timer, green_timer, and yellow_timer should be declared as 'volatile unsigned long' so that the compiler will know they are being modified outside the main loop. Otherwise it may optimize the code out.

Secondly, the three xxx_timer variables are unsigned longs (32-bits). On an 8-bit processor reading/writing them will be non-atomic and require manipulating four bytes. When you access them outside the ISR you have to account for the fact that you might get interrupted right in the middle of reading/writing to them in the main loop. Either disable interrupts around the accesses or write functions to repeat the operation until you get the desired result, ie when writing to the variable repeat setting it until the value reads what you're attempting to write, and when reading read it multiple times until two successive reads are equal.
That way you know you were not interrupted during the operation.
 

tumbleweed

Joined Jun 27, 2023
19
You're using an 8-bit processor, so to set or compare a 32-bit value will require at least 4 operations.
If you get interrupted in the middle of these, the operation may end up using invalid values if, say an overflow from one byte into another happens.

This isn't necessarily in your code, but say red_timer = 00 00 FF FF. An increment operation would result in the sequence 00 00 FF FF -> 00 00 FF 00 -> 00 00 00 00 -> 00 01 00 00 as the 32-bit value is changed. So, statements like 'if (red_timer >= 20000)' could result in an invalid comparison if interrupted. A similar thing can happen with 'red_timer = 0;' assignment depending on the value of red_timer when the interrupt occurs.
 

BobTPH

Joined Jun 5, 2013
11,533
Would it be possible to provide some way for elderly/disabled pedestrians to enable the normal period to be extended so that they can cross safely?
I am elderly (71) and I often cross diagonally when there is a walk light both ways. I have to walk very fast to make it in time!
 

BobTPH

Joined Jun 5, 2013
11,533
This is a simple sequential program. I would never do it by using timers for each light, that is an unnecessarily complicated method.

Here is how a would do it:

1. Identify each unique state and their sequence. For example.

State 1:

NS green
EW red
Ped red
time 10 sec.

State 2:

NS Yellow
EW red
Ped red
time 5 sec

State 3:
All red
time 2 sec

State 4:
NS red
EW green
time 10 sec



Then it is a simple task to implement each state in a sequential program and put it in a loop.
 

Thread Starter

Kittu20

Joined Oct 12, 2022
511
1. Identify each unique state and
NS green
EW red
Ped red
time 10 sec.
I'm using timer interrupts to avoid writing blocking code because there are several features I plan to add later if I get the basic functionality working.

Regarding the timing in each state in your example , can you confirm if its blocking or non-blocking delays?
 

BobTPH

Joined Jun 5, 2013
11,533
I'm using timer interrupts to avoid writing blocking code because there are several features I plan to add later if I get the basic functionality working.

Regarding the timing in each state in your example , can you confirm if its blocking or non-blocking delays?
Could be either. You need an RTOS get non-blocking. Or you could implement it as a state machine where each state schedules an interrupt at the end of its’s time interval, and the handler advances the state.
 

Thread Starter

Kittu20

Joined Oct 12, 2022
511
Could be either. You need an RTOS get non-blocking. Or you could implement it as a state machine where each state schedules an interrupt at the end of its’s time interval, and the handler advances the state.
RTOS is also on my wishlist. That's why I've chosen to begin with a state machine approach for now
 

Thread Starter

Kittu20

Joined Oct 12, 2022
511
I have written code that addresses the timing as shown in the table for traffic light system. Its work fine for me

1697031261978.png

However, I must admit that this code is not ideal, and there is some cleaning up to do. I'm working on making the code more easier to understand.

C:
#include <xc.h>
#include <stdio.h>

#define RED_PIN_1    LATB0
#define GREEN_PIN_1  LATB1
#define YELLOW_PIN_1 LATB2

#define RED_PIN_2    LATB3
#define GREEN_PIN_2  LATB4
#define YELLOW_PIN_2 LATB5

#define PEDESTRIAN_CROSS_BLUE LATB6
#define PEDESTRIAN_DONT_CROSS_RED LATB7

enum LightState {
    RED_STATE,
    YELLOW_STATE,
    GREEN_STATE
};


enum LightState currentLightState = RED_STATE;


volatile unsigned int elapsedTime = 0;

void initializeHardware() {
    TRISB0 = 0;
    TRISB1 = 0;
    TRISB2 = 0;

    TRISB3 = 0;
    TRISB4 = 0;
    TRISB5 = 0;
    TRISB6 = 0;
    TRISB7 = 0;

    LATB = 0x00;

    RED_PIN_1 = 1;
    YELLOW_PIN_1 = 0;
    GREEN_PIN_1 = 0;

    RED_PIN_2 = 1;
    YELLOW_PIN_2 = 0;
    GREEN_PIN_2 = 0;
  
    PEDESTRIAN_CROSS_BLUE = 1;
    PEDESTRIAN_DONT_CROSS_RED = 0;

    ANCON0 = 0x00;
    ANCON1 = 0x00;

    INTCON = 0;

    INTCONbits.GIE = 1;
    INTCONbits.PEIE = 1;
}

void initializeTimer2(void) {
    T2CONbits.T2OUTPS = 0b0000;
    T2CONbits.TMR2ON = 1;
    T2CONbits.T2CKPS = 0b10;
    PR2 = 124;

    PIE1bits.TMR2IE = 1;
}

void main(void) {
    initializeHardware();
    initializeTimer2();

    while (1) {
        unsigned int tempElapsedTime = elapsedTime;

        // Temporarily disable global interrupts
        INTCONbits.GIE = 0;

        // Re-enable global interrupts
        INTCONbits.GIE = 1;

        // Update traffic light states and pedestrian signals
        switch (currentLightState) {
            case RED_STATE:
                if (tempElapsedTime >= 20000) {
                    currentLightState = GREEN_STATE;
                    INTCONbits.GIE = 0;
                    elapsedTime = 0;
                    INTCONbits.GIE = 1;
                    RED_PIN_1 = 0;
                    GREEN_PIN_1 = 1;
                    YELLOW_PIN_1 = 0;
                  

                    RED_PIN_2 = 0;
                    GREEN_PIN_2 = 1;
                    YELLOW_PIN_2 = 0;

                    PEDESTRIAN_CROSS_BLUE = 0;
                    PEDESTRIAN_DONT_CROSS_RED = 1;
                
                }
                break;
            case GREEN_STATE:
                if (tempElapsedTime >= 20000) {
                    currentLightState = YELLOW_STATE;
                    INTCONbits.GIE = 0;
                    elapsedTime = 0;
                    INTCONbits.GIE = 1;
                    RED_PIN_1 = 0;
                    YELLOW_PIN_1 = 1;
                    GREEN_PIN_1 = 0;

                    RED_PIN_2 = 0;
                    YELLOW_PIN_2 = 1;
                    GREEN_PIN_2 = 0;

                    PEDESTRIAN_CROSS_BLUE = 0;
                    PEDESTRIAN_DONT_CROSS_RED = 1;
                  
                }
                break;
            case YELLOW_STATE:
                if (tempElapsedTime >= 3000) {
                    currentLightState = RED_STATE;
                    INTCONbits.GIE = 0;
                    elapsedTime = 0;
                    INTCONbits.GIE = 1;
                    RED_PIN_1 = 1;
                    YELLOW_PIN_1 = 0;
                    GREEN_PIN_1 = 0;

                    RED_PIN_2 = 1;
                    YELLOW_PIN_2 = 0;
                    GREEN_PIN_2 = 0;

                    PEDESTRIAN_CROSS_BLUE = 1;
                    PEDESTRIAN_DONT_CROSS_RED = 0;
                }
                break;
        }
    }
}


void __interrupt() ISR(void) {
    PIR1bits.TMR2IF = 0;
    elapsedTime++;
}
Note that I have only used one pair of two LEDs for pedestrian signals, whereas the table mentions two pairs
 
Last edited:

BobTPH

Joined Jun 5, 2013
11,533
Those are not the right kind of states!

The states should reflect the status of the system as a whole, not the status of one LED. Each change of 1 or more lights initiates a new state.

Go back and look at the kind states I identified in post #24. Each state should specify the status of all of lights in all directions.

After all this time, it looks like you basically have no understanding of what a state machine is.
 

eetech00

Joined Jun 8, 2013
4,705
I have written code that addresses the timing as shown in the table for traffic light system. Its work fine for me

View attachment 304704

However, I must admit that this code is not ideal, and there is some cleaning up to do. I'm working on making the code more easier to understand.
Your truth table should show the relationship between inputs and outputs for all scenarios and their outcomes.

For example,

inputs:
pedestrian traffic, no pedestrian traffic
Vehicle traffic, no Vehicle traffic
Button pressed, Button not pressed

Outputs:
Vehicle Traffic R
Vehicle Traffic G
Vehicle Traffic Y
Pedestrian Blu

The number of "states" is basically determined by the number of rows to complete a truth table for the inputs vs outputs above.
From this you can then develop the logic. These would be "run" states.

Additionally, there is an "initialization" state that represents the first system state after power on.
For example, you might want to start with the Red LEDs "flashing.
 

Thread Starter

Kittu20

Joined Oct 12, 2022
511
Those are not the right kind of states!

After all this time, it looks like you basically have no understanding of what a state machine is.
I have designed it as I wanted,

A two-way traffic light system, where the traffic lights for both directions run together or stop altogether, is designed to manage the flow of vehicular traffic and ensure the safety of pedestrians.

In a two-way traffic light system:

  • Green Light: Both directions of vehicles can proceed, and pedestrians should not cross.
  • Yellow Light: A transition warning for vehicles; pedestrians should not start crossing.
  • Red Light: All vehicles come to a stop, and pedestrians are encouraged to cross during this phase. signifying that it's safe for pedestrians to cross.

C:
#include <xc.h>
#include <stdio.h>

// Define hardware pins for vehicle and pedestrian lights
#define RED_PIN_1    LATB0
#define GREEN_PIN_1  LATB1
#define YELLOW_PIN_1 LATB2

#define RED_PIN_2    LATB3
#define GREEN_PIN_2  LATB4
#define YELLOW_PIN_2 LATB5

#define PEDESTRIAN_CROSS_BLUE     LATB6
#define PEDESTRIAN_DONT_CROSS_RED LATB7

// Define states for the traffic lights (common for both directions)
enum VehicleLightState {
    VEHICLE_STOP_STATE,
    VEHICLE_GO_STATE,
    VEHICLE_SLOW_DOWN_STATE
};

enum VehicleLightState currentVehicleLightState = VEHICLE_STOP_STATE;

enum PedestrianLightState {
    PED_STOP_STATE,
    PED_GO_STATE
};

enum PedestrianLightState currentPedestrianLightState = PED_STOP_STATE;

volatile unsigned int elapsedTime = 0;

// Function to initialize hardware pins and interrupt
void initializeHardware() {
    // Set pins as outputs
    TRISB = 0x00;

    LATB = 0x00;  // Clear all pins

    // Set initial states for the lights (common for both directions)
    RED_PIN_1 = 1;
    YELLOW_PIN_1 = 0;
    GREEN_PIN_1 = 0;
    RED_PIN_2 = 1;
    YELLOW_PIN_2 = 0;
    GREEN_PIN_2 = 0;

    PEDESTRIAN_CROSS_BLUE = 1;
    PEDESTRIAN_DONT_CROSS_RED = 0;

    ANCON0 = 0x00;
    ANCON1 = 0x00;

    INTCON = 0;

    INTCONbits.GIE = 1;  // Enable global interrupts
    INTCONbits.PEIE = 1; // Enable peripheral interrupts
}

// Function to initialize Timer2
void initializeTimer2(void) {
    T2CONbits.T2OUTPS = 0b0000;
    T2CONbits.TMR2ON = 1;
    T2CONbits.T2CKPS = 0b10;
    PR2 = 124;

    PIE1bits.TMR2IE = 1; // Enable Timer2 interrupt
}

void main(void) {
    initializeHardware();
    initializeTimer2();

    while (1) {
        // Temporarily disable global interrupts
        INTCONbits.GIE = 0;
      
        unsigned int tempElapsedTime = elapsedTime;

        // Re-enable global interrupts
        INTCONbits.GIE = 1;

        // Update vehicle and pedestrian light states together
        switch (currentVehicleLightState) {
            case VEHICLE_STOP_STATE:
                if (tempElapsedTime >= 20000) {
                    // Transition to VEHICLE_GO_STATE
                    currentVehicleLightState = VEHICLE_GO_STATE;
                    currentPedestrianLightState = PED_GO_STATE;
                    INTCONbits.GIE = 0;
                    elapsedTime = 0;
                    INTCONbits.GIE = 1;

                    // Update the lights
                    RED_PIN_1 = 0;
                    GREEN_PIN_1 = 1;
                    YELLOW_PIN_1 = 0;
                  
                    RED_PIN_2 = 0;
                    GREEN_PIN_2 = 1;
                    YELLOW_PIN_2 = 0;

                    // Update the pedestrian lights
                    PEDESTRIAN_CROSS_BLUE = 0;
                    PEDESTRIAN_DONT_CROSS_RED = 1;  // Pedestrians should not cross during the vehicle "Go" phase
                }
                break;
            case VEHICLE_GO_STATE:
                if (tempElapsedTime >= 20000) {
                    // Transition to VEHICLE_SLOW_DOWN_STATE
                    currentVehicleLightState = VEHICLE_SLOW_DOWN_STATE;
                    currentPedestrianLightState = PED_STOP_STATE;
                    INTCONbits.GIE = 0;
                    elapsedTime = 0;
                    INTCONbits.GIE = 1;

                    // Update the lights
                    RED_PIN_1 = 0;
                    YELLOW_PIN_1 = 1;
                    GREEN_PIN_1 = 0;
                  
                    RED_PIN_2 = 0;
                    YELLOW_PIN_2 = 1;
                    GREEN_PIN_2 = 0;

                    // Update the pedestrian lights
                    PEDESTRIAN_CROSS_BLUE = 0;
                    PEDESTRIAN_DONT_CROSS_RED = 1;  // Pedestrians can't cross as it's not safe
                }
                break;
            case VEHICLE_SLOW_DOWN_STATE:
                if (tempElapsedTime >= 3000) {
                    // Transition to VEHICLE_STOP_STATE
                    currentVehicleLightState = VEHICLE_STOP_STATE;
                    INTCONbits.GIE = 0;
                    elapsedTime = 0;
                    INTCONbits.GIE = 1;

                    // Update the lights
                    RED_PIN_1 = 1;
                    YELLOW_PIN_1 = 0;
                    GREEN_PIN_1 = 0;
                    RED_PIN_2 = 1;
                    YELLOW_PIN_2 = 0;
                    GREEN_PIN_2 = 0;

                    // Update the pedestrian lights
                    PEDESTRIAN_CROSS_BLUE = 1;
                    PEDESTRIAN_DONT_CROSS_RED = 0;  // Pedestrians can now cross as it's safe
                }
                break;
        }
    }
}

// Interrupt service routine for Timer2
void __interrupt() ISR(void) {
    PIR1bits.TMR2IF = 0; // Clear Timer2 interrupt flag
    elapsedTime++;
}
 

BobTPH

Joined Jun 5, 2013
11,533
Okay. I thought two way meant two directions if traffic. How is your system two way? It looks like one way to me?

And it is trivial, will teach you nothing about state machines.
 

Thread Starter

Kittu20

Joined Oct 12, 2022
511
How is your system two way? It looks like one way to me?
Please find diagram in post 1

And it is trivial, will teach you nothing about state machines.
Thank you for the feedback. I understand the concern about the simplicity of my system. I'm committed to taking on more complex challenges and embracing a deeper learning experience. I appreciate any guidance or suggestions on how to do that.

I have a total of 8 LEDs: two green, two yellow, three red, and one blue. Presently, I'm utilizing the blue LED for pedestrian crossing and the red LED for indicating 'don't cross.'

I'm interested in developing a state machine of your choice. Clear statement what needs to do? Could you please provide me with some use cases or scenarios that I can use as a foundation for creating the state machine?

Your suggestions would be greatly appreciated.
 
Last edited:

Thread Starter

Kittu20

Joined Oct 12, 2022
511
It is a "Two-Way Roadway with a Signalized Pedestrian Crossing".
I'm waiting for the opinion of the member @BobTPH . Additionally, I have plans to add new features, such as updating time values over UART, incorporating a temperature sensor, and implementing emergency buttons..etc. Any new features advice will be appreciated.
 

BobTPH

Joined Jun 5, 2013
11,533
I was thinking of the lights at a crossroads. There are 4 sets of lights. The East and West would be green when the North and South are red and vice versa. Normally there is a short time when it is red both ways to allow the intersection ti clear if people running the yellow light. The pedestrian lights could be done in various ways. Around here, we tend to have pedestrians cross both ways at the same time with no cars going during that time.

This makes a little more sense for a state machine. But even that does not involve states that have more than one suceessor.

Now add in buttons to activate the walk lights and you have a more standard, but simple use of a state machine. The next state would depend on whether anyone pushed the walk button or not.
 

eetech00

Joined Jun 8, 2013
4,705
I'm waiting for the opinion of the member @BobTPH . Additionally, I have plans to add new features, such as updating time values over UART, incorporating a temperature sensor, and implementing emergency buttons..etc. Any new features advice will be appreciated.
Is this project an academic exercise? or product development?
 

Thread Starter

Kittu20

Joined Oct 12, 2022
511
Thank you both. I will choose the timing of my choice. Since this is a hobby project aimed at increasing my knowledge
 
Top