Hello all, I am hoping someone has some incite as to what I am doing wrong here,
I figure it is a small problem but I am out of ideas. This is just a test to
figure out how to pass char * through methods. The code should print Hello World
to the serial port which it does, then print tes repeatedly, which it does not,
rather the output is as follows:
Hello
World"""""""""""""""""""""""""""""""""""""""""""""""""""""""
""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
Here is the code, also below is the sci.c and sci.h that do the actually serial
communication.
Thank You in advanced !
Zach Long
MAIN:
SCI include:
Dragon Board 12 Serial Communication C code Help
Started by ●October 26, 2009
Reply by ●October 27, 20092009-10-27
Probably malloc didn't succeed
char* outData =(char*) malloc (sizeof (char*)* 5); // make room for 5 char
BTW comment is wrong, should be make room for 5 pointers to char
Edward
char* outData =(char*) malloc (sizeof (char*)* 5); // make room for 5 char
BTW comment is wrong, should be make room for 5 pointers to char
Edward
Reply by ●October 27, 20092009-10-27
> Edward Karpicz writes on 12:51 AM 10/27/2009
>Probably malloc didn't succeed
>
>char* outData =(char*) malloc (sizeof (char*)* 5); // make room for 5 char
>
>BTW comment is wrong, should be make room for 5 pointers to char
Both points are good.
Furthermore, for this series of processors or smaller and generally
in embedded systems, avoid malloc and free. These smaller processors
are just not that good at supporting dynamic memory. For a Freescale
PowerPC with 100's of MB of memory - fine, dynamic memory using
malloc and free can be used. But you must always consider that
memory can fragment and requests for memory can fail over long runs
of the program (malloc returns NULL on error).
I'm not looking for a big debate on malloc/free in an embedded
system. On a system with less than 32MB of RAM, I just don't do it - ever.
Bill
>Probably malloc didn't succeed
>
>char* outData =(char*) malloc (sizeof (char*)* 5); // make room for 5 char
>
>BTW comment is wrong, should be make room for 5 pointers to char
Both points are good.
Furthermore, for this series of processors or smaller and generally
in embedded systems, avoid malloc and free. These smaller processors
are just not that good at supporting dynamic memory. For a Freescale
PowerPC with 100's of MB of memory - fine, dynamic memory using
malloc and free can be used. But you must always consider that
memory can fragment and requests for memory can fail over long runs
of the program (malloc returns NULL on error).
I'm not looking for a big debate on malloc/free in an embedded
system. On a system with less than 32MB of RAM, I just don't do it - ever.
Bill
Reply by ●October 27, 20092009-10-27
I agree that malloc failed and it is passing back 0 to indicate that
it failed, and you are using that as an address of your string.
Please have some empathy for the programmer that has to figure out
your bugs. Check your return codes, don't use malloc, calloc, realloc,
global variables, GOTOs, or "magic numbers".
A
it failed, and you are using that as an address of your string.
Please have some empathy for the programmer that has to figure out
your bugs. Check your return codes, don't use malloc, calloc, realloc,
global variables, GOTOs, or "magic numbers".
A
Reply by ●October 27, 20092009-10-27
Thanks everyone, your comments on not using malloc make sense for this system,
however I am still left with the issue of incorrect output, and the thought of
how that pointer works if it is never assigned to memory. Not to get side
tracked, if anyone has any working examples of printing variable data to the
serial port I would be very interested in seeing it, since this is the goal of
this series of post.
Thank You,
Zach
Thank You,
Zach
Reply by ●October 27, 20092009-10-27
Zach,
The code for the serial port looks OK, although for production code it would benefit from a code review.
SCI_OutStatus(1); // Is it clear to send (buffer empty)
This returns the status, so as is in main does nothing. Since SCI_OutString checks the SCI status for each character, testing the status before the call isn't needed anyway.
The following might work (I don't have a running system or I would run it).
Bill
-------------
The code for the serial port looks OK, although for production code it would benefit from a code review.
SCI_OutStatus(1); // Is it clear to send (buffer empty)
This returns the status, so as is in main does nothing. Since SCI_OutString checks the SCI status for each character, testing the status before the call isn't needed anyway.
The following might work (I don't have a running system or I would run it).
Bill
-------------
Reply by ●October 28, 20092009-10-28
Okay, let's take a look at the output and the evidence and make some
wild ass guesses.
You can assume that SCI_OutString is okay since 1) the "Hello World"
comes out okay and 2) it is what is known as gospel code - written by
a god like being and written in a book.
Next, in your message you say "then print tes repeatedly". Well, it
isn't printing tes (plural of T??) it is printing double quotes.
Nowhere in your code do you have a buffer that should be holding a ".
So we can assume that either encode() isn't working properly (but by
inspection it is pretty simple), or when you call SCI_OutString, you
are using a garbage pointer and it is printing random stuff.
In the definition of encode - either use pointers or arrays, don't
intermix. Use void encode( char out[]) instead. What you have would
probably cause all kinds of damage.
Now, the character " has the hex value 0x22, so what you are printing
looks like 22222222222222222... in hex, with no 0x00 to terminate the
string, so it's going to print a whole bunch of it. It is quite hard
to determine what you are printing since you are in an infinite loop.
The data could be 0x2200, which would print out a single ", but since
it is in an infinite loop you would get """"""""""""""""""""""""""".
Running your code through lint showed that if malloc failed you would
have outData getting set to NULL. This would be a pointer to location
0. Encode would try and alter locations 0, 1, 2 and 3. Then you print
the string at location 0. The zero page of an HC12 is internal
registers. So you would set PORTA to 0x74, PORTB to 0x65, DDRA to
0x73, DDRB to 0x00, then print them out as a string.
If you are running some sort of a bootloader/monitor/debugger it is
possible to move the registers and possibly put FLASH down in the zero
page, or uninitialized RAM. It's really hard to predict what is going
to get printed.
Either way, you are printing a location that IS a long string of hex
0x22. We can see that from the output.
First, alter your code to have:
No infinite loop until you get a bit of control.
Fix encode as above.
char outData[5];
Then, use SCI_OutChar(outData[0]); and make sure that you are getting
a 't'. This would make sure that encode is working properly.
Fix the malloc call so that the syntax is expressing what you actually
want and check the return code. Use SCI_OutString to print an error
message if malloc returns NULL.
If you really want to use malloc, use SCI_OutUHex((int)outData) and
take a look at where outData is pointing. I'm pretty sure that you
will find a whole bunch of 0x22 at that location.
From the 10 commandments: Thou shalt run lint frequently and study
its pronouncements with care, for verily its perception and judgement
oft exceed thine.
From the MISRA guidelines: Don't use pointers.
Andrei
---------------------
Andrei Chichak
Systems Developer
CBF Systems Inc.
4-038 NINT Innovation Centre
11421 Saskatchewan Drive
Edmonton, Alberta
Canada
T6G 2M9
wild ass guesses.
You can assume that SCI_OutString is okay since 1) the "Hello World"
comes out okay and 2) it is what is known as gospel code - written by
a god like being and written in a book.
Next, in your message you say "then print tes repeatedly". Well, it
isn't printing tes (plural of T??) it is printing double quotes.
Nowhere in your code do you have a buffer that should be holding a ".
So we can assume that either encode() isn't working properly (but by
inspection it is pretty simple), or when you call SCI_OutString, you
are using a garbage pointer and it is printing random stuff.
In the definition of encode - either use pointers or arrays, don't
intermix. Use void encode( char out[]) instead. What you have would
probably cause all kinds of damage.
Now, the character " has the hex value 0x22, so what you are printing
looks like 22222222222222222... in hex, with no 0x00 to terminate the
string, so it's going to print a whole bunch of it. It is quite hard
to determine what you are printing since you are in an infinite loop.
The data could be 0x2200, which would print out a single ", but since
it is in an infinite loop you would get """"""""""""""""""""""""""".
Running your code through lint showed that if malloc failed you would
have outData getting set to NULL. This would be a pointer to location
0. Encode would try and alter locations 0, 1, 2 and 3. Then you print
the string at location 0. The zero page of an HC12 is internal
registers. So you would set PORTA to 0x74, PORTB to 0x65, DDRA to
0x73, DDRB to 0x00, then print them out as a string.
If you are running some sort of a bootloader/monitor/debugger it is
possible to move the registers and possibly put FLASH down in the zero
page, or uninitialized RAM. It's really hard to predict what is going
to get printed.
Either way, you are printing a location that IS a long string of hex
0x22. We can see that from the output.
First, alter your code to have:
No infinite loop until you get a bit of control.
Fix encode as above.
char outData[5];
Then, use SCI_OutChar(outData[0]); and make sure that you are getting
a 't'. This would make sure that encode is working properly.
Fix the malloc call so that the syntax is expressing what you actually
want and check the return code. Use SCI_OutString to print an error
message if malloc returns NULL.
If you really want to use malloc, use SCI_OutUHex((int)outData) and
take a look at where outData is pointing. I'm pretty sure that you
will find a whole bunch of 0x22 at that location.
From the 10 commandments: Thou shalt run lint frequently and study
its pronouncements with care, for verily its perception and judgement
oft exceed thine.
From the MISRA guidelines: Don't use pointers.
Andrei
---------------------
Andrei Chichak
Systems Developer
CBF Systems Inc.
4-038 NINT Innovation Centre
11421 Saskatchewan Drive
Edmonton, Alberta
Canada
T6G 2M9
Reply by ●October 28, 20092009-10-28
As stated by others it is folly to use malloc/free in simple embedded systems.
The best solution is to implement asynchronous static ring buffers for rx/tx
with timeout error reporting (see
http://www.seattlerobotics.org/encoder/200009/sciint11.c for an asynch example
without timeouts). In the case where the output is fixed I wouldn't even
bother with a buffer.
If the output is variable, the addition of the something like the following can be useful. The obvious danger of using this function is the ease of overflowing the buffer. Beware also that compiler implementations of this are typically not re-entrant so don't call it from inside an ISR or thread.
-rob
>
> #include "sci.h"
>
> int main(void);
> void encode(char *out);
>
> int main(void)
> {
> char* head = "Hello World"; // declare first print statement
> char* outData =(char*) malloc (sizeof (char*)* 5); // make room for 5 char
>
> SCI_Init(9600,0); //Startup serial port 0(under LCD) at 9600 BAUD
> SCI_OutStatus(1); // Is it clear to send (buffer empty)
> SCI_OutString(head,0); // print "Hello World" to serial port (WORKS !!)
>
> while(1)
> {
> if(SCI_OutStatus(0))// Is it clear to send (buffer empty)
> {
> encode(outData);//,out.throt,out.heading); // build output
> SCI_OutString(outData,0); // print out put to serial port
> }
> }
> return 0;
> }
>
> void encode(char *out) // take in pointer and fill with chars
> {
> out[0] = 't';
> out[1] = 'e';
> out[2] = 's';
> out[3] = 0;
> }
If the output is variable, the addition of the something like the following can be useful. The obvious danger of using this function is the ease of overflowing the buffer. Beware also that compiler implementations of this are typically not re-entrant so don't call it from inside an ISR or thread.
-rob
>
> #include "sci.h"
>
> int main(void);
> void encode(char *out);
>
> int main(void)
> {
> char* head = "Hello World"; // declare first print statement
> char* outData =(char*) malloc (sizeof (char*)* 5); // make room for 5 char
>
> SCI_Init(9600,0); //Startup serial port 0(under LCD) at 9600 BAUD
> SCI_OutStatus(1); // Is it clear to send (buffer empty)
> SCI_OutString(head,0); // print "Hello World" to serial port (WORKS !!)
>
> while(1)
> {
> if(SCI_OutStatus(0))// Is it clear to send (buffer empty)
> {
> encode(outData);//,out.throt,out.heading); // build output
> SCI_OutString(outData,0); // print out put to serial port
> }
> }
> return 0;
> }
>
> void encode(char *out) // take in pointer and fill with chars
> {
> out[0] = 't';
> out[1] = 'e';
> out[2] = 's';
> out[3] = 0;
> }
Reply by ●October 29, 20092009-10-29
> Andrei Chichak writes on 02:20 AM 10/28/2009
>In the definition of encode - either use pointers or arrays, don't
>intermix. Use void encode( char out[]) instead. What you have would
>probably cause all kinds of damage.
There is no harm to use pointers and arrays and no damage results
from doing so. They are equivalent - the only time a problem will
occur is when a pointer and array are used in different modules (one
public and the other externed).
Bill
>In the definition of encode - either use pointers or arrays, don't
>intermix. Use void encode( char out[]) instead. What you have would
>probably cause all kinds of damage.
There is no harm to use pointers and arrays and no damage results
from doing so. They are equivalent - the only time a problem will
occur is when a pointer and array are used in different modules (one
public and the other externed).
Bill
Reply by ●October 31, 20092009-10-31
--- In 6..., Andrei Chichak wrote:
> On 2009-October-28, at 11:06 PM, Bill Auerbach wrote:
> >> > Andrei Chichak writes on 02:20 AM 10/28/2009
> >> >In the definition of encode - either use pointers or arrays, don't
> >> >intermix. Use void encode( char out[]) instead. What you have would
> >> >probably cause all kinds of damage.
You say "probably cause all kinds of damage". Before anyone here thinks that's a really bad thing and it means they are doing something wrong, perhaps you should back this statement with something concrete. Show us a case that causes "all kinds of damage". If you can't, please don't put your gloom and doom programming rules out on a public forum where beginners may be trying to learn correct programming practices.
> >> There is no harm to use pointers and arrays and no damage results
> >> from doing so. They are equivalent - the only time a problem will
> >> occur is when a pointer and array are used in different modules (one
> >> public and the other externed).
> >>
> >> Bill
> >
>> Remember, the program will only be read twice after you write it,
>> once by the compiler (and it doesn't care what you do), and the poor
>> sod that has to maintain it.
It's a much larger concern and problem if the maintainer doesn't know the fundamental properties and semantics of the language.
> On 2009-October-28, at 11:06 PM, Bill Auerbach wrote:
> >> > Andrei Chichak writes on 02:20 AM 10/28/2009
> >> >In the definition of encode - either use pointers or arrays, don't
> >> >intermix. Use void encode( char out[]) instead. What you have would
> >> >probably cause all kinds of damage.
You say "probably cause all kinds of damage". Before anyone here thinks that's a really bad thing and it means they are doing something wrong, perhaps you should back this statement with something concrete. Show us a case that causes "all kinds of damage". If you can't, please don't put your gloom and doom programming rules out on a public forum where beginners may be trying to learn correct programming practices.
> >> There is no harm to use pointers and arrays and no damage results
> >> from doing so. They are equivalent - the only time a problem will
> >> occur is when a pointer and array are used in different modules (one
> >> public and the other externed).
> >>
> >> Bill
> >
>> Remember, the program will only be read twice after you write it,
>> once by the compiler (and it doesn't care what you do), and the poor
>> sod that has to maintain it.
It's a much larger concern and problem if the maintainer doesn't know the fundamental properties and semantics of the language.
