I am trying to program a controller for a Pelican type pedestrian road crossing. It should control the red, amber and green lights for the traffic, the red and green pedestrian lights, and the bleeper. I have written some code but unfortunately it does not work. Any ideas where the problem is? Code size limit is 2K.
Thanks!
Marko

#include <reg51.h>

sbit red_traf=P1^0;
sbit amber_traf=P1^1;
sbit green_traf=P1^2;
sbit red_ped=P1^3;
sbit green_ped=P1^4;
sbit bleeper=P1^5;
sbit button=P2^0;
unsigned char count;

void delay_vshort()
		  	{
		  	unsigned int x;
		  	for (x=0;x<65536;x++);
		  	}

void delay_short()
		  	{
		  	unsigned int x;
		  	for (x=0;x<131072;x++);
		  	}

void delay_long()
		  	{
		  	unsigned int x;
		  	for (x=0;x<1048576;x++);
		  	}


main()
	{
	P2 = 0xff;
	s0:
	green_traf=1;
	red_ped=1;
	if (button==1) goto s1;
	else goto s0;
	
	s1:
	amber_traf=1;
	red_ped=1;
	delay_short();
	goto s2;

	s2:
	red_traf=1;
	green_ped=1;
	bleeper=1;
	delay_long();
	goto s3;

	s3:
	for (count=0;count<5;count++)
		{
		amber_traf = 1;
		green_ped = 1;
		delay_vshort();
		amber_traf = 0;
		green_ped = 0;
		delay_vshort();
		}
		goto s4;
	
	s4:
	amber_traf=1;
	red_ped=1;
	delay_short();
	goto s0;
	}

: I am trying to program a controller for a Pelican type pedestrian road crossing. It should control the red, amber and green lights for the traffic, the red and green pedestrian lights, and the bleeper. I have written some code but unfortunately it does not work. Any ideas where the problem is? Code size limit is 2K.
: Thanks!
: Marko
:

What doesn't work about it? Does it compile?
At any rate, there isn't anywhere to see the output so it's hard to say if it works or not.
Most likely your program will lock up, cause it's effectively an infinite loop without 'rest moments' for the processor.
On what/with what is this code supposed to run?
And I'm curious, what's P1? (PS: You do realize ^ is bitwise xor, right?)

Best Regards,
Richard

The way I see it... Well, it's all pretty blurry

: : I am trying to program a controller for a Pelican type pedestrian road crossing. It should control the red, amber and green lights for the traffic, the red and green pedestrian lights, and the bleeper. I have written some code but unfortunately it does not work. Any ideas where the problem is? Code size limit is 2K.
: : Thanks!
: : Marko
: :
:
: What doesn't work about it? Does it compile?
: At any rate, there isn't anywhere to see the output so it's hard to say if it works or not.
: Most likely your program will lock up, cause it's effectively an infinite loop without 'rest moments' for the processor.
: On what/with what is this code supposed to run?
: And I'm curious, what's P1? (PS: You do realize ^ is bitwise xor, right?)
:
: Best Regards,
: Richard
:
: The way I see it... Well, it's all pretty blurry
:
:
Thanks for a quick response!
The code does compile.(BTW I am using Keil Vision3). The code is supposed to run a 8051 based microcontroller. P1 is an 8-bit output port
of 8051 and P2 is an 8-bit input port of 8051. Bits 0 to 5 of P1 should control the light signals and the bleeper and bit 0 of P2 is supposed to control the pedestrian button(P2 = 0xff means P2 is set as an input port = all bits are 1). The program is supposed to be an infinite loop, traffic lights do usually run continuously.
Regards,
Marko

: Thanks for a quick response!
: The code does compile.(BTW I am using Keil Vision3). The code is supposed to run a 8051 based microcontroller. P1 is an 8-bit output port
: of 8051 and P2 is an 8-bit input port of 8051. Bits 0 to 5 of P1 should control the light signals and the bleeper and bit 0 of P2 is supposed to control the pedestrian button(P2 = 0xff means P2 is set as an input port = all bits are 1). The program is supposed to be an infinite loop, traffic lights do usually run continuously.
: Regards,
: Marko
:

I think I know the problem:

P1^1 means bit P1 xor 00000001 = xxxxxxx1
x is either 0 or 1... it depends on the corresponding bits of P1
I think what you want is (P1 & 00000001) = 0000000x, where x = 0 if P1's first bit is 0 and x = 1 if P1's first bit is 1.

Furthermore, the fourth bit is not given by P1 & 3, but by P1 & 8, or better yet P1 & (1 << 3). Continuing this logic:
Check if the n-th bit of P1 is set <-> P1 & (1 << n)

On a further note, if it eventually is doing something, but not exactly what you'd expect, check if the bits of P1 truly correspond with what you think they do (for instance, the bit order might be reversed and what you think is the 1st bit is actually the 8th bit)

Best Regards,
Richard

