why for loop is not working on PIC16F877A?

Thread Starter

Shyamal805

Joined Mar 20, 2012
27
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?
 

mitko89

Joined Sep 20, 2012
127
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...
 

WBahn

Joined Mar 31, 2012
30,058
It would help if you formatted your code to make it more readable:

Rich (BB 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);
         }
      }   
   }
}
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

Joined Jun 26, 2012
4,787
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:

THE_RB

Joined Feb 11, 2008
5,438
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.
 

JohnInTX

Joined Jun 26, 2012
4,787
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.
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:
Rich (BB code):
PORTB = 0x01;
delay
PORTB = 0x02;  // or whatever construct writes a complete byte to the port
delay
PORTB = 0x08;
delay
PORTB = 0x10;
delay
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.
 

WBahn

Joined Mar 31, 2012
30,058
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.
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

Joined Jun 26, 2012
4,787
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.
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!
 

WBahn

Joined Mar 31, 2012
30,058
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

Joined Jun 26, 2012
4,787
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
 
Top