PIC 16F877A traffic system

Thread Starter

Aamil

Joined Jan 2, 2015
23
Dear friends,

I am trying to design a basic traffic system using 16F877A and LEDs.

Code:
#include <stdio.h>
#include <stdlib.h>

#define _XTAL_FREQ 2000000
#include <xc.h>

// BEGIN CONFIG
#pragma config FOSC = HS // Oscillator Selection bits (HS oscillator)
#pragma config WDTE = ON // Watchdog Timer Enable bit (WDT enabled)
#pragma config PWRTE = OFF // Power-up Timer Enable bit (PWRT disabled)
#pragma config BOREN = ON // Brown-out Reset Enable bit (BOR enabled)
#pragma config LVP = OFF // Low-Voltage (Single-Supply) In-Circuit Serial Programming Enable bit (RB3 is digital I/O, HV on MCLR must be used for programming)
#pragma config CPD = OFF // Data EEPROM Memory Code Protection bit (Data EEPROM code protection off)
#pragma config WRT = OFF // Flash Program Memory Write Enable bits (Write protection off; all program memory may be written to by EECON control)
#pragma config CP = OFF // Flash Program Memory Code Protection bit (Code protection off)
//END CONFIG

int main()
{
    PORTA = 0x00;
    TRISA = 0x00; //RA0 as Output PIN


while(1)
  {
        RA0 = 1; 
    __delay_ms(1000);
       RA0 = 0;RA1 = 1; 
        __delay_ms(1000);
    RA1 = 0;RA2 = 1; 
        __delay_ms(1000);
    RA2 = 0;
  }
  return 0;
}
I can download the code to PIC16F877A using pickit3.
But only first LED which is connected to RA0 blinks. None other LEDs are blinking. All LED s are connected along with a 100 Ohm series resistor


Can anyone help?
 

John P

Joined Oct 14, 2008
2,025
I've had enough problems with PIC processor outputs that I just force myself to be defensive and use "shadow ports" and live with the extra time, code space and memory use that it involves. The trouble is Microchip's worst idea ever, the read-modify-write operation. It means that changing a single bit of a port may unpredictably change other pins. Not saying that's the problem here, but I'd avoid having to worry. Every action on a port pin looks like this:
Code:
  unsigned char porta_shadow;

  bit_set(porta_shadow, 0);
  porta = porta_shadow;
 

Thread Starter

Aamil

Joined Jan 2, 2015
23
I've had enough problems with PIC processor outputs that I just force myself to be defensive and use "shadow ports" and live with the extra time, code space and memory use that it involves. The trouble is Microchip's worst idea ever, the read-modify-write operation. It means that changing a single bit of a port may unpredictably change other pins. Not saying that's the problem here, but I'd avoid having to worry. Every action on a port pin looks like this:
Code:
  unsigned char porta_shadow;

  bit_set(porta_shadow, 0);
  porta = porta_shadow;
Sorry.. I could not understand this ..I read about the read-write-modify issue. But what is this shadow port idea? How can I use it to individual pins?
 

John P

Joined Oct 14, 2008
2,025
Sorry if I wasn't totally clear. You create a variable that holds the "shadow" data. You can set or clear its bits as you need to, but you only write to the actual port by means of a full 8-bit transfer, and you never just set or clear a single pin. I thought my little code sample made this clear.

Yes, Port A has fewer than 8 bits. That's OK, you can still write to it. The top bits simply won't have any effect.
 

ErnieM

Joined Apr 24, 2011
8,377
Here's some general hints for using PICs:

1) When an input may be either an analog or a digital input it will *always* default on power up to analog. The reason being is an analog input is more tolerant of wide voltage swings. Thus analog pins must be changed to digital pins.

2) In the data sheet for each and every device the section on the IO ports will have a "SUMMARY OF REGISTERS ASSOCIATED WITH PORT(X)" It is well worth reading until every register is understood and any conflicts with whatever operation you intend to use. For the '877, note that ADCON1 will enable all 8 analog inputs.

