|
|
View previous topic :: View next topic |
Author |
Message |
benoitstjean
Joined: 30 Oct 2007 Posts: 566 Location: Ottawa, Ontario, Canada
|
Garbage serial data not making sense |
Posted: Thu Oct 11, 2012 9:39 am |
|
|
Ok, I have a problem here I can't find what's going-on and it doesn't make any sense at all. I'm using a PIC24HJ128GP306 mounted on the CCS REAL-ICE board. The compiler is 4.132.
> UART1 is going to a GSM module at 115.2kbps and that stream is called SIM908_SERIAL.
> UART2 is going to a PC running CCS's serial monitor on COM1 at 115.2kbps and that stream is called MONITOR_SERIAL.
#INT_RDA is the interrupt for UART1 (SIM908 GSM module). What I do in here basically is that I parse the characters as they are received serially, store them in an array and when I hit the "carriage return" character and I have more than 0 characters, I display the message... and this is where I don't know what's going on.
In plain english:
1) I get the character from the module and store it in the cChar variable.
2) Then, I check if it's a valid ASCII character anywhere between SPACE (0x20) and TILDE (~ / 0x7E) or if it's CARRIAGE RETURN (0x0D) and that my character counter has more than 0 characters
3) If it's between SPACE and TILDE, then I store it in a character array and increment the character counter. For troubleshooting purposes, I added the <fputc( sReceivedData[ulSIM908SerialCharCounter], MONITOR_SERIAL );> to display the received character on my MONITOR CONSOLE.
-> Up to this point, as I receive the serial characters from the SIM908_SERIAL stream, I immediately print them on my MONITOR_SERIAL stream to see the characters on my screen. That works fine. I see all my GSM messages "RDY, +PCASP: 1, GPS Ready etc"....
Then:
4) If the character is CARRIAGE RETURN, that is supposed to mean that it's then 'end' of a GSM message such as "RDY" or "+PACSP: 1" etc. If it's a carriage return, the ulSIM908SerialCharCounter is most likely greater than 0 therefore I simply display the contents of the character array. At this point, the displayed data is pure garbage. But while displaying the individual characters as they are received, then I don't have any problems.
Code: |
#INT_RDA
void INT_SIM908_Serial( void )
{
cChar = fgetc( SIM908_SERIAL );
if(( cChar >= ' ' && cChar <= '~' ) ||
( cChar == 0x0D && ulSIM908SerialCharCounter > 0 ))
{
if( cChar >= 0x20 && cChar <= 0x7E )
{
sReceivedData[ulSIM908SerialCharCounter] = cChar;
fputc( sReceivedData[ulSIM908SerialCharCounter], MONITOR_SERIAL );
ulSIM908SerialCharCounter ++;
}
else
{
if( cChar == 0x0D && ulSIM908SerialCharCounter > 0 )
{
// THE LINE BELOW SIMPLY DISPLAYS GARBAGE AND THE
// GARBAGE IS ALWAYS THE SAME THING
fputs( sReceivedData, MONITOR_SERIAL );
// Clear the string for the next SIM908 command
memset( sReceivedData, NULL, sizeof( sReceivedData ));
}
}
}
}
|
What could be the issue?? Is the serial port speed too fast? I just thought of that while finishing typing this post... |
|
|
temtronic
Joined: 01 Jul 2010 Posts: 9225 Location: Greensville,Ontario
|
|
Posted: Thu Oct 11, 2012 11:05 am |
|
|
off the top of my head...
ISRs MUST be short and fast !
Capture the data into a buffer and exit, do the parsing and any resending of data(to PC..) in the MAIN section of your program.
Also, do you have 'errors' added to the use rs232(....) options?
hth
jay |
|
|
benoitstjean
Joined: 30 Oct 2007 Posts: 566 Location: Ottawa, Ontario, Canada
|
|
Posted: Thu Oct 11, 2012 11:52 am |
|
|
Thanks. I may have found the problem.
This code was working on a PIC18F4620. Going from an 18F4620 with CCS version 4.056 to PIC24 with CCS 4.132, I found new bugs that weren't there before just like the one causing the issue.
I got it to work not long after posting the message. The only difference I did from PIC18 to PIC24 is that instead at the begining of my main(), I added the following lines:
memset( sReceivedData, NULL, sizeof( sReceivedData ));
ulSIM908SerialCharCounter = 0;
After doing this, the errors went away. But I don't understand because in my header file, where I create the character array and the character counter, I declared them like this:
char sReceivedData[DATA_STRING_SIZE] = "";
unsigned long ulSIM908SerialCharCounter = 0;
This has always worked for me but I think that in the newer compiler, it doesn't seem to take for granted that I initialized the variables when creating them therefore I MUST initialize them in my main() before my while(1).
off-topic but related:
The reason I believe this is a "new" bug is that I also have an application that has been functional for over a year on a PIC18F4620 using compiler 4.056. Last week, I had to add a very simple line of code and when I recompiled it under 4.132, the circuit didn't work anymore.
I was able to narrow it down to a line where I do a string compare between two character strings.
I noticed that the 'strcmp' would never return 0 although both strings were identical because I forced them to be the same thing. Then I noticed that when I declared both strings, once of them was a character array of 10 bytes and the other 11 bytes but BOTH had the same 6 characters. As soon as I changed the "10" to an "11", it worked. |
|
|
PCM programmer
Joined: 06 Sep 2003 Posts: 21708
|
|
Posted: Thu Oct 11, 2012 12:46 pm |
|
|
Quote: | fputs( sReceivedData, MONITOR_SERIAL );
|
fputs() expects sReceivedData to be a string. But your ISR code never
writes a string-terminator character of 0x00 at the end of the string.
Without that 0x00 byte, it's not a string.
Quote: | char sReceivedData[DATA_STRING_SIZE] = "";
|
This doesn't fill the entire array with 0x00. It only writes the first byte
as 0x00.
It's all because you weren't making the text in sReceivedData into a
string inside the isr. fputs(), strcmp(), all expect a string. |
|
|
benoitstjean
Joined: 30 Oct 2007 Posts: 566 Location: Ottawa, Ontario, Canada
|
|
Posted: Thu Oct 11, 2012 1:02 pm |
|
|
Ok, well, that's weird and that's different from previous versions as that code worked perfectly on 4.056.
I've always created variables and initialized them at the same time such as:
char sReceivedData[DATA_STRING_SIZE] = "";
since passing a "null" string ( "" ) would seem to always initialize the array with all NULLS. To be honest, I've never ever encountered any issues under 4.056 and prior. So I guess CCS must have added code in later releases such as the one I'm using...
So basically, what you're telling me is that when I create my variable such as above, then in my main I should always use 'memset' to initialize then entire string to NULL:
memset( sReceivedData, NULL, sizeof( sReceivedData )); ?? |
|
|
PCM programmer
Joined: 06 Sep 2003 Posts: 21708
|
|
Posted: Thu Oct 11, 2012 1:28 pm |
|
|
I mean to add the line shown in bold below:
Quote: |
if(cChar == 0x0D && ulSIM908SerialCharCounter > 0 )
{
sReceivedData[ulSIM908SerialCharCounter] = 0; // Add string terminator
fputs( sReceivedData, MONITOR_SERIAL );
// Clear the string for the next SIM908 command
memset( sReceivedData, NULL, sizeof( sReceivedData ));
}
|
|
|
|
benoitstjean
Joined: 30 Oct 2007 Posts: 566 Location: Ottawa, Ontario, Canada
|
|
Posted: Fri Oct 12, 2012 5:50 am |
|
|
Thanks. But it still doesn't explain why it was working in v4.056 and now it's not... other than if CCS modified their code.... and it doesn't make any sense since when declaring the variable < char sReceivedData[ulSIM908SerialCharCounter] = "" >, automatically the array should be filled with nulls....
So then what would be the difference between initializing it with nulls or initializing it with "123456"? If it's initialized, it's initialized, right? |
|
|
RF_Developer
Joined: 07 Feb 2011 Posts: 839
|
|
Posted: Fri Oct 12, 2012 6:18 am |
|
|
benoitstjean wrote: | Thanks. But it still doesn't explain why it was working in v4.056 and now it's not... other than if CCS modified their code.... and it doesn't make any sense since when declaring the variable < char sReceivedData[ulSIM908SerialCharCounter] = "" >, automatically the array should be filled with nulls....
So then what would be the difference between initializing it with nulls or initializing it with "123456"? If it's initialized, it's initialized, right? |
No, as PCM stated, it does NOT imply that the array will be filled with nulls. It only implies that the first character will be a null. The others can be anything. They will most likely be anything other than nulls after a warm start, i.e. restarting a running PIC that's received data previusly, as happens all the time while debugging for example. It will probably be all nulls, i.e. 0, only after a power up reset. This means the code's behaviour in debug is likely NOT to be the same as in release code. To initialise it to all nulls you'd have to do:
Code: |
char sReceivedData[ulSIM908SerialCharCounter] = "\0\0\0\0";
|
where there were one less nulls than the size of the array, as the compiler will (hopefully) put in the final null, or as you've found, explicitly fill it with nulls by memset or memcpy. So, no, there is intialisation and there is initialisation.
Also, the compiler hasn't just been up-issued, its a whole different compiler, PCD for PIC24 rather than PCH for PIC18. There's a lot of possible differences, but yes, code handling, particularly with strings which on 18s and below, are handled awkwardly due to the harvard architecture of the most PICs, until the MIPS based ones. Also the power up behaviour of PICs may change, i.e. as to what, if anything RAM initialises to. C doesn't assume anything about that: you have to explicitly or through compiler specific means such as #pragmas and #device zero_ram type thingies, ensure the default init is what you need. Remember C started in the days of core store and core was essentially non-volatile, hence unless explicitly initialies by start-up code or the programmer, it had what ever was left after the last bit if code to run, even after power cycling.
RF Developer |
|
|
Ttelmah
Joined: 11 Mar 2010 Posts: 19505
|
|
Posted: Fri Oct 12, 2012 12:16 pm |
|
|
It is important to understand, that initialisation of variables, is something that _you_ need to do properly. You can make the compiler initilise a variableto 0, by declaring it as static. Relying on 'previous behaviour', is a sign of bad programming, since unless a behaviour is explicitly defined (as it is for static variables), it is basically down to luck....
Best Wishes |
|
|
benoitstjean
Joined: 30 Oct 2007 Posts: 566 Location: Ottawa, Ontario, Canada
|
|
Posted: Fri Oct 12, 2012 12:29 pm |
|
|
Yes, that's what I'm starting to realize now (at least with the new version).
Thanks all for your help. |
|
|
|
|
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
|