The way I see it... Well, it's all pretty blurry

: : Thanks for a quick response!
: : The code does compile.(BTW I am using Keil Vision3). The code is supposed to run a 8051 based microcontroller. P1 is an 8-bit output port
: : of 8051 and P2 is an 8-bit input port of 8051. Bits 0 to 5 of P1 should control the light signals and the bleeper and bit 0 of P2 is supposed to control the pedestrian button(P2 = 0xff means P2 is set as an input port = all bits are 1). The program is supposed to be an infinite loop, traffic lights do usually run continuously.
: : Regards,
: : Marko
: :
:
: I think I know the problem:
:
: P1^1 means bit P1 xor 00000001 = xxxxxxx1
: x is either 0 or 1... it depends on the corresponding bits of P1
: I think what you want is (P1 & 00000001) = 0000000x, where x = 0 if P1's first bit is 0 and x = 1 if P1's first bit is 1.
:
: Furthermore, the fourth bit is not given by P1 & 3, but by P1 & 8, or better yet P1 & (1 << 3). Continuing this logic:
: Check if the n-th bit of P1 is set <-> P1 & (1 << n)
:
: On a further note, if it eventually is doing something, but not exactly what you'd expect, check if the bits of P1 truly correspond with what you think they do (for instance, the bit order might be reversed and what you think is the 1st bit is actually the 8th bit)
:
: Best Regards,
: Richard
:
: The way I see it... Well, it's all pretty blurry
:
:


Well... I am not quiet sure what P1^1 means Port1 bit 1 in your compiler. But most compilers P1.1 means Port1 bit 1.

Assuming that P1^1 means Port1 bit 1, What i have noticed is that you never cleared the green_traf and the red_traf.

I also suggest you avoid using the goto statement ( I am not saying its wrong but it makes the code difficult to read ). I suggest you use the while loop. Maybe you should try it like this:

main()
{
   P1 = 0x00;    //This would clear all LEDs

   while(1)
      {
         green_traf=1;
	 red_ped=1;

         while(!button);  //wait for button to be pressed
         
         green_traf=0;    //switch off green LED
         amber_traf=1;    //switch on Orange LED 
	 delay_short();
         amber_traf=0;    //switch off Orange LED 
    
         for (count=0;count<5;count++)
	  {
		red_traf=1;      //switch on Red LED
		green_ped = 1;
		delay_vshort();
		green_ped = 0;
		delay_vshort();
                green_traf=1;
                delay_short();
                green_traf=0;
                amber_traf=1;    //switch on Orange LED
		delay_vshort();
                amber_traf=0;    //switch off Orange LED
           }
           
         amber_traf=1;
	 red_ped=1;
	 delay_short(); 
         amber_traf=0;
       }
 }

It did not test the code, but i hope it helps in your mission.

loco

: Well... I am not quiet sure what P1^1 means Port1 bit 1 in your compiler. But most compilers P1.1 means Port1 bit 1.
:
: Assuming that P1^1 means Port1 bit 1, What i have noticed is that you never cleared the green_traf and the red_traf.
:

I stand corrected. It's just that I've never seen direct use of a port in C before, so it confused me.
Ignore the (previous) post I made

Best Regards,
Richard

The way I see it... Well, it's all pretty blurry

Thanks loco and Richard that was really useful.
However using the while loop is a bit too advanced for me. Because of my
experience with BASIC dating twenty years ago the "goto" statement is familiar teritory for me, although it is very much disapproved in programming circles. But I am only a beginner and for now I would like to make this program work and than build on the knowledge I will acquire. I have rewritten the code and now it looks like this:

#include <reg664.h>									//Register map for 8051.

sbit red_traf=P1^0;									//Red traffic light is connected to bit 0 of P1.
sbit amber_traf=P1^1;								//Amber traffic light is connected to bit 1 of P1.
sbit green_traf=P1^2;								//Green traffic light is connected to bit 2 of P1.
sbit red_ped=P1^3;									//Red pedestrian light is connected to bit 3 of P1.
sbit green_ped=P1^4;								//Green pedestrian light is connected to bit 4 of P1.
sbit bleeper=P1^5;									//Bleeper is connected to bit 5 of P1.
sbit button=P2^0;									//Pedestrian button is connected to bit 0 of P2.
unsigned char count;

void delay_vshort()									//This defines a very short delay.
		  	{
		  	unsigned int x;
		  	for (x=0;x<65536;x++);
		  	}

void delay_short()
		  	{									   	//This defines a short delay.
		  	unsigned int x;
		  	for (x=0;x<131072;x++);
		  	}

void delay_long()									//This defines a long delay.
		  	{
		  	unsigned int x;
		  	for (x=0;x<1048576;x++);
		  	}


