Question about incrementing and holding a value

Thread Starter

hunterage2000

Joined May 2, 2010
487
I've written the below code and I'm wondering why the variable led_state is not holding a shifted value when a button is pressed. Initially bit0 of 4 leds is on then when the button is pressed the led is shifted left so bit1 is on then bit2 then bit3 etc but after a while the led state go's back to bit0. I was expecting when button is 0 it would keep on returning the shifted value. Can anyone tell me why its not? I was thinking that it might have something to do with the scope of led_state but it is declared as a global variable.

Code:
#pragma config FOSC = INTOSCIO
#include <stdio.h>
#include <pic16f785.h>
#include <xc.h>
#include <stdlib.h>

struct led_addr_bits {
unsigned bit7 : 1;
unsigned bit6 : 1;
unsigned bit5 : 1;
unsigned bit4 : 1;
unsigned bit3 : 1;
unsigned bit2 : 1;
unsigned bit1 : 1;
unsigned bit0 : 1;
}led_addr_bits;

char inc_led_state(char);
char led_state = 0x01;
char inc_state;
int main()
{
OSCCON = 0x39;

ANSEL1bits.ANS11 = 0;
TRISBbits.TRISB5 = 1;
TRISCbits.TRISC6 = 0;
TRISCbits.TRISC7 = 0;
TRISBbits.TRISB7 = 0;

while(1)
{
led_state = inc_led_state(led_state);


led_addr_bits.bit3 = led_state >> 3;
led_addr_bits.bit2 = led_state >> 2;
led_addr_bits.bit1 = led_state >> 1;
led_addr_bits.bit0 = led_state;

PORTBbits.RB7 = led_addr_bits.bit0;
PORTCbits.RC7 = led_addr_bits.bit1;
PORTCbits.RC6 = led_addr_bits.bit2;
PORTCbits.RC3 = led_addr_bits.bit3;

}
}
     

char inc_led_state(char led_state)
{
    if(PORTBbits.RB5==0)
    {
    inc_state = led_state;
    }
    
    if(PORTBbits.RB5==1)
    {
    while(PORTBbits.RB5==1);
   
    inc_state = led_state << 1;
    }
    return inc_state;
}
 

MrChips

Joined Oct 2, 2009
30,808
As far as I can see the way your code is written, when the one is shifted to the left led_state will eventually become 00000000 again.
How does it ever get back to 00000001?
 

Thread Starter

hunterage2000

Joined May 2, 2010
487
This was just to see if the value would hold. If it did I would of made it not go beyond bit3 and have a increment right button to go back to and not go beyond bit0.
 

Thread Starter

hunterage2000

Joined May 2, 2010
487
If I press once led bit0 go's off and led bit1 go's on. I am thinking that once the button is not 1, the value that was incremented inside the if block is lost so returns back to the original value of 1. I'm assuming this has something to do with block scope but not sure how to get round this.
 

Thread Starter

hunterage2000

Joined May 2, 2010
487
Yeah I've looked at a few books and I think the problem is led_state loses its incremented value when button = 0. I was thinking maybe send the led_state within the if statement and retrieve it in the while loop.
 

Kermit2

Joined Feb 5, 2010
4,162
Sounds backwards to me. Wouldn't you want to sense the LED state in the IF statement?
THEN code the response you expect for the sensed state?
Consider addind a prior IF check for the exception state. I think it is the 0000000... state here, to force the desired 00000001 state before your IF loop executes.

Disclaimer. I don't know the code being used, but I understand programming.
 

Thread Starter

hunterage2000

Joined May 2, 2010
487
I tried this but got the same. Can someone explain why the value doesn't hold? It must be something to do with scope.

Code:
#pragma config FOSC = INTOSCIO
#include <stdio.h>
#include <pic16f785.h>
#include <xc.h>
#include <stdlib.h>

struct led_addr_bits {
unsigned bit7 : 1;
unsigned bit6 : 1;
unsigned bit5 : 1;
unsigned bit4 : 1;
unsigned bit3 : 1;
unsigned bit2 : 1;
unsigned bit1 : 1;
unsigned bit0 : 1;
}led_addr_bits;

char inc_led_state(char led_state);
char led_state = 0x01;
char inc_state;
int main()
{
OSCCON = 0x39;

ANSEL1bits.ANS11 = 0;
TRISBbits.TRISB5 = 1;
TRISCbits.TRISC6 = 0;
TRISCbits.TRISC7 = 0;
TRISBbits.TRISB7 = 0;

while(1)
{

    if(PORTBbits.RB5==0)
    {
    inc_led_state(led_state);
    }

    if(PORTBbits.RB5==1)
    {
    while(PORTBbits.RB5==1);
    led_state = led_state << 1;
    inc_led_state(led_state);
    }

led_addr_bits.bit3 = led_state >> 3;
led_addr_bits.bit2 = led_state >> 2;
led_addr_bits.bit1 = led_state >> 1;
led_addr_bits.bit0 = led_state;

PORTBbits.RB7 = led_addr_bits.bit0;
PORTCbits.RC7 = led_addr_bits.bit1;
PORTCbits.RC6 = led_addr_bits.bit2;
PORTCbits.RC3 = led_addr_bits.bit3;

}
}
     

char inc_led_state(char led_state)
{
    return led_state;
   
}
 

dannyf

Joined Sep 13, 2015
2,197
I've looked at a few books
i'm sure you did but none made it to your code.

Here is a short list of what I think is wrong:

1) you didn't read the compiler manual: or you wouldn't have included the header files the way you did;
2) you didn't know the typical workflow for a PIC or your would have set up the configuration fuses correctly;
3) you didn't read the data sheet or you would have initiated the device correctly;
4) you didn't understand C or you would have assign values to the bits differently;
5) you didn't thought through the logic of your code;
6) you didn't recognize the RMW problem you are having here.
.....

Any of those issues would have gotten you in trouble and you have all of them in one short piece.

I think getting back to the basics is the fastest way you could move forward.
 

Thread Starter

hunterage2000

Joined May 2, 2010
487
I've been through the basics. The header files excluding pic16f785.h and xc.h are initially included. The config is set for an internal osc. The 4 output and 1 input was tested and works. Char values are used temporarily until I look into why I get errors when using the bit data type.

The logic I thought was:

initially led_state is 1 and LED bit0 is 'ON'
IF button isn't pressed, led_state = led_state
IF button is pressed, led_state is shifted left and becomes 2 and LED bit1 is 'ON'
WHEN button is released button isnt pressed, led_state = led_state = 2

then I expected this to hold at 2 until button is pressed and shifted left again.

The value 2 holds a while then returns to 1.


I think the scope of inc_state is the problem as it eventually reverts back to a value of 1 but not sure why as this is returned from within the if block.
 
Top