problem in string compare in KEIL c for 8051

[sachinkp21587]

i am facing problem in string compare. i am always finding it unequal. can anybody please sort out this. i have really tried a lot but cant find the solution.

#include <regx51.h>
#include <string.h>

void init_rs232_tx(void);
void tx_data(unsigned char );
void display_tx(unsigned char[]);
unsigned char rx_data(void);
unsigned char chk_response(unsigned char [3]);
unsigned char n_ip[15] = 0;
unsigned char input_port[4];

void main(void)
{
unsigned char ip_en = 8,i=0,op_en = 1;
init_rs232_tx();
debug:
display_tx("+++*");
if(chk_response("<E>"))
goto debug;

void init_rs232_tx(void)
{
P3 = 0XFF;
TMOD = 0x20;
TH1 = -3;
SCON = 0x50;
TR1 = 1;
}

unsigned char rx_data(void)
{
unsigned char a=0;
while(RI == 0);
a = SBUF;
RI = 0;
return a;
}

unsigned char chk_response(unsigned char response[])
{
unsigned char n_data[3],i=0;
for(i=0;i<3;i++)
{
again:
n_data = rx_data();
if(n_data[0] != response[0])
goto again;
}
if(strcmp(n_data,response) == 0)
return 0;
else
return 1;

}

when i am using following function instead of above thn the string seems to be equal to microcontroller.
unsigned char chk_response(unsigned char response[])
{
unsigned char n_data[3] = "<E>",i=0;
if(strcmp(n_data,response) == 0)
return 0;
else
return 1;

}

i am not getting why is it behaving like this.
thanks in advance

 

[alexan_e]

I don't get what you do with this part

---

Code C - [expand]
1
2
3
4
5
6
7
for(i=0;i<3;i++)
{
again:
n_data[i] = rx_data();  // you assign rx_data() result to n_data[i]
if(n_data[0] != response[0])  // and then you always compare the same variables (n_data[0] instead of n_data[i]), if this is true you will stuck in the goto loop
goto again;
}

---

Alex

 

[bigdogguru]

In the past couple of decades of coding C, I can count on one hand how many times I've used a GOTO and it wouldn't require all five fingers.

Usually the presence of a GOTO in someone's code indicates a much larger underlying problem.

What is the exact purpose of the routine and string compare?

I believe if you can describe in detail the exact purpose of the code a better solution can be found.

 

[sachinkp21587]

actually this is for wiznet ethernet module response check
in the main function in line "if(chk_response("<E>"))" i am checking for a <E> as response
so till the first byte received is '<' i am flushing the received data.
whenever i get that one thn i take the next bytes too so as to compare with the expected response.
As in chk_response() i have passed a string to it named "response"(which holds the expected response from the device),i am comparing response's first byte i.e response[0] with received data n_data[0] till it becomes valid.
and when it becomes valid i just put that data into next string byte place

 

[FvM]

I agree with Alex about the strange repeat condition code. But in practice, it will only repeat for i=0 until the first character matches and read in the consecutive characters. So it can work. But n_data[3] is short by one character. It misses room for the string \0 terminator, and code to load it. So strcmp will only work correctly, if n_data has a zero character following it by chance.

As another point, I never saw char nn[] for a formal string pointer argument. But appaerntly, it's accepted by the Keil compiler.

P.S.:

I believe if you can describe in detail the exact purpose of the code a better solution can be found.

Yes, I hope so, too. What I wouldn't do is to perform a rs232 read without knowing, if characters will ever come in.

 

[sachinkp21587]

thanks for response BIGDOGGURU.
actually the purpose is to check for a valid response after issuing a command to the WIZ105SR ethernet module.
like i am sending "+++" firstly
the response should be <E> if is implemented successfully.
if the packet received is a valid one but is not as expected i.e. <E> but is something like say <S> thn i have to issue the same command again till it gets implemented successfully.
i want this response check to be a generic function otherwise one bye one byte compare can be done easily. i want to be implemented in this way only i.e by strcmp as it will also help me in some other bigger projects that i am working on right now.
i hope this information helps u ppl to get the point wt i meant to say

---------- Post added at 20:35 ---------- Previous post was at 20:25 ----------

Mr. FVM
i have also tried the way you are telling to insert NULL at last manually.
also as far as i know when we are declaring a string say char n[3] we are allowed to access n[0] n[1] and n[2] since compiler itself inserts null at n[3].
whatever, this didnt helped sir

 

[bigdogguru]

What I wouldn't do is to perform a rs232 read without knowing, if characters will ever come in.

FvM is correct. What you don't want to do is constantly poll for characters hoping one of these days they arrive.

Doing so leaves you at the mercy of the WIZ105SR Ethernet Module and possibly stuck in an endless loop, doing nothing but burning cycles.

This is a good example of when to use an interrupt routine, that MCU can be handling other tasks instead of waiting aimlessly.

What variant of '51 are you using in your design?

---------- Post added at 20:57 ---------- Previous post was at 20:53 ----------

I believe what FvM is pointing out is the n_data[3] should be n_data[4] terminated by a null character for the strcmp() to function properly.

 

[FvM]

also as far as i know when we are declaring a string say char n[3] we are allowed to access n[0] n[1] and n[2] since compiler itself inserts null at n[3].

Double wrong.
- You must reserve space for the terminator by specuifying n_char[4], otherwise n[3] will be possibly assigned to other variables
- The compiler will insert a null in case of stringcopy or for string constants, but not for single char copy. It neither will initialize local variables, thus assigning n_char[3]=0 is necessary.

There may be other problems with your code, but the said point can cause it's failure.

 

[sachinkp21587]

thank u mr FVM. i was mistaken at at some point when i said i have tried making the string to be n[4].
i tried this way n its working now.
n thanks for info of insertion of null when to do forcefully and when not to.
n this is just for testing purpose right now. now i will do the receive part in interrupt only so no time issue thn.
thanks again to all of u