3) There is a simulator built into MPLAB. Learn to use it. Especially with faults involving the IO pins there is a general catch phrase I use: "if it doesn't sim(ulate) you can't win." If your part works funky use the simulator to see why.
 
Last edited:

JohnInTX

Joined Jun 26, 2012
4,787
For the '877, note that ADCON1 will enable all 8 analog inputs.
Oops! That's what I get for not verifying the register names. Good catch.
I'm with John P as well and always shadow ports on midrange.
 

Thread Starter

Aamil

Joined Jan 2, 2015
23
I am not familiar with code configurator plugin..Just started using PIC and XC8...Thanks for your suggestioon :)
 

ke5nnt

Joined Mar 1, 2009
384
Shadowing is to create a variable that mimics the port register. You have PORTA, which is the actual 8-bit port register, then you create an unsigned char data type variable (i.e. "unsigned char PORTAimg") that is an 8-bit variable. You perform the bit operations on the shadow register, then copy the value of the shadow register to the port register. You can do this using bit masking, where if your red traffic light is bit 1 of PORTA, you would:
Code:
#define REDmask 0b00000010
If you bitwise OR this mask with the shadow register variable, it will change the value of bit 1 from 0 to 1.
Mask = 00000010 OR ( | )
Shadow = 00000000
Result = 00000010

Then copying the shadow register to the port register (an 8-bit wide operation) negates r-m-w problems. To turn on red LED, the following demonstrates the above ideals:
Code:
#define REDmask 0b00000010    //mask red led on bit 1
unsigned char PORTAimg;    //an 8-bit variable that images the PORTA register
...
...
PORTAimg |= REDmask;    //perform bitwise OR operator and make PORTAimg equal to the result
PORTA = PORTAimg;     //copy results to PORTA register, an 8-bit operation
...
...
Obviously other code would be required for the full program, but this demonstrates what is needed to shadow port registers. Others may have additional thoughts/alternative methods, but this is how I do it.

Regards
 

tshuck

Joined Oct 18, 2012
3,534
You are not resetting your watchdog timer. Either set WDTE to OFF, or reset the WDT before it expires.

Edit: You also need to set the appropriate values in ADCON1 (see page 128 in the datasheet).
 
Last edited:

Thread Starter

Aamil

Joined Jan 2, 2015
23
i tried by disabling ADC, using ADCON1=7

as in the below code
Code:
int main()
{
  TRISA = 0; //RB0 as Output PIN
  PORTA=0;
  ADCON1=7;
  while(1)
  {
  RA0 = 1;RA1=0;RA2=0;RA4=0;  // LED ON
  __delay_ms(10000); // 1 Second Delay
  RA0 = 0;RA1=1;RA2=0;RA4=0;  // LED ON
  __delay_ms(10000); // 1 Second Delay
  RA0 = 0;RA1=0;RA2=1;RA4=0;  // LED ON
  __delay_ms(10000); // 1 Second Delay
  RA0 = 0;RA1=0;RA2=0;RA4=1;  // LED ON
  __delay_ms(10000); // 1 Second Delay
  RA0 = 0;RA1=0;RA2=0;RA4=0;  // LED ON
  }
  return 0;
}

The LEDs connected to RA0,RA1 and RA2 turns on . RA4 LED is not turning on.
After RA2 LED turns on , immediately RA0 LED glows.
 

Thread Starter

Aamil

Joined Jan 2, 2015
23
You are not resetting your watchdog timer. Either set WDTE to OFF, or reset the WDT before it expires.

Edit: You also need to set the appropriate values in ADCON1 (see page 128 in the datasheet).
Let me learn that.. Thanks for giving hints :)
 

ke5nnt

Joined Mar 1, 2009
384
Looking at the device datasheet, it seems to me that ADCON1 must be set on bits 3 to 0 for digital I/O. In your code, ADCON1 = 7 is not any way I've ever seen something programmed. Try replacing that with:
Code:
ADCON1 = 0b00001111
For the watchdog timer, your code from your original post shows the #pragma config WDTE = ON. Turning off the timer is as simple as changing "ON" to "OFF".

