 |
 |
| View previous topic :: View next topic |
| Author |
Message |
BillySmith
Joined: 09 Jan 2015
Posts: 3
|
| Programming PIC with Flowchart chart |
 Posted: Fri Jan 09, 2015 9:46 am |
 |
|
Hi there,
I have stepped into a different world of programming from my mechanical roots. I am working on a side project. It requires some programming so i decided to learn and give it a go. I have attached a small portion of the flowchart that i am trying to code. The image and the code is below. I don't have the chip right now to test it so i need help from you guys to see if the code even remotely makes any sense. I apologize in advance for poor coding. Thanks
| Code: |
#include <18F2620.h>
#fuses XT, NOWDT, NOPROTECT, PUT, BROWNOUT, NOLVP,
#use delay(clock=4000000)
output_low(PIN_B0); //set RB0 to LOW
output_low(PIN_B2); //set RB2 to LOW
output_low(PIN_B3); //set RB3 to LOW
output_high(PIN_B1); //set RB1 to HIGH
output_high(PIN_B4); //set RB3 to HIGH
input(PIN_A2); //set RA2 as INPUT
input(PIN_A3); //set RA3 as INPUT
if (PIN_A2 == 1) //Check, is RA2 high?
goto A; //yes go to A
else
output_low(PIN_RB0);
A: (
output_high(PIN_B0); //set output RB0 to HIGH
if (PIN_A3 == 0)
output_high(PIN_B3);
output_high(PIN_B4);
goto Pulse1;
else if (PIN_A3 == 0)
output_high(PIN_B3);
output_high(PIN_B4);
else
output_high(PIN_B2);
X: (
output_high(PIN_B4);
output_low(PIN_B0);
output_high(PIN_B2);
);
if (PIN_A0 == 0)
goto X;
else if (PIN_A3 == 0)
goto Pulse1;
else
goto X;
);
|
|
|
 |
newguy
Joined: 24 Jun 2004
Posts: 1909
|
|
| Posted: Fri Jan 09, 2015 9:56 am |
|
|
I didn't really look through your code but one thing jumps out immediately. C will give you just enough rope to hang yourself so you have to be careful.
What I'm talking about is the code that executes in if - else statements. If you have more than one line of code to execute in an if - else, they have to be wrapped in braces {}. If you only have one line that you'd like to execute in an if - else, you can omit the braces.
So your if-else block becomes:
| Code: | if (PIN_A3 == 0) {
output_high(PIN_B3);
output_high(PIN_B4);
goto Pulse1;
}
else if (PIN_A3 == 0) {
output_high(PIN_B3);
output_high(PIN_B4);
}
else
output_high(PIN_B2); // this is okay because it's only a single line and doesn't have to be wrapped in braces BUT it's bad practice to mix the two styles |
It's also generally verboten to use goto's in C. There's always a way around them - for instance your A, X and Pulse1 could become functions instead of goto's. |
|
|
BillySmith
Joined: 09 Jan 2015
Posts: 3
|
|
| Posted: Fri Jan 09, 2015 10:10 am |
|
|
| Thank you for your input. I will dig a little more into loops and avoid the GOTO command. |
|
|
gpsmikey
Joined: 16 Nov 2010
Posts: 588
Location: Kirkland, WA
|
|
| Posted: Fri Jan 09, 2015 11:00 am |
|
|
Another common mistake that I don't see in your code, but it will get you if you are just getting into this is the difference between "=" and "==" the first case assigns the value to something, the second compares two variables. Where you get nailed with that is with a statement like "if ( x=1)" which makes perfect sense to you when you read it AND it compiles without error. Unfortunately, it does NOT do what you meant to do - it assigns 1 to x which is always true so you end up with it always being a true statement AND your variable "x" gets set to 1. That one can take a while to find too. One way around that is to always write your tests reversed from what is normal - instead of writing your tests as "if (x == 1)" write it as "if (1 == x)" - now, when you forget the second "=" sign in the test, the compiler will complain about you trying to assign x to the value 1. Having stepped in this one a number of times, it is worth mentioning :-)
mikey
_________________
mikey
-- you can't have too many gadgets or too much disk space !
old engineering saying: 1+1 = 3 for sufficiently large values of 1 or small values of 3 |
|
|
ckielstra
Joined: 18 Mar 2004
Posts: 3680
Location: The Netherlands
|
|
| Posted: Fri Jan 09, 2015 11:15 am |
|
|
1) One huge problem is that you are lacking a 'main' function. In C, program code can only be executed when it is located inside a function. It is an error of the CCS compiler that it doesn't generate an error code for this.
The first function the processor will jump to on startup is called 'main' and should always be present.
Normal program flow is that it starts with some initialization and then continues to do its thing forever, a.k.a. doing a loop.
Something like: | Code: | #include <18F2620.h>
#fuses XT, NOWDT, NOPROTECT, PUT, BROWNOUT, NOLVP,
#use delay(clock=4MHz)
void main()
{
// Your initialization code here
// Repeat forever
while(1)
{
// Normal program flow
}
} |
The above fragment is something you can save as a template, it is how all your future programs will start.
2) Then, a few branches in your program flow are a dead end. If that's what you intend to happen, then it prevents confusion by adding an extra state named 'end' or 'stop'.
3) Another issue is not an error but has to do with making your program easier to read. An easy to read program results in fewer bugs.
| Code: | | output_high(PIN_B3); |
Will be easier to read when you replace the pin code by a name making more sense. For example: | Code: | #define ALARM_LED PIN_B3
output_high(ALARM_LED); |
4) A last bug: | Code: | if (PIN_A2 == 1) //Check, is RA2 high?
goto A; //yes go to A
else
output_low(PIN_RB0); |
Funny how you use the correct function to write an output pin, but forget to use a function for reading the pin. The correct version: | Code: | if (input(PIN_A2) == 1) //Check, is RA2 high?
... rest unchanged |
A suggestion for testing your code is to install the Microchip MPLAB program. This free program includes a simulator that you can use to step through your code, one program line at a time and inspect the results.
Don't install the latest MPLAB versions called 'MPLAB X'. These used to be incompatible with CCS. It might be these problems have been fixed by now but the old v8.xx versions are working for sure and is what most people on this forum are using.
I suggest using v8.92 or newer, it can be found here: www.microchip.com/archived |
|
|
BillySmith
Joined: 09 Jan 2015
Posts: 3
|
|
| Posted: Fri Jan 09, 2015 12:48 pm |
|
|
| Thanks you all. I really appreciate your advice. |
|
|
|
|
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum
|
Powered by phpBB © 2001, 2005 phpBB Group
|
FAQ 
Search
Register
Profile 
Log in to check your private messages 
Log in
