0
\$\begingroup\$

In this simple circuit, there is a button connected to the A0 pin.

PIC toggle LED's

I'd like to toggle half of the LED's when button is pressed.

#include <pic.h>

#ifndef _XTAL_FREQ
// Clock frequency is 16 Mhz
#define _XTAL_FREQ 16000000
#endif

__CONFIG (FOSC_HS & WDTE_OFF & LVP_OFF & CPD_OFF & CP_OFF);

int main()
{
    static char flashingLeds = 0xFF;

    ADCON1 = 0x06;        // RA0 digital input
    TRISAbits.TRISA0 = 1; // RA0 input

    TRISB = 0;    // PORTB is output
    PORTB = 0;    // Make all pins on PORTB low

    while(1)
    {
        // If button is pressed reverse LED's
        if (PORTAbits.RA0 == 1)
        {
            __delay_ms(250);
            flashingLeds = ~flashingLeds;
        }
        PORTB = flashingLeds;
    }
}

When button is pressed the program toggles all of the LED's on and off.

On the other hand I want to toggle only half of the lights. When I change flashingLeds variable to 0x0F it doesn't work as it should.

static char flashingLeds = 0x0F;

When I change the variable to 0x0F and when the button is pressed, I expect half of the LED's should be on. In binary notation as follows :

b 0000 1111

After some time I pressed the button again LED's should switch to this state.

b 1111 0000

Any ideas? Thanks.

Update: After making the recommend changes the code became like this. Just for testing purposes, I gave different values to flashingLEDS variable. Like 0xF0, 0xAA, 0x0F etc.

#include <pic.h>

#ifndef _XTAL_FREQ
// Clock frequency is 16 Mhz
#define _XTAL_FREQ 16000000
#endif

__CONFIG (FOSC_HS & WDTE_OFF & LVP_OFF & CPD_OFF & CP_OFF);

int main()
{
    static char flashingLeds = 0xAA;

    ADCON1 = 0x06;        // RA0 digital input
    TRISAbits.TRISA0 = 1; // RA0 input

    TRISB = 0;    // PORTB is output
    PORTB = 0;    // Make all pins on PORTB low

    while(1)
    {
        // If button is pressed reverse LED's
        if (PORTAbits.RA0 == 1)
        {
            __delay_ms(250);
            flashingLeds = ~flashingLeds;
        }
        PORTB ^= flashingLeds;
    }
}
\$\endgroup\$
11
  • 4
    \$\begingroup\$ Input is PORT output is LAT. \$\endgroup\$
    – vini_i
    Oct 29, 2020 at 18:17
  • \$\begingroup\$ what does this mean? ... it doesn't work as it should \$\endgroup\$
    – jsotola
    Oct 29, 2020 at 18:21
  • \$\begingroup\$ I've added a little more explanation to the question. I think the micro controller which I use (which is PIC16F877A) doesn't have port named LAT. \$\endgroup\$
    – user211748
    Oct 29, 2020 at 18:33
  • 1
    \$\begingroup\$ I would really try and get the MPLAB X IDE installed. You will learn a lot about what goes on inside the chip! No more guessing! \$\endgroup\$
    – Aaron
    Oct 29, 2020 at 19:55
  • 1
    \$\begingroup\$ Move the the delay_ms(250); to after toggling the LEDs, so they will change as soon as you hit the button and the subsequent delay will debounce the switch. Then remove the 100nF capacitor from the switch circuit (where it could glitch the power supply) and put it between Vcc and gnd, where it will help to prevent glitches. \$\endgroup\$ Oct 30, 2020 at 4:03

2 Answers 2

1
\$\begingroup\$

You should do this: PORTB ^= flashingleds; (instead of "=") With flashingleds being 0x0f. You only have to set ones, where You want to toggle, no need for changing this variable every time.

Whay happens is, that when You do ~0x0f You get 0xf0, so toggling half to on and half to off. When You do what I suggest - You will be xoring half of the port, so this is what was needed.

Another approach could be to set half of the port: PORTB |= flashingleds and in the next cycle: PORTB &= ~flashingleds.

Look that I'm never directly writing to port by "=" - always or, xor, and. This is used to not disturb the other half of port.

Reading of port state is ok, checked in RM.

\$\endgroup\$
17
  • \$\begingroup\$ First of all thanks. When flashingLeds = 0x0F; your solution works fine. But when I change flashingLeds to 0xAA or 0xF0 it sometimes work and sometimes doesn't. What could be the reason for this? \$\endgroup\$
    – user211748
    Oct 29, 2020 at 19:06
  • 2
    \$\begingroup\$ Is your switch actually wired like that or is it a mistake on the circuit diagram? \$\endgroup\$
    – user173271
    Oct 29, 2020 at 19:13
  • 1
    \$\begingroup\$ Problem is in Your switch connection. Resistor should be right on the pin to pull it up. Check this. Alternatively You can turn on internal pullup on switch pin (but in this case not possible - IT seems that PORTA doesnt have pullup) \$\endgroup\$
    – fifi_22
    Oct 29, 2020 at 19:25
  • 1
    \$\begingroup\$ What actually happens is, that It doesnt detect some switch edges and sometimes detects more than one. adding 100nF cap from switch pin to ground will also get rid of bumping contacts. \$\endgroup\$
    – fifi_22
    Oct 29, 2020 at 19:30
  • 1
    \$\begingroup\$ @fifi_22 Note that something like PORTB ^= flashingleds is a read-modify-write instruction which can lead to bugs on the older 16F PIC's like the OP is using. (They added the LAT registers in newer PIC's to solve this). Shadowing the values for the output ports like the OP is trying to do is actually the preferred method on these older devices. \$\endgroup\$ Oct 30, 2020 at 15:17
0
\$\begingroup\$

I solved the problem with the help of @Bruce Abbott.

I put the 100nF capacitor between Vcc and ground. I moved the delay_ms(250); to after toggling the LEDs.

The solution was xoring flashingLeds to hexadecimal 0xFF value.

The correct working code is as follows.

#include <pic.h>

#ifndef _XTAL_FREQ
// Clock frequency is 16 Mhz
#define _XTAL_FREQ 16000000
#endif

__CONFIG (FOSC_HS & WDTE_OFF & LVP_OFF & CPD_OFF & CP_OFF);

int main()
{
    static char flashingLeds = 0xAA;

    ADCON1 = 0x06;        // RA0 digital input
    TRISAbits.TRISA0 = 1; // RA0 input

    TRISB = 0;    // PORTB is output
    PORTB = 0;    // Make all pins on PORTB low

    while(1)
    {
        // If button is pressed reverse LED's
        if (PORTAbits.RA0 == 1)
        {
            /*  change recommended by @bruce-abbott */
           flashingLeds = ~flashingLeds;
            __delay_ms(250);
        }

        PORTB = flashingLeds ^ 0xFF;
    }
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.