| View previous topic :: View next topic |
| Author |
Message |
pilar
Joined: 30 Jan 2008
Posts: 197
|
| Problem with IF statement |
 Posted: Thu Oct 15, 2009 2:29 pm |
 |
|
Hi, I'm trying to make a comparison of a string received through the UART from a PC, the problem is as follows:
I receive the characters correctly but when I try to make the comparison using the following routine all conditions are executed even though only one of three conditions is true, someone could say me what is my mistake
| Code: | void Test_SMS_RX(){
if ((cbuff[10]=='R')&& (cbuff[11]=='E') && (cbuff[12]=='L') && (cbuff[13]=='A')&& (cbuff[14]=='Y')&& (cbuff[15]=='-') && (cbuff[16]=='O') && (cbuff[17]=='N')){
delay_ms(100);
ON(RELAY);
break;
}
if ((cbuff[10]=='R')&& (cbuff[11]=='R') && (cbuff[12]=='L') && (cbuff[13]=='A')&& (cbuff[14]=='Y')&& (cbuff[15]=='-') && (cbuff[16]=='O') && (cbuff[17]=='F')&& (cbuff[18]=='F')){
delay_ms(100);
OFF(RELAY);
break;
}
if ((cbuff[10]=='T')&& (cbuff[11]=='I') && (cbuff[12]=='M') && (cbuff[13]=='E')&& (cbuff[14]=='R')&& (cbuff[15]=='-') && (cbuff[16]=='O') && (cbuff[17]=='N')){
delay_ms(100);
ON (TIMER);
break;
}
} |
|
|
 |
