going from ASM to C and stuck with if statement

Discussion in 'Embedded Systems and Microcontrollers' started by hunterage2000, May 17, 2014.

  1. hunterage2000

    Thread Starter Active Member

    May 2, 2010
    400
    0
    I am starting to learn C and I am stuck on multiple expressions in an if and if-else statement. I want a red led to be on and green led off when neither of 2 buttons are pressed but if the failsafe button is pressed then red is off and green is on. I have coded what I thought would work but the red light remains when I press it.

    Can anyone see what I am missing?

    Code ( (Unknown Language)):
    1.  
    2.  
    3. #include <pic16f690.h>
    4. #include <htc.h>
    5.  
    6. //Port initialize
    7.  
    8. #define Button_Input_Init TRISBbits.TRISB7
    9. #define Fan_Red_Light_Output_Init TRISBbits.TRISB6
    10. #define Fan_Green_Light_Output_Init TRISBbits.TRISB5
    11. #define Failsafe_Input_Init TRISCbits.TRISC2
    12.  
    13. // Port bit names
    14.  
    15. #define Button_State PORTBbits.RB7
    16. #define Red_Light_State PORTBbits.RB6
    17. #define Green_Light_State PORTBbits.RB5
    18. #define Failsafe_State PORTCbits.RC2
    19.  
    20. void main(void)
    21. {
    22.     // Initiate's I/O port bits
    23.  
    24.     Button_Input_Init = 1;
    25.     Fan_Red_Light_Output_Init = 0;
    26.     Fan_Green_Light_Output_Init = 0;
    27.     Failsafe_Input_Init = 1;
    28.  
    29.    if(Button_State == 0 && Failsafe_State == 0)
    30.     {
    31.        
    32.         Red_Light_State = 1;
    33.         Green_Light_State = 0;
    34.  
    35.     }
    36.    else if(Button_State == 0 && Failsafe_State == 1)
    37.    {
    38.        Red_Light_State = 0;
    39.         Green_Light_State = 1;
    40.    }
    41.    
    42.     while(1);
    43.  
    44. }
    45.  
    I am using MPLAB, PICKIT 2 and pic16f690
     
    Last edited: May 17, 2014
  2. MrChips

    Moderator

    Oct 2, 2009
    12,440
    3,361
    Since you may not be certain on the order of operations, always enclose your statements with brackets to make certain you know which operations are performed first, e.g.

    Code ( (Unknown Language)):
    1.  
    2. if ( (Button_State == 0)  &&  (Failsafe_State == 0) )
    3.  

    You can also use DeMorgan's Equivalents to simplify the expression, e.g.
    Code ( (Unknown Language)):
    1.  
    2. if ( Button_State ||  Failsafe_State )
    3.  
    Note the resulting code flow has to be reversed.
     
  3. hunterage2000

    Thread Starter Active Member

    May 2, 2010
    400
    0
    Thanks for the tip MrChips, Its still not working. Any ideas on why? I changed the code so if both buttons were 1 then the light would change to green. Does this mean that both buttons have to change to 1 at the exact same time? If not i'm not sure whats up.
     
  4. shteii01

    AAC Fanatic!

    Feb 19, 2010
    3,388
    497
    Ok. Here is what it looks like to me.

    Your code is executed sequentially. So you execute first if statement. Then you execute the else if statement. Then you enter while super loop and never exit it.

    Of the top of my head it seems to me that your if and else if statements executed so fast that there is no time for user to change anything. So basically you are providing the definitions of what you want to detect. But you are not providing a detection method.

    Normally the simplest way to do detection is to use polling. For the sake of simplicity at this point you can put your if statements inside the while super loop. This way the program will constantly check the states of the buttons.

    Also. Your conditions for which light is On and which light is Off are separate, not related to each other. You should be using two if statements instead of if and else if. The two if statements should not be nested. Think of them as cases.
    Case 1: if this and that happens, do X.
    Case 2: if this other and that other happens, do Y.
    See? The two cases are not related to each other, so you don't need a relationship of if-else.
     
  5. hunterage2000

    Thread Starter Active Member

    May 2, 2010
    400
    0
    I read that while(1) at the bottom of the code would be an infinite loop. I think I may of interpreted this wrong. So what you are saying is put the while(1) loop at the top before the if statements start?
     
  6. shteii01

    AAC Fanatic!

    Feb 19, 2010
    3,388
    497
    while(1) is infinite loop, but it is also an empty loop.

    What you want is this:
    Code ( (Unknown Language)):
    1.  
    2. while(1)
    3. {
    4.   if(condition1 && condition2)
    5.   {
    6.      Do X;
    7.   }
    8.  
    9.   if(condition3 && condition4)
    10.   {
    11.      Do Y;
    12.   }
    13. }
    14.  
    See? You have your conditions and commands inside the infinite loop. The program will constantly check the validity of the conditions (polling) and if one set of conditions is valid, the command attached to that condition will be executed.
     
    Last edited: May 17, 2014
  7. jjw

    Member

    Dec 24, 2013
    173
    31
    while(1); <- see the semicolon!
    Is the same as loop: goto loop
    Your program ends in an infinite loop.

    If you wan't to execute all statements in an infinite loop, put while(1) without semicolon at the top and the statements within curly brackets.

    Edit: I was slow, shteii01 explained the same.
     
  8. Art

    Distinguished Member

    Sep 10, 2007
    785
    61
    Code ( (Unknown Language)):
    1.  
    2. void main(void) {
    3.  
    4. BOOL exit = 0;
    5.  
    6. while (exit = 0) {
    7. if (Button_State == 0) {
    8.     if (Failsafe_State == 0) {
    9.     Red_Light_State = 1;
    10.     Green_Light_State = 0;
    11.     } else { // Failsafe_State
    12.     Red_Light_State = 0;
    13.     Green_Light_State = 1;
    14.     } // Failsafe_State
    15. } // Button_State
    16. } // while
    17.  
    18. } // main
    19.  
     
  9. THE_RB

    AAC Fanatic!

    Feb 11, 2008
    5,435
    1,305
    I think the main thing you are missing is telling us if the button/LED states are temporary or latching! We need to know that. :)
     
Loading...