why for loop is not working on PIC16F877A?

Discussion in 'Embedded Systems and Microcontrollers' started by Shyamal805, Aug 13, 2013.

  1. Shyamal805

    Thread Starter New Member

    Mar 20, 2012
    27
    0
    Hi, i'm using 16F877A microcontroller for rotating stepper motor. I also using C compiler and MPLAB IDE. My code is working good on Proteus Professional. But when i write on PIC, for loop is not working. Only one loop is working. It is noted that for checking output, i used 4 LED. When i press the button the 4 LED is ON sequentially(first loop) but the next loop is not working.

    I'm new that's why can't understand why for loop is not working? This is my code.

    #include<16F877A.h>
    #fuses HS,NOWDT
    #use delay(clock=10000000)

    void main()
    {
    set_tris_a(0xFF);
    set_tris_b(0x00);
    port_b_pullups(TRUE);
    output_b(0x00);

    while(true){
    if(input(PIN_A0)==0){
    int i;
    for(i=0; i<6; i++){
    output_bit(PIN_B1,1); //34 No. PIN
    delay_ms(500);
    output_bit(PIN_B1,0);

    output_bit(PIN_B2,1); //35 No. PIN
    delay_ms(500);
    output_bit(PIN_B2,0);

    output_bit(PIN_B4,1); //37 No. PIN
    delay_ms(500);
    output_bit(PIN_B4,0);

    output_bit(PIN_B5,1); //38 No. PIN
    delay_ms(500);
    output_bit(PIN_B5,0);
    }
    }
    }
    }

    Please give me the solution why "for loop" is not working?
     
  2. tshuck

    Well-Known Member

    Oct 18, 2012
    3,531
    675
    What is connected to pin A0? Does it actually pull the pin low?
     
  3. ErnieM

    AAC Fanatic!

    Apr 24, 2011
    7,386
    1,605
    So when you the button the leds blink just once?
     
  4. Shyamal805

    Thread Starter New Member

    Mar 20, 2012
    27
    0
    Normally, A0 is connected to +5volt with 10k resistor and a push button. When button is pressed then the A0 pin is will be connected to ground.
     
  5. mitko89

    Member

    Sep 20, 2012
    123
    19
    Remove the for loop and try to do the running LEDs with a while and shift operation on the port control register. This will ensure your hardware is fine. I see no errors in the code, it doesn't make sense not to work...
     
  6. Shyamal805

    Thread Starter New Member

    Mar 20, 2012
    27
    0
    Every button pressed, the led blink
     
  7. WBahn

    Moderator

    Mar 31, 2012
    17,716
    4,788
    It would help if you formatted your code to make it more readable:

    Code ( (Unknown Language)):
    1.  
    2. #include<16F877A.h>
    3. #fuses HS,NOWDT
    4. #use delay(clock=10000000)
    5.  
    6. void main()
    7. {
    8.    set_tris_a(0xFF);
    9.    set_tris_b(0x00);
    10.    port_b_pullups(TRUE);
    11.    output_b(0x00);
    12.  
    13.    while(true){
    14.       if(input(PIN_A0)==0){
    15.          int i;
    16.          for(i=0; i<6; i++){
    17.             output_bit(PIN_B1,1);     //34 No. PIN
    18.             delay_ms(500);
    19.             output_bit(PIN_B1,0);
    20.  
    21.             output_bit(PIN_B2,1);     //35 No. PIN
    22.             delay_ms(500);
    23.             output_bit(PIN_B2,0);
    24.  
    25.             output_bit(PIN_B4,1);     //37 No. PIN
    26.             delay_ms(500);
    27.             output_bit(PIN_B4,0);
    28.  
    29.             output_bit(PIN_B5,1);     //38 No. PIN
    30.             delay_ms(500);
    31.             output_bit(PIN_B5,0);
    32.          }
    33.       }  
    34.    }
    35. }
    36.  
    I don't see a problem with the code and I don't see how a switch bouncing problem or some kind of funky switch behavior could cause what you are describing.

    If I understand what you are saying, right now you get the thing running and it sits there doing nothing. You then momentarily press and release the button and what should happen is the four LEDs should come on in a cycle in which they turn on and off sequentially, with each one being on for half a second, and then repeat that cycle six times, and then go back to sitting there doing nothing until you press the button again. If you were to hold the button down, you would expect it to continue cycling until you release the button and then you would expect it to complete the current set of six cycles before going back to sitting there doing nothing.

    But what you are actually seeing is that, with each button press, the LEDs only go through one cycle (each LED turns on once) and not six cycles.

    What happens if you hold the button down (for longer than the time that it should take to complete the loop, which looks like it should be 12 seconds)?

    What happens if you get rid of the if() statement and the for() loop and just put the code that is within the loop directly within your while(true) loop?

    What happens if you open a blank file and type in the code from scratch (not copy/paste)? Sometimes you can get a hidden character in a text file that causes wierd things and simulators and compilers may well deal with it differently. I would expect the code not to compile if that were the case, but you never know.
     
    JohnInTX likes this.
  8. Shyamal805

    Thread Starter New Member

    Mar 20, 2012
    27
    0
    This is my circuit ...
     
  9. JohnInTX

    Moderator

    Jun 26, 2012
    2,341
    1,024
    You haven't set ADCON1 to B'00000110' to configure the analog pins as all digital. That would keep your switch reading as '0' which should keep the LEDs running all the time.

    Is it OK to declare int i; inside the loop? Unusual.

    You should finish setting the config bits. Turn the brownout detector off and the power up reset timer PWRT on for now.

    Which compiler / MPLAB are you using? Any compiler warnings?

    EDIT: you don't seem to have any decoupling caps at the PIC.
    Are you sure that the oscillator keeps running? Measuring OSC2 with a DMM should get you roughly 1/2 Vdd if it is.

    Rather than flipping individual bits on the port, consider maintaining a byte internally with the rotating pattern. Update it then write the whole byte directly to the port to ensure no r-m-w issues.
     
    Last edited: Aug 14, 2013
  10. Shyamal805

    Thread Starter New Member

    Mar 20, 2012
    27
    0
    I'm using C Compiler and MPLAB IDE v8.88.

    Have no warning.
     
  11. Shyamal805

    Thread Starter New Member

    Mar 20, 2012
    27
    0
    How can i set ADCON1 to B'00000110' on my code?
     
  12. THE_RB

    AAC Fanatic!

    Feb 11, 2008
    5,435
    1,305
    In MikroC it is;
    ADCON1 = 0b00000110;

    If your compiler does not allow binary bit radix like that, (ie is a bad compiler) then you really need to spend some time learning how to change binary numbers to hex or decimal.

    With a bad compiler you will be doing that CONSTANTLY.
     
  13. JohnInTX

    Moderator

    Jun 26, 2012
    2,341
    1,024
    Agreed. Actually, you need to tell us which compiler and version, there are many that run with MPLAB 8.x

    For starters:
    Fix ADCON1
    Move the int x; declaration out of the loop. Even if it compiles OK, its not where anyone would think to look for it.
    Use direct writing to the port. For now:
    Code ( (Unknown Language)):
    1. PORTB = 0x01;
    2. delay
    3. PORTB = 0x02;  // or whatever construct writes a complete byte to the port
    4. delay
    5. PORTB = 0x08;
    6. delay
    7. PORTB = 0x10;
    8. delay
    9. etc..
    in your loop. This will eliminate the possibility of a r-m-w problem. As it is written, you have successive r-m-w bit operations on a port as you change LEDs. That is a recipe for problems, even with simple IO such as this. I cannot begin to tell you how many time I've seen this cause problems, even when the code otherwise conforms to the recommendations in the data sheets.
     
  14. WBahn

    Moderator

    Mar 31, 2012
    17,716
    4,788
    Can you envision any mechanism whereby a r-m-w issue could cause what the OP appears to be seeing, namely that the loop executes exactly once instead of the six times it is supposed to?

    Could such an issue somehow affect the value of the index variable?

    Or perhaps interact with the loop test that is executed immediately after the final r-m-w operation? That seems more likely.

    It might be interesting for the OP to insert a delay at the end of the loop and see if that fixes it. If not, then insert delays after each r-m-w instruction that doesn't already do so.

    I'm not suggesting this as a fix -- your recommendation is much better; I'm just trying to think of ways to confirm that this is, infact, the issue.
     
    JohnInTX likes this.
  15. JohnInTX

    Moderator

    Jun 26, 2012
    2,341
    1,024
    Good thoughts. My reasoning is:
    On the r-m-w, if the OP is using the LEDs as the indicator of whether his code is operating (as opposed to a debugger), it follows that he has to know that there are no IO problems. The only way to beat it is not to use it. I develop PIC-based stuff for a living and would NEVER use r-m-w on a port. Ever. That said, I can't see where it would affect the running of the loop itself.

    The index variable bothers me because its out of place. Presumably, the compiler is OK with it (no errors/warnings per the OP) but is that true or did the compiler miss it? I don't know because 1) don't know which compiler and 2)its a weird place for it. If I were debugging it, I'd move it on the chance that the code is re-initializing it or some other issue.

    I guess the best way to put it is that we know the code should run OK but doesn't. I know that the IO as written can be problematic. I would fix that. I also know that the int declaration is in a funny place. I would move it. Why not eliminate the easy ones, particularly when debugging from a distance?

    Other easy ones are the lack of decoupling caps, making sure the power supply can handle the additional LED loads, finishing up the CONFIG bits and, that I didn't note it at the time, finishing up the IO init rather than have a bunch of floating inputs.

    Your good suggestion that the OP eliminate the loop would come after that.

    Gotta run.
    Thanx WBahn.
    Good luck!
     
  16. WBahn

    Moderator

    Mar 31, 2012
    17,716
    4,788
    Yeah, I just hate "fixing" a problem without getting a decent idea of exactly what the problem was, especially when there are multiple "fixes" that should be applied to clean things up. My concern is that the real problem was something else and the fixes just happen to coincidentally make the problem go away. But, sometimes you just can't, or don't have the time/resources, to positively understand what is going on.

    As for the int declaration, C (at least C99, not sure about C89) allows variables to be declared within a compound statement (i.e., delimited by curly braces). I prefer to declare them all at the top of the function, but lot's of people (and it seems to be becoming more common) like to declare them where they decide they need it. Personally, I think it's partly laziness, but there are some good arguments that can be made for doing it (and good arguments that can be made against). Perhaps one good argument for, particularly in resource-starved architectures, is that the compiler doesn't have to keep the variable storage allocated for the entire duration of the function, just the duration of the compound statement. I don't know if many (or any) compilers leverage this. I suspect most just collect them all together and put then on the stack just as though they had been declared "properly". But it isn't hard to imagine a compiler making the effort to determine the maximum amount of stack variable space needed and then to multiplex values into that space as they come into or go out of scope.
     
    JohnInTX likes this.
  17. JohnInTX

    Moderator

    Jun 26, 2012
    2,341
    1,024
    typing on phone so will be brief. Interesting information about the different compilers. I did not know that. makes sense though. I do like eliminating the suspicious things and the easy things especially when troubleshooting from a distance thanks WBahn
     
Loading...