|
|
View previous topic :: View next topic |
Author |
Message |
Gabriel
Joined: 03 Aug 2009 Posts: 1067 Location: Panama
|
Not so stuck anymore... |
Posted: Sat Aug 15, 2009 5:51 pm |
|
|
hi all...
I have these 3 functions which depend on each other.
I have a long data string coming to my pic through a gsm module
(SE T290A).
I got communication with the module done, previous to these functions.
My programs works fine until this point.
The problem is that when I give the command to receive a text message,
I get a pretty long string, that varies in size.
This string contains miscellaneous data, phone#, date, time, actual text msg, etc.
M pic is supposed to find the "actual text message", which is a command
I send via sms, as in, a sms I type on another phone.... pretty basic concept... I'm sure you guys know the drill.
Below is an example of received text msg.
Quote: |
AT+CMGR=2\0D
\0D
\0A
+CMGR: "REC UNREAD","01150971734","Dn55 Movil","09/08/14,19:51:49-12"\0D
\0A
*LED1 1*\0D
\0A
\0D
\0A
OK\0D
\0A
|
*Led1 1* being the actual message... and the "command" / string I'm looking for.
I wrote 3 small functions to get the job done.
In order of appearance as will be posted below.
First function, ignores all the characters until a '*' is found.
and then resets the buffer counter so that the string
Led1 1* starts at position 0 in my buffer (its a circular buffer).
Second function looks for the string...
If found sets a global variable.
Third function takes the formentioned global variable and
using ifs executes a command... in this case... on/off a led.
Code: |
int READ_SMS()
{
counter_read=0;
printf("AT+CMGR=1"); // send command, MEMORY LOCATION OF SMS IS ALWAYS ONE, SINCE I DELETE THEM AFTER PROCESSING
putchar(0x0D);
while(recieve_string[counter_read]!='*') // ignore everything untill message starts
{
}
counter_read=0; // start saving message at the begining of the array
while(recieve_string[counter_read]!='*') // get the message
{
}
counter_read=0;
delay_ms(1000);
}
int GET_CMD()
{
const char CMD1[]={"Led1 0"};
const char CMD2[]={"Led1 1"};
const char CMD3[]={"Led2 0"};
const char CMD4[]={"Led2 1"};
const char CMD5[]={"Led3 0"};
const char CMD6[]={"Led3 1"};
const char CMD7[]={"Led4 0"};
const char CMD8[]={"Led4 1"};
NEXT_CMD=0xff;
strcpy (CMD_string, CMD1);
if(strstr(Recieve_String,CMD_string))
NEXT_CMD=0;
strcpy (CMD_string, CMD2);
if(strstr(Recieve_String,CMD_string))
NEXT_CMD=1;
strcpy (CMD_string, CMD3);
if(strstr(Recieve_String,CMD_string))
NEXT_CMD=2;
strcpy (CMD_string, CMD4);
if(strstr(Recieve_String,CMD_string))
NEXT_CMD=3;
strcpy (CMD_string, CMD5);
if(strstr(Recieve_String,CMD_string))
NEXT_CMD=4;
strcpy (CMD_string, CMD6);
if(strstr(Recieve_String,CMD_string))
NEXT_CMD=5;
strcpy (CMD_string, CMD7);
if(strstr(Recieve_String,CMD_string))
NEXT_CMD=6;
strcpy (CMD_string, CMD8);
if(strstr(Recieve_String,CMD_string))
NEXT_CMD=7;
output_high(pin_d1);
delay_ms(1000);
output_low(pin_d1);
delay_ms(1000);
}
Void DO_CMD()
{
if(NEXT_CMD==0)output_low(PIN_B1);
if(NEXT_CMD==1)output_high(PIN_B1);
if(NEXT_CMD==2)output_low(PIN_B2);
if(NEXT_CMD==3)output_high(PIN_B2);
if(NEXT_CMD==4)output_low(PIN_B3);
if(NEXT_CMD==5)output_high(PIN_B3);
if(NEXT_CMD==6)output_low(PIN_B4);
if(NEXT_CMD==7)output_high(PIN_B4);
output_high(pin_d1);
delay_ms(1000);
output_low(pin_d1);
delay_ms(1000);
}
|
I wrote the code in a very non compact way.
Just to get it working, and cause I'm pretty burned from coding and I'm not getting too many bright ideas lately. I know it can be done much more efficiently, if you guys have any suggestions.... ways to compact the code... all welcomed.
Below I posted my ISR in case it helps.
Code: |
#INT_RDA
void SerialInt()
{
Recieve_String[counter_read]=getchar();
counter_read++;
if(counter_read==69)counter_read=0;
}
|
Thank you, I'm fried.
gabriel... _________________ CCS PCM 5.078 & CCS PCH 5.093
Last edited by Gabriel on Mon Aug 17, 2009 4:25 pm; edited 1 time in total |
|
|
Ttelmah Guest
|
|
Posted: Sun Aug 16, 2009 2:52 am |
|
|
Several comments.
First strstr, looks for a string, in a string. As in your previous question, a _string_, needs to be 'null terminated'. There is no guarantee that your circular buffer, will contain a null termination (there is nothing to put this there), so the strstr function, will end up searching through the whole of memory, if it does not find a match....
You need to declare your buffer one character larger than you use (so if the buffer is 69 charcaters, declare a 70 character buffer space), and then load the extra character with a 'null' character, so strstr, will find this and stop searching.
Then, strstr, is going to take quite a long time, if the buffer is large. Why not 'break' out, from your test sequence, as soon as a match is found?.
Then, strstr, does not return true/false. It returns the location in the first string, where the second is found. Your tests need to bear this in mind.
Now, you are doing a lot of repeated work, that is unnecessary, searching the whole buffer, for the entire sequence. Instead simplify your searching. Effectively, search through the buffer, till 'L' is found. Then from this location, see if you have the next two characters 'ed'. If so, then look just in the next three characters, for '1 0' etc.. The actual amount of processing time being spent searching the entire buffer, will be enormous....
Best Wishes |
|
|
Gabriel
Joined: 03 Aug 2009 Posts: 1067 Location: Panama
|
|
Posted: Sun Aug 16, 2009 12:26 pm |
|
|
hi Ttelmah
This is my first attempt at using strstr...i guess it shows hahah
I was under the impression that strstr returned a null character if the string was not found... i build my function to work based on this...
I guess i was wrong.... in anycase this was a last desperate attempt..
Just trying to get a "shorter" way of doing this... if you can believe that hahah
My buffer size is 70... thus my ISR loops at 69...i get this...
I tried doing it this way since i dont really know a SHORT way of finding continuos characters in a buffer... exept with ifs...
The more continuos characters.. the more ifs.... and it doesent really solve much of my problem if my commands are not exactly the same size and have alot of repeated letters.... so i think....
got any suggestions?
any sort of help is much apreciated _________________ CCS PCM 5.078 & CCS PCH 5.093 |
|
|
Gabriel
Joined: 03 Aug 2009 Posts: 1067 Location: Panama
|
|
Posted: Sun Aug 16, 2009 12:28 pm |
|
|
forgot to mention....
Time is not really an issue in my situation...for my circuit that is.
I got 2 or 3 seconds to spare....even 5
g _________________ CCS PCM 5.078 & CCS PCH 5.093 |
|
|
Ttelmah Guest
|
|
Posted: Mon Aug 17, 2009 8:40 am |
|
|
Lets start with the simplest bit. Have you added the null terminator to the input buffer?. This is the 'killer' for the code at present. Without it, the search will walk through memory, till it either hits a '0' there by accident, or finds the string you are searching for. Remember that your search string, is itself in memory (you have copied it there, to do the search), so without the terminator, the 'odds' are that all the tests will always return a non zero value....
Now, you can save a lot of work, by not searching the whole lot every time.
Simply 'walk' through the buffer yourself, looking for the letter 'L'.
Then check if the rest of the message is present, using strncmp, from this location.
The advantage here is that you don't have to keep going through the whole buffer each time.
Alternatively, and better really, forget about searching the buffer at all. Use a state machine, on the incoming data.
Start in 'state0'. In this, check for a '*', and when it is seen, advance to state1.
In state1, check for the letter 'L'. If it is seen, advance to state2.
In state2, check for 'e'. If it is seen, advance to state3, if not, go back to state1, and check again.
In state3, check for 'd'. If it is seen, advance to state4. If not, go back to state1, and try again.
Similarly, then look for 1,2,3 & 4, then for the space, and finally the 0 or 1.
There only needs to be seven tests, and on any individual character, there will normally only be a single test used....
Best Wishes |
|
|
Gabriel
Joined: 03 Aug 2009 Posts: 1067 Location: Panama
|
|
Posted: Mon Aug 17, 2009 2:25 pm |
|
|
hi
Quote: | Then check if the rest of the message is present, using strncmp, from this location. |
How?
I thought strncmp took the name of the string... i give it an index? what?
The ccs help index tab is not very explicative... i hate when they generalize the examples and function explanations... like they do with string.
...
Anyways i changed my code... it works... but i still need help with it..
I need to condense it a bit... and make it a tad more efficient...
I feel i repeat too much code... but ill see if i can handle it today when i get home... else ill post my function here...
As a side note: even though sometimes after you or any other posts a suggestion or advice i still read it, even thought it might not apply to the current version of my code..... learning is something i can always do some more of.
thank you
g _________________ CCS PCM 5.078 & CCS PCH 5.093 |
|
|
Ttelmah Guest
|
|
Posted: Mon Aug 17, 2009 3:07 pm |
|
|
You need to learn more C.
In C, the _name_ of an array, is a shortcut notation, for the pointer to the first element. Strncmp, can be used to compare 'n' characters from anywhere in two arrays, by just giving it the required pointers and count.
All the string functions, have source code available to you. Also they are described in any C book.
The first line of the 'help' parameters sectio, tells you everything:
"s1 and s2 are pointers to an array of characters (or the name of an array)."
Since the search functions return pointers, you can use the value returned as the point you want to search _from_.
Best Wishes |
|
|
mkuang
Joined: 14 Dec 2007 Posts: 257
|
|
Posted: Mon Aug 17, 2009 3:35 pm |
|
|
I think this:
Code: |
const char CMD1[]={"Led1 0"};
|
should be written as:
Code: |
const char CMD1[]="Led1 0";
|
The null termination character \0 is then automatically attached to the end of the string.
Also is the '0' in "LED1 0" suppose to be a null termination character? IF it is the syntax is \0.
Also if you look at the input/output variables of some of the functions in string.h, anything that says it accepts or returns a pointer is going to complain about you declaring a constant char array (a location in ROM and not RAM). |
|
|
Gabriel
Joined: 03 Aug 2009 Posts: 1067 Location: Panama
|
i got it working.... |
Posted: Mon Aug 17, 2009 4:07 pm |
|
|
so...
i got it working... pretty neatly i think... im still open for suggestions.
it can be optimized i think...and would love to hear your thoughts
Code: |
int GET_CMD()
{
const char CMD1[8][7]={"Led1 0","Led1 1","Led2 0","Led2 1","Led3 0","Led3 1","Led4 0","Led4 1"};
int offset=0;
int store_counter=0;
counter_search=0;
while((Recieve_String[counter_search]!='!')&&(counter_search<69)) // wait till command indicator is found '!'
{
counter_search++;
}
counter_search=counter_search+1; // increment one to actuall first letter of command
store_Counter=counter_search; // store current possition for multiple searches
NEXT_CMD=0; // NEXT_CMD keeps track of the command being read, thus used
while((HitCounter!=6)&&(NEXT_CMD<8)) // compare to all commands in list.
{
counter_search=store_Counter; // initialize counter search with stored counter value.
offset=0; // since value of counter is unknown use a separate counter for the const array
HitCounter=0; // counts number of equal letters found.
while((HitCounter!=6)&&(offset<=6)) // keeps the counters in check...to not overshoot. and compares the strings
{
if(Recieve_String[counter_search]==CMD1[NEXT_CMD][offset]) // if letter is found
HitCounter++;
offset++;
counter_search++;
}
if(HitCounter==6) // if 6 chars found...exit..
{
Return(1);
}
NEXT_CMD++; // if increase to search next command.
}
Return(0);
}
|
works quite nicely... i have control over 4 LEDs via sms....
i had a moment of clarity... i guess i was just stuck and frustrated...and blind as a consequence...
Quote: |
You need to learn more C.
|
....OUCH!...
its been a few years.... i was wicked when i was 18... graduated top of my C++ IB class (AP for you guys in the US)...
even got to teach some assembler in college...top in my microcontroller class....
learned pearl as well... wasnt that good at it hahaha...
and even some ladder logic...that was awesome.
im not bragging... im sure that pretty much all of the people here giving help can kick my [spam] at programming no doubt...thats why i come here for help... my point is... i am rusty as hell.... and i dont have any of my books here...i do have plenty... dusty, most of them... but i am not in my country right now... i live 9 months of every year traveling because of work.
this is my first attempt at programing anything in 5 years?
stay away from programing that long...and you will be me.
im sorry if i ask stupid questions... but after that long, im kinda improvising. and yes... i should learn more C. but for now i do what i can.
thanks for your help...
love to hear comments on my function of you guys have any.
g _________________ CCS PCM 5.078 & CCS PCH 5.093
Last edited by Gabriel on Mon Aug 17, 2009 4:22 pm; edited 1 time in total |
|
|
PCM programmer
Joined: 06 Sep 2003 Posts: 21708
|
|
Posted: Mon Aug 17, 2009 4:10 pm |
|
|
Quote: |
I think this:
const char CMD1[]={"Led1 0"};
should be written as:
const char CMD1[]="Led1 0";
The null termination character \0 is then automatically attached to
the end of the string. |
The compiler generates identical code if the braces are used, or if they
are not used. In both cases, the compiler inserts the string terminator
byte (0x00) at the end of the string. Here is the .LST file.
Note: You need to comment out the #nolist statement at the top of the
.H file for your PIC, and then re-compile, to see this code in the .LST file.
Code: |
0004: BCF 0A.0
0005: BCF 0A.1
0006: BCF 0A.2
0007: ADDWF 02,F
0008: RETLW 4C // 'L'
0009: RETLW 65 // 'e'
000A: RETLW 64 // 'd'
000B: RETLW 31 // '1'
000C: RETLW 20 // space
000D: RETLW 30 // '0'
000E: RETLW 00 // String terminator |
Code: | #include <16F877.H>
#fuses XT,NOWDT,PUT,BROWNOUT,NOLVP
#use delay(clock=4000000)
#use rs232(baud=9600, xmit=PIN_C6, rcv=PIN_C7, ERRORS)
const char CMD1[]={"Led1 0"};
//======================================
void main(void)
{
puts(CMD1);
while(1);
} |
|
|
|
Gabriel
Joined: 03 Aug 2009 Posts: 1067 Location: Panama
|
|
Posted: Mon Aug 17, 2009 4:12 pm |
|
|
Mkuang:
I'm using "LED1 1" means LED#1 ON...
and "LED1 0" means LED#1 OFF...
g _________________ CCS PCM 5.078 & CCS PCH 5.093 |
|
|
Gabriel
Joined: 03 Aug 2009 Posts: 1067 Location: Panama
|
while we are at it |
Posted: Mon Aug 17, 2009 4:20 pm |
|
|
I have to send via rs232 the following command
Code: |
AT+CPMS="ME","ME","ME"\0D
|
Inside a printf function the "" marks mess it up.
So I resorted to using this which I hate:
Code: |
printf("AT+CPMS="); //text part of the command.
putchar(0x22); // print " (printf uses " so print them as hex individually.
printf("ME"); // phone memory, for getting sms
putchar(0x22); // print "
putchar(0x2C); // print coma.
putchar(0x22); // the same but for writing sms
printf("ME");
putchar(0x22);
putchar(0x2C); // print coma
putchar(0x22); // sms storage memory.
printf("ME");
putchar(0x22);
putchar(0x0D); // enter
|
Can't seem to get around those ""...
Any suggestions?
g _________________ CCS PCM 5.078 & CCS PCH 5.093 |
|
|
PCM programmer
Joined: 06 Sep 2003 Posts: 21708
|
|
|
Gabriel
Joined: 03 Aug 2009 Posts: 1067 Location: Panama
|
|
Posted: Mon Aug 17, 2009 7:16 pm |
|
|
PCM programmer
nice tutorial..
like it. thank you
g _________________ CCS PCM 5.078 & CCS PCH 5.093 |
|
|
|
|
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
|