tags:

views:

298

answers:

6

Hello,

In my application, I have a char array defined which can take one of three options: "okay", "high", "low" which are then sent down a serial port to a remote device. I currently have the array sized to take the 4 character words plus carriage return and line feed, but when I have to send "low" I get a null character in the strings, which I am concerned would confuse the host terminal.

array definition

char mod1_status_char[6] = {'0','0','0','0','0','0'};     
char mod2_status_char[6] = {'0','0','0','0','0','0'};     
char mod3_status_char[6] = {'0','0','0','0','0','0'};     

sample of switch case statement:

void DCOKStatus(uint8_t *ptr_status)
{
    uint8_t status = *ptr_status;

    switch (status) 
    {
        case 0x00:
            strcpy(mod1_status_char, "okay");
            strcpy(mod2_status_char, "okay");
            strcpy(mod3_status_char, "okay");
            break; 
        case 0x10:
            strcpy(mod1_status_char, "okay");
            strcpy(mod2_status_char, "okay");
            strcpy(mod3_status_char, "low");
            break;
     }

This is the struct which makes the message string to send

    strcpy(MsgStatus_on.descriptor_msg, "$psu_");
    MsgStatus_on.address01 = hex_addr[0];
    MsgStatus_on.address02 = hex_addr[1];
    MsgStatus_on.space01 = 0x20;
    strcpy(MsgStatus_on.cmdmsg01, "op_en op1_");
    strcpy(MsgStatus_on.statusmsg01, mod1_status_char);
    MsgStatus_on.space02 = 0x20;
    strcpy(MsgStatus_on.cmdmsg02, "op2_");
    strcpy(MsgStatus_on.statusmsg02, mod2_status_char);
    MsgStatus_on.space03 = 0x20;
    strcpy(MsgStatus_on.cmdmsg03, "op3_");
    strcpy(MsgStatus_on.statusmsg03, mod3_status_char);
    MsgStatus_on.CR = 0x0D;
    MsgStatus_on.LF = 0x0A;

and this sends the message

void USARTWrite(char *object, uint32_t size)
{    
    GPIO_SetBits(GPIOB, GPIO_Pin_1);

    char *byte;
    for (byte = object; size--; ++byte)                                                                       
    {                                       
          USART_SendData(USART1,*byte);                                 

    }

Would anyone be able to suggest a good approach to dynamically size the array to one character shorter when I need to send "low"?

Thanks

+6  A: 

I don't think a dynamically sized array is called for. There are two ways in C literally to dynamically size an array: allocate it with malloc or similar; or use C99 VLAs. But in this case where you have strings of different lengths, surely the important point is to write the correct bytes in the correct order? Personally I'd prefer something like this, maybe:

char * strings[] = {"okay\r\n", "high\r\n", "low\r\n"};

serial_send(strings[msg_number], strlen(strings[msg_number]));

You don't have to call strlen, necessarily, you could store the lengths in another array. But even on the tiniest embedded device, counting to 6 takes very little time compared with sending serial data.

Of course I'm assuming that whatever function you call to actually send the data, takes a pointer and a length. But if it doesn't, I don't see how a dynamically sized array helps either.

I think the general problem here which makes it difficult to answer the question, is that you don't really say what the "size" of your array is, or why it has anything to do with the number of bytes actually written to the serial port.

Edit: with your additional explanation, the key thing seems to be this struct that the three individual strings are "passed into". Not sure what passing a string into a struct means. If it currently looks like this:

struct serialmessage {
    char first[6];
    char second[6];
    char third[6];
};

serialmessage msg;
memcpy(msg.first, mod1_status_char, 6); // etc.

Then maybe it would be better to do this:

char *char mod1_status_char; // etc.

switch(status) {
    case 0x00:
        mod1_status_char = strings[0]; // or #define STATUS_OK 0
        mod2_status_char = strings[0];
        mod3_status_char = strings[0];
        break;
    case 0x10:
        mod1_status_char = strings[0];
        mod2_status_char = strings[0];
        mod3_status_char = strings[2]; // STATUS_LOW
};

serialmessage msg[3*MAX_STRING_LENGTH+1];
strcpy(msg, mod1_status_char); // or use stpcpy if you have it
strcat(msg, mod2_status_char);
strcat(msg, mod3_status_char);

Then send the struct using strlen(msg). msg isn't exactly "dynamic" here, but the length of the string in it varies according to the data, which might be what you want. Or maybe I'm still misunderstanding the role of these three char arrays.

Copying the strings more than necessary just seems to me to introduce complications. Refer to them by pointer until the last possible moment, when your message is assembled, and you minimise the number of places in your code where you have to get buffer sizes right.

Steve Jessop
You may want to allow for the \0 at the end of the string as well."strlen(strings[msg_number]) + 1"strlen does not include the \0 at the end of 'C' strings. so depending on what is receiving the string at the other end it may want the \0 to be sent too. You may also have to consider a way to parse the message at the other end, out of the serial stream. maybe some sort of very low overhead protocol around the strings with the string length embedded in it. Although you can use either the terminating \r\n or the \0 for this purpose.
simon
Yes, I was assuming that the serial protocol uses \r\n as terminator, but if that's not true then the actual terminator (nul or whatever), and its length, needs to be included too. I'm also assuming the serial protocol is inflexible, if it's under the control of the questioner then I'd probably suggest some changes.
Steve Jessop
This is starting to make sense. I don't have control over the protocol at the other end of the link, messages are terminated by <CR><LF>, and no need to send \0. I assume that in the string definition I can remove the \r\n as they are going to beinserted into a larger string.Just one thing about the msg struct, what does [3*MAX_STRING_LENGTH+1] do? Do I define max_string_length globally somewhere, and why multiply by 3?
droseman
Yes, MAX_STRING_LENGTH would be defined globally as the length of the longest single status string (excluding nul terminator). That's why I'm multiplying by 3, but I didn't realise there was anything else in the message. The trouble with the MsgStatus_on struct that you've added, is that (as you guessed) it can only define fixed-length messages, whereas the protocol defines a variable length message. So after the first variable-length component, you'll have to append the remaining parts of the message to the buffer.
Steve Jessop
A: 

You got few options to choose from:

  1. Why you are sending the text if the options are predefined?
    You could send just the ID.
  2. Normally, protocol messages are not fixed length except some rare cases.
    Length first, message second, CRC third.
  3. Fill empty char array space with spaces and trim the string on the other side.
    Not an option, I did not wrote this.
Andrejs Cainikovs
1. The array is filled from the results of some optocouplers on my hardware, e.g portA = 0x30, array shows "okay". This is acheived by a switch case statement to strcpy the strings into the array2. In the customer spec, there are only these three messages permissible
droseman
A: 

By truncating "head" of array.

Suppose you have char words[5] (or 6 -- to hold "\r\n").

So in case of "okay" and "high" you send content of words starting from first element -- words, and in case of low just send content starting from second one: words + 1.

EDIT: of course in this case you should write "low" starting from words[1], not words[0].

Alexander Poluektov
Pointer to an array??? Over the serial to a different CPU?
Andrejs Cainikovs
Thanks, text fixed.
Alexander Poluektov
+1  A: 

Am I missing something here? The send routine should just use strlen() on the string so it only sends the data in the buffer.

serWrite( mod1_status, strlen( mod1_status));
serWrite( "\r\n", 2);
tomlogic
Only if NULL terminators are added to the strings, of course.
bta
+3  A: 

"The status_char arrays are then passed to a struct, which is then sent using a send routine."

Be very careful doing this, depending on how you code it you can get all kinds of junk in there. Remember in C the compiler can pad structs however it pleases.

As a side note, your string buffers are too short to hold a string correctly. With 4 character + CR + LF you need a buffer of 7 characters as you need to store the null terminator '\0'. If you do not do this, do not use any 'str' functions as you are not dealing with proper C strings, all your going to do is create an issue further down the road when someone goes to read this/make a change and finds out after copying a str around your hacking off the null termination (strcopy is copying "low\0" into your buffer, your apparently tossing /r/n onto the end somewhere else for some reason) use memcpy.

Onto a solution:

Why are you copying these string around at all? Why not just send an indication to your send function to tell it what it should send and just have the string statically allocated?

You could create an enum with values for (E_LOW,E_OKAY,E_HIGH), just send in 3 enums to a send function and have it store the actual strings to send as static variables locally. If space is an issue you could use bit flags instead of an enum.

Your send function just needs to copy the string value a byte at a time into the send buffer and send strlen() bytes.

Mark
A: 

I'd like to see your definition of MsgStatus_on.

I'm betting you have something like this:

tyepdef struct {
   char descriptor_msg[6];
   char address01;
   char address02;
   char space01;
   char cmdmsg01[11];
   char statusmsg01[6];
   char space02;
   char cmdmsg02[5];
   char statusmsg02[6];
   char space03;
   char cmdmsg03[5];
   char statusmsg03[6];
   char CR;
   char LF;
} MsgStatus;

MsgStatus MsgStatus_on;

And then my guess is that you're doing a straight byte pointer when you call USARTWrite, like so:

USARTWrite ((char *)&MsgStatus_on, sizeof(MsgStatus_on));

If this is the case then it's copying the extra byte in your buffer. In fact it should be putting extra \0's on all of your char arrays. Unless you declared all of them one less than I did and you're actually overrunning your arrays when you do the strcpy(). It's not causing you problems because you then set the overrun memory in the next statement.

An alternative option could be to use sprintf:

char *message[100] //or some size big enough to hold the whole message.

sprintf (message, "$psu_%d%d op_en op1_%s op2_%s op3_%s\r\n",
   address01, address02, mod1_status_char, mod2_status_char, mod3_status_char);

Then call:

USARTWrite (message, strlen(message));

EDIT: Oops I guess this question is quite old. Oh well, I'll leave the answer in case it's useful to you.

DarthNoodles