PCM programmer
Joined: 06 Sep 2003
Posts: 21708
|
|
| Posted: Thu Oct 15, 2009 2:45 pm |
|
|
I made a test program, and it doesn't execute all conditions.
When the array has "RELAY-ON" in it, only the 1st if() statement is
executed. Here is the result.
It's possible that you mean all the tests are done in all if() statements
even though it can stop doing more tests once it has found a match.
That's because you are not using an "if-else-else if" structure. You
need to get a book on C and study that section. Or look at these links:
http://www.intap.net/~drw/cpp/cpp04_02.htm
http://gd.tuwien.ac.at/languages/c/programming-bbrown/c_025.htm
Here is the test program:
| Code: | #include <16F877.H>
#fuses XT, NOWDT, NOPROTECT, BROWNOUT, PUT, NOLVP
#use delay(clock=4000000)
#use rs232(baud=9600, xmit=PIN_C6, rcv=PIN_C7,ERRORS)
int8 cbuff[20] = {" RELAY-ON"};
void Test_SMS_RX()
{
if ((cbuff[10]=='R')&& (cbuff[11]=='E') && (cbuff[12]=='L') && (cbuff[13]=='A')&& (cbuff[14]=='Y')&& (cbuff[15]=='-') && (cbuff[16]=='O') && (cbuff[17]=='N'))
{
// delay_ms(100);
// ON(RELAY);
printf("RELAY ON\n\r");
break;
}
if ((cbuff[10]=='R')&& (cbuff[11]=='R') && (cbuff[12]=='L') && (cbuff[13]=='A')&& (cbuff[14]=='Y')&& (cbuff[15]=='-') && (cbuff[16]=='O') && (cbuff[17]=='F')&& (cbuff[18]=='F'))
{
// delay_ms(100);
// OFF(RELAY);
printf("RELAY OFF\n\r");
break;
}
if ((cbuff[10]=='T')&& (cbuff[11]=='I') && (cbuff[12]=='M') && (cbuff[13]=='E')&& (cbuff[14]=='R')&& (cbuff[15]=='-') && (cbuff[16]=='O') && (cbuff[17]=='N'))
{
// delay_ms(100);
// ON (TIMER);
printf("TIMER ON\n\r");
break;
}
}
//============================
void main()
{
Test_SMS_RX();
while(1);
} |
|
|
|
pilar
Joined: 30 Jan 2008
Posts: 197
|
|
| Posted: Thu Oct 15, 2009 3:07 pm |
|
|
Thank you, I found my mistake  |
|
|
asmboy
Joined: 20 Nov 2007
Posts: 2128
Location: albany ny
|
|
| Posted: Thu Oct 15, 2009 3:46 pm |
|
|
do you have any control over the HOST transmitted RELAY command
syntax ?
if so - why do you need so much verbosity?
when a command struct like 'R0' or 'R1' could haul your ashes so effciently?
you are making this so much harder than you need to - if you can change the host command routines
then you need only decide if the 1st char is 'R'
and then detect an ascii 1 or 0 for instance
to have a compact command that is EZ to parse. |
|
|
bkamen
Joined: 07 Jan 2004
Posts: 1611
Location: Central Illinois, USA
|
|
| Posted: Fri Oct 16, 2009 12:25 am |
|
|
| asmboy wrote: | | if so - why do you need so much verbosity? |
Amen.
Or do a string comparison...
but yes, if something is hard to read, then it's hard to understand.
At that point it's easy to mess up.
-Ben
_________________
Dazed and confused? I don't think so. Just "plain lost" will do. :D |
|
|
Wayne_
Joined: 10 Oct 2007
Posts: 681
|
|
| Posted: Fri Oct 16, 2009 2:03 am |
|
|
The break instruction in your code is irrelevant!
break will break out of a loop or select/case statement. if you want to exit the function use return.
And as stated look at string functions. inpaticular strncmp
one note though, with CCS you can't use consts in some string functions so you need to do something lilke
| Code: |
char match[10];
strcpy(match, "RELAY-ON");
if (strncmp(&cbuff[10], match, 8) == 0)
{
}
strcpy(match, "RELAY-OFF");
if (strncmp(&cbuff[10], match, 9) == 0)
{
}
strcpy(match, "TIMER-ON");
if (strncmp(&cbuff[10], match, 8) == 0)
{
}
|
Now isn't that more readable. |
|
|
John P
Joined: 17 Sep 2003
Posts: 331
|
|
| Posted: Fri Oct 16, 2009 6:59 am |
|
|
Do you need that "match" business at all?
How about
| Code: |
if (!strncmp(cbuff+10, "RELAY-ON", 8))
{
}
else if (!strncmp(cbuff+10, "RELAY-OFF", 9))
{
}
else if (!strncmp(cbuff+10, "TIMER-ON", 8))
{
}
|
"Else if" saves some very time-consuming operations! And I rewrote some other things the way I'd write the code, but being concise in programming has to be balanced against what one is used to seeing--it's better to avoid constructions that we don't recognize easily. |
|
|
Wayne_
Joined: 10 Oct 2007
Posts: 681
|
|
| Posted: Fri Oct 16, 2009 7:29 am |
|
|
My version of CCS specifies that you cannot use a const in the stncpy function so
| Quote: |
iresult=strncmp (s1, s2, n)
Compare s1 to s2 (n bytes)
Parameters:
s1 and s2 are pointers to an array of characters (or the name of an array). Note that s1 and s2 MAY NOT BE A CONSTANT (like "hi").
|
if (!strncmp(cbuff+10, "RELAY-ON", 8))
Does not work. You have to put the string in to a char array first as in my code.
But, if the newer versions of CCS do allow it I would like to know.
Also there is no difference between using cbuff+10 and &cbuff[10]; That I can see in the .lst file so it is up to your preference.
Cheers. |
|
|
John P
Joined: 17 Sep 2003
Posts: 331
|
|
| Posted: Fri Oct 16, 2009 8:21 am |
|
|
| Sorry, that suggestion was based on "standard C" and I don't know for sure what CCS--especially, a particular CCS version--would be able to do. As far as I can recall, I've never needed to do any processing of text strings on a PIC beyond just sending them to a display. |
|
|
bkamen
Joined: 07 Jan 2004
Posts: 1611
Location: Central Illinois, USA
|
|
| Posted: Fri Oct 16, 2009 9:32 am |
|
|
| Wayne_ wrote: |
Does not work. You have to put the string in to a char array first as in my code.
But, if the newer versions of CCS do allow it I would like to know.
Also there is no difference between using cbuff+10 and &cbuff[10]; That I can see in the .lst file so it is up to your preference.
Cheers. |
you can now use #device PASS_STRINGS=IN_RAM and that allows not having to copy to ram first.
-ben
_________________
Dazed and confused? I don't think so. Just "plain lost" will do. :D |
|
|
|
FAQ 
Search
Register
Profile 
Log in to check your private messages 
Log in