Looks like I'm also noticing your delay macros. __delay_ms(10000) is a 10 second delay, not 1 second, and the compiler should be giving you errors on a value this large, if it lets you compile it at all. 1 second is 1,000mS, a value I know will be allowed by XC8.

Here is your code with notes:
Code:
int main()                           main program function is "void main(void)" you are defining an integer here
{
TRISA = 0; //RB0 as Output PIN         Should really be defined in binary, not decimal
PORTA=0;                                             Should really be defined in binary, not decimal
ADCON1=7;                     see note in above post
while(1)
{
RA0 = 1;RA1=0;RA2=0;RA4=0; // LED ON
__delay_ms(10000); // 1 Second Delay                change delays to 1000ms for 1 second delay
RA0 = 0;RA1=1;RA2=0;RA4=0; // LED ON
__delay_ms(10000); // 1 Second Delay
RA0 = 0;RA1=0;RA2=1;RA4=0; // LED ON
__delay_ms(10000); // 1 Second Delay
RA0 = 0;RA1=0;RA2=0;RA4=1; // LED ON
__delay_ms(10000); // 1 Second Delay
RA0 = 0;RA1=0;RA2=0;RA4=0; // LED ON
}
return 0;                                      This line of code should not be needed
}
 
Last edited:

Thread Starter

Aamil

Joined Jan 2, 2015
23
Looking at the device datasheet, it seems to me that ADCON1 must be set on bits 3 to 0 for digital I/O. In your code, ADCON1 = 7 is not any way I've ever seen something programmed. Try replacing that with:
Code:
ADCON1 = 0b00001111
For the watchdog timer, your code from your original post shows the #pragma config WDTE = ON. Turning off the timer is as simple as changing "ON" to "OFF".

Looks like I'm also noticing your delay macros. __delay_ms(10000) is a 10 second delay, not 1 second, and the compiler should be giving you errors on a value this large, if it lets you compile it at all. 1 second is 1,000mS, a value I know will be allowed by XC8.

Here is your code with notes:
Code:
int main()                           main program function is "void main(void)" you are defining an integer here
{
TRISA = 0; //RB0 as Output PIN         Should really be defined in binary, not decimal
PORTA=0;                                             Should really be defined in binary, not decimal
ADCON1=7;                     see note in above post
while(1)
{
RA0 = 1;RA1=0;RA2=0;RA4=0; // LED ON
__delay_ms(10000); // 1 Second Delay                change delays to 1000ms for 1 second delay
RA0 = 0;RA1=1;RA2=0;RA4=0; // LED ON
__delay_ms(10000); // 1 Second Delay
RA0 = 0;RA1=0;RA2=1;RA4=0; // LED ON
__delay_ms(10000); // 1 Second Delay
RA0 = 0;RA1=0;RA2=0;RA4=1; // LED ON
__delay_ms(10000); // 1 Second Delay
RA0 = 0;RA1=0;RA2=0;RA4=0; // LED ON
}
return 0;                                      This line of code should not be needed
}
Thanks ke5nnt..

I used the following code and only RA1 LED blinks now. I think I am making some serious silly mistake..

Code:
#include <stdio.h>
#include <stdlib.h>

#define _XTAL_FREQ 2000000
#include <xc.h>

// BEGIN CONFIG
#pragma config FOSC = HS // Oscillator Selection bits (HS oscillator)
#pragma config WDTE = OFF // Watchdog Timer Enable bit (WDT disabled)
#pragma config PWRTE = OFF // Power-up Timer Enable bit (PWRT disabled)
#pragma config BOREN = ON // Brown-out Reset Enable bit (BOR enabled)
#pragma config LVP = OFF // Low-Voltage (Single-Supply) In-Circuit Serial Programming Enable bit (RB3 is digital I/O, HV on MCLR must be used for programming)
#pragma config CPD = OFF // Data EEPROM Memory Code Protection bit (Data EEPROM code protection off)
#pragma config WRT = OFF // Flash Program Memory Write Enable bits (Write protection off; all program memory may be written to by EECON control)
#pragma config CP = OFF // Flash Program Memory Code Protection bit (Code protection off)
//END CONFIG

