0

I'm working with an USART device that send to my MCU a series of different commands (also different is size) and I want to try the best way to parse the commands.

I defined two packed structure (one for each command)

typedef ccport_PACKED( struct TASK_CommandStandard
{
    UINT8  startByte;
    UINT16 length;
    UINT8  command;
    UINT16 crc16;
}) TASK_CommandStandard_t;

typedef ccport_PACKED( struct TASK_CommandExitBootloader
{
    UINT8  startByte;
    UINT16 length;
    UINT8  command;
    UINT8  reserved;
    UINT16 crc16;
}) TASK_CommandExitBootloader_t;

and one Union:

typedef union TASK_Command
{
    TASK_CommandStandard_t       standard;
    TASK_CommandExitBootloader_t exitbootloader;

} TASK_Command_t;

My application receives the USART command inside a UINT8 buffer and after that, looking into the 4th byte I can detect the type of the command (standard or exitbootloader).

To parse the command, my idea is to use one pointer TASK_Command_t *newCommand and based on the command code, assign the address of instance.rxFrameBuffer to:

newCommand->exitbootloader = (TASK_CommandExitBootloader_t *)instance.rxFrameBuffer

or

newCommand->standard = (TASK_CommandStandard_t *)instance.rxFrameBuffer

This is my function:

static void TASK_FSM_FrameReceived( void )
{
    UINT8                   commandCode;
    TASK_Command_t          *newCommand;

    commandCode = instance.rxFrameBuffer[TASK_COMMAND_CODE_INDEX];

    if( commandCode == TASK_COMMAND_CODE_EXIT_BOOTLOADER )
    {
        newCommand->exitbootloader = (TASK_CommandExitBootloader_t *)instance.rxFrameBuffer;
    }
    else
    {
        newCommand->standard = (TASK_CommandStandard_t *)instance.rxFrameBuffer;
    }

    ......
}

Unfortunately, the compiler returns this error:

incompatible types when assigning to type 'TASK_CommandExitBootloader_t' {aka 'struct TASK_CommandExitBootloader'} from type 'TASK_CommandExitBootloader_t *' {aka 'struct TASK_CommandExitBootloader *'}

Can someone give me a hint?

Federico
  • 1,117
  • 6
  • 20
  • 37
  • 1
    Unrelated to your question these structs might give you lots of trouble with padding. `struct` isn't really good at all for serializing raw data. You can beat it into shape with non-standard packing/no padding instructions but that's non-portable. – Lundin May 19 '21 at 08:39
  • I would memcpy received data in a union, and then use one struct or the other to read data. Union are made to access the same data with different var. Also you should use packed attribute with your structs – GabrielT May 19 '21 at 08:41

4 Answers4

0

newCommand->exitbootloader isn't a pointer, it's a struct, thus if you want to copy data of instance.rxFrameBuffer to this struct, you need to use memcpy, or union. You could also dereference like so *((TASK_CommandExitBootloader_t *)instance.rxFrameBuffer) however this may be undefined behaviour depending the type of rxFrameBuffer, so I don't recommend it.

  • My idea is to use a pointer to raw data in order to save space. How it is possible to use the poiter to union to parse my raw data directly from rxFrameBuffer? Instead make a copy. – Federico May 19 '21 at 08:44
  • If you want it to be copy free, in that case you have no choice, but use a pointer. In that case union is meaningless for size as all pointers are the same size, but it's still useful for typing. Make the members of the union pointers instead of structs and you can then cast your `rxFrameBuffer` to them. Since I don't have access to the full context here in code, I can't really advice if this is the best approach however. If you have full control of `rxFrameBuffer` what you could do is have union with the `rxFrameBuffer` and all the other structs, then you can reinterpret the data as you wish. – Jari Vetoniemi May 19 '21 at 08:54
  • @Federico You can't, none of this makes any sense. See my answer. How did you get the data into this rxFrameBuffer to begin with? Is that the hardware registers or something else? Where is re-entrancy handled? And so on. – Lundin May 19 '21 at 08:54
0
  • newCommand is an uninitialized pointer so it can't be used until you point at valid memory somewhere. The code will crash & burn when you attempt newCommand->exitbootloader.

  • You can't just wildly point into an UART rx buffer and merrily be on your way. Where is this data coming from, interrupts or DMA? How do you handle re-entrancy? Where are the actual volatile qualifier registers and how did you get the data from there?

  • Strict aliasing is real and it is nasty, particularly if using gcc. So you can't point into a pre-declared uint8_t array buffer for that reason. You can cast between those two different struct types if the union containing them both is present, but better to avoid all such conversions.

I'd also strongly recommend dropping "my local garage standard" types UINT8 or whatever in favour for internationally standardized, well-known C standard types uint8_t etc from stdint.h.

Also regarding hard copy of raw UART buffers on low-end microcontroller systems, that's a very common beginner mistake. See this answer for an example of how to do it properly.

Lundin
  • 195,001
  • 40
  • 254
  • 396
  • I have a state machine used to handle USART RX. One state in used to polling a UART driver, that return a single byte. The bytes are put into the rxBuffer (static buffer into FSM module) until they reach the end of the commands. Next FSM is used to parse the command and here I wanto to use union. My MCU runs at 144 Mhz so my idea to use a pointer to a union instead another buffer is to try to find best approach. – Federico May 19 '21 at 08:59
  • @Federico State machines are higher layer concepts. How does the data make it from the UART hardware buffers into your variables? – Lundin May 19 '21 at 09:00
  • The UART driver uses a Queue. Every time UART receives a new char, the interrupt is fired and the new char in pushd into a Queue. On hige level I call a function that pops n character from the Queue. – Federico May 19 '21 at 09:07
  • @Federico Ok so the queue handles re-entrancy? And this buffer here is some go-between? Why don't you place data directly into the struct from that queue then? – Lundin May 19 '21 at 10:36
0

I had to do something similar (pointer to raw data to save memory)

I did something like this :

typedef struct TASK_CommandStandard
{
    volatile UINT8  startByte;
    volatile UINT16 length;
    volatile UINT8  command;
    volatile UINT16 crc16;
} TASK_CommandStandard_t;

typedef struct TASK_CommandExitBootloader
{
    volatile UINT8  startByte;
    volatile UINT16 length;
    volatile UINT8  command;
    volatile UINT8  reserved;
    volatile UINT16 crc16;
} TASK_CommandExitBootloader_t;

#define USART_Foo_Address   0x0F00000
#define USART_cmd   ((TASK_CommandStandard_t*) USART_Foo_Address)
#define USART_exit  ((TASK_CommandExitBootloader_t*) USART_Foo_Address)

Don't forget to check padding/alignment, it was working fine on my MCU

You use it like this :

static void TASK_FSM_FrameReceived( void )
{
    if( instance.rxFrameBuffer[TASK_COMMAND_CODE_INDEX] == TASK_COMMAND_CODE_EXIT_BOOTLOADER )
    {
        USART_exit->length;
        ....
    }
    else
    {
        USART_cmd->length;
        ....
    }
    ......
}
GabrielT
  • 397
  • 6
  • 17
0

I solved modifing the union:

typedef union TASK_Command
{
    TASK_CommandStandard_t       *standard;
    TASK_CommandExitBootloader_t *exitbootloader;

} TASK_Command_t;

and in my function insted using a

TASK_Command_t *newCommand; 

I used

TASK_Command_t newCommand;

In this way I can use the same variable to cast my different messages without make any buffer copy.

I can access to the UART buffer with

 newCommand.standard = (TASK_CommandStandard_t *)instance.rxFrameBuffer;
Federico
  • 1,117
  • 6
  • 20
  • 37