main()
	{
	P1 = 0x00;										//P1 defined as an output port.
	P2 = 0xff;										//P2 defined as an input port.
	s0:
	green_traf=1;									//Green traffic light is on.
	red_ped=1;										//Red pedstrian light is on.
	if (button==1) goto s1;							//Check if button was pressed.
	else goto s0;
	
	s1:
	green_traf=0;									//Switch off green traffic light.
	amber_traf=1;									//Switch on amber traffic light.
	delay_short();									//Leave amber traffic light on for a short time.
	
	s2:
	green_traf=0;									//Switch off green traffic light.
	red_traf=1;										//Swich on red traffic light.										
	red_ped=0;										//Switch off red pedestrian light.
	green_ped=1;									//Switch on green pedestrian light.												
	bleeper=1;										//Switch on the bleeper.
	delay_long();									//Remain in the same state for a long time.
	
    s3:
	red_traf=0;										//Switch off red traffic light.
	for (count=0;count<5;count++)					//This will make amber traffic and green pedestrian light
												   	//flash five times.
		{
		amber_traf = 1;								//Switch amber traffic light on.
		green_ped = 1;							   	//Switch green pedestrian light on.
		delay_vshort();								//Wait for a very short time.
		amber_traf = 0;								//Switch both previous lights off.
		green_ped = 0;
		delay_vshort();								
		}
		
	s4:
	bleeper=0;										//Turn off the bleeper.
	green_ped=0;									//Turn off green pedestrian light.
	amber_traf=1;									//Turn on amber traffic light.
	red_ped=1;										//Turn on red pedestrian light.
	delay_short();									//Wait for a short time.
	goto s0;										//Return to the beginning.
	}

The basic problem is that the program should read the pin of P2(pin 0 to be precise, which is defined as the pedestrian button). This is how I think input and output ports are defined, but it obviously does not work.

main()
	{
	P1 = 0x00;										//P1 defined as an output port.
	P2 = 0xff;										//P2 defined as an input port.
	s0:

When the pedestrian button is pressed(when I tick pin 0 of P2 in the Keil simulator) the program should go to s1 but that does not happen.
Infact when I run the program amber traffic and red pedestrian lights stay on indefinetly.
Any ideas?
Thank again for helping a complete beginner.
Regards,
Marko

: When the pedestrian button is pressed(when I tick pin 0 of P2 in the Keil simulator) the program should go to s1 but that does not happen.
: Infact when I run the program amber traffic and red pedestrian lights stay on indefinetly.
:

What happens if you try to set all Pin's of P2 to 1. Perhaps, like I said, the bit order is reversed/hussled up.
At the very least, try for each pin once.

Best Regards,
Richard

The way I see it... Well, it's all pretty blurry

: : When the pedestrian button is pressed(when I tick pin 0 of P2 in the Keil simulator) the program should go to s1 but that does not happen.
: : Infact when I run the program amber traffic and red pedestrian lights stay on indefinetly.
: :
:
: What happens if you try to set all Pin's of P2 to 1. Perhaps, like I said, the bit order is reversed/hussled up.
: At the very least, try for each pin once.
:
: Best Regards,
: Richard
:
: The way I see it... Well, it's all pretty blurry
:
:

Well its difficult to see where the problem is. Are you able to go through your code line by line with the simulator? And can you toggle the Port pins while you are doing the similations?

What happens if you change if(button==1) to if(button==0)??

:
: Well its difficult to see where the problem is. Are you able to go through your code line by line with the simulator? And can you toggle the Port pins while you are doing the similations?
:
: What happens if you change if(button==1) to if(button==0)??
:
:

Sorted!
Thanks for your help guys!
Regards,
Marko

:
: :
: : Well its difficult to see where the problem is. Are you able to go through your code line by line with the simulator? And can you toggle the Port pins while you are doing the similations?
: :
: : What happens if you change if(button==1) to if(button==0)??
: :
: :
:
: Sorted!
: Thanks for your help guys!
: Regards,
: Marko
:

What was wrong?

:
: What was wrong?
:
:
1.As you pointed out I never cleared the green_traf and the red_traf.
2.I wrote the delay functions in a very stupid way. The 8051 supports max. 16-bit numbers(up to 65535) and two of my delays were using bigger numbers. Interestingly the compiler did not report an error/warning. But when I single-stepped through the program it hanged on the second delay(65536) and I think that it entered an infinite loop or something(the CPU was 100% utilized). I sorted that out and the program is basically working apart from delays wich are way too short.
The problem now is to replace the stupid delay functions with a delay generated by the 8051 internal timer, so that I get three delays(0.5s; 4s and 10s) that can be used instead of the current delay functions.
When I get that sorted out I will try replacing the "goto" statement with the while loop and at the end I will try to extend the whole thing by a lock-out feature so that green_ped cannot go green too quickly after the last time it was pressed(timers again I assume). Hopefully I will get a half decent code at the end and learn a lot in the process.
Any suggestions appreciated.
Thanks again for pointing me in the right direction.
Best regards,
Marko