void main(void)
{
  TRISA = 0b00000000; //RA0 as Output PIN
  PORTA=  0b00000000;
  ADCON1 = 0b00001111;
  while(1)
  {
    RA0 = 1;RA1=0;RA2=0;RA3=0;  //RA0 LED ON
    __delay_ms(1000); // 1 Second Delay
    RA0 = 0;RA1=1;RA2=0;RA3=0;  //RA1  LED ON
    __delay_ms(1000); // 1 Second Delay
    RA0 = 0;RA1=0;RA2=1;RA3=0;  //RA2  LED ON
    __delay_ms(1000); // 1 Second Delay
    RA0 = 0;RA1=0;RA2=0;RA3=1;  //RA3  LED ON
    __delay_ms(1000); // 1 Second Delay
    RA0 = 0;RA1=0;RA2=0;RA3=0;  //All LEDs OFF

  }
}
Could you guide me to a simple traffic light LED code? I tried to learn multiple things and getting confused :(
 

John P

Joined Oct 14, 2008
2,025
I'm still not seeing any use of a shadow register for the port. Or have you tried it and found that it didn't make any difference?

If you find that impossible to do, try writing to PortA as a byte rather than one bit at a time. So instead of
Code:
RA0 = 1;RA1=0;RA2=0;RA3=0; //RA0 LED ON
__delay_ms(1000); // 1 Second Delay
RA0 = 0;RA1=1;RA2=0;RA3=0; //RA1 LED ON
etc
you could say
Code:
PORTA = 0b00000001;
__delay_ms(1000); // 1 Second Delay
PORTA = 0b00000010;
etc
 

ErnieM

Joined Apr 24, 2011
8,377
i tried by disabling ADC, using ADCON1=7

as in the below code
Code:
#define _XTAL_FREQ 2000000   // added this
#include <xc.h>              // added this
 
int main()
{
  TRISA = 0; //RB0 as Output PIN
  PORTA=0;
  ADCON1=7;
  while(1)
  {
  RA0 = 1;RA1=0;RA2=0;RA4=0;  // LED ON
  __delay_ms(10000); // 1 Second Delay
  RA0 = 0;RA1=1;RA2=0;RA4=0;  // LED ON
  __delay_ms(10000); // 1 Second Delay
  RA0 = 0;RA1=0;RA2=1;RA4=0;  // LED ON
  __delay_ms(10000); // 1 Second Delay
  RA0 = 0;RA1=0;RA2=0;RA4=1;  // LED ON
  __delay_ms(10000); // 1 Second Delay
  RA0 = 0;RA1=0;RA2=0;RA4=0;  // LED ON
  }
  return 0;
}

The LEDs connected to RA0,RA1 and RA2 turns on . RA4 LED is not turning on.
After RA2 LED turns on , immediately RA0 LED glows.
I just pasted your code into MPLAB (with the additions noted above) . I saw exactly what you describe, after soon after RA2 turns on the watchdog timer expires. The watchdog will reset your PIC so the code starts all over so the observed sequence is RA0,RA1, RA2, RA0,RA1, RA2, and so on.

The fix for this was noted above, either turn the watchdog timer off, or reset it after each of the long delays using the "CLRWDT();" command you will find in the XC8 User's Guide.

Once you fix the watchdog you should see all 4 LEDs light.

Note the last state for all off has no delay so you will never see that happen.

Also note that "ADCON1=7;" is completely correct code. The default radix in C is base 10, so 7 is a binary 00000111. That is a bit different than the 00001111 posted.
 
Top