Electronics > Microcontrollers
Parsing UART packets / state machines / packed structs
uer166:
I am in need to parse some UART packets in a format I have no control over. The format is something like:
|0x55|0xAA|LENGTH|ADDRESS|0x01|CMD|DATA0....DATAn|CHKSUM|.
The easiest way seems to be a switch/case state machine, to basically go byte by byte per invocation. It's to be called at a rate higher than the incoming bytes of course to not miss anything, or alternatively use the hardware 8-byte FIFO to call this every 8 bytes, reducing ISR rate. After a packet data is read and validated, it is then memcpy-d into a packed struct based on the command byte which defines the data format, therefore automatically getting the right bits in place. Something like:
--- Code: ---/* Data structure declarations */
volatile struct BMS_batstat bms_batstat;
/* UART state machine state for reading/parsing packets */
static uint8_t length = 0;
static uint16_t chksum = 0;
static uint8_t cmdreq = 0;
static uint8_t databuf[255] = {0};
static uint8_t databuf_i = 0;
void BMS_UART_ParseData(void)
{
if(cmdreq == CMDREQ_BATSTAT)
{
memcpy(&bms_batstat, databuf, sizeof(bms_batstat));
}
else
{
// unknown packet, TODO
}
}
void BMS_UART_StateMachine_Rx(uint8_t rx)
{
switch(bms_uart_state)
{
case UART_wait_0x55:
if(rx == 0x55) { bms_uart_state = UART_wait_0xAA; } else { bms_uart_state = UART_wait_0x55; }
break;
case UART_wait_0xAA:
if(rx == 0xAA) { bms_uart_state = UART_read_length; } else { bms_uart_state = UART_wait_0x55; }
break;
case UART_read_length:
length = rx;
databuf_i = 0;
bms_uart_state = UART_wait_0x25;
break;
case UART_wait_0x25:
if(rx == 0x25) { bms_uart_state = UART_wait_0x01; } else { bms_uart_state = UART_wait_0x55; }
break;
case UART_wait_0x01:
if(rx == 0x01) {bms_uart_state = UART_read_cmdreq; } else { bms_uart_state = UART_wait_0x55; }
break;
case UART_read_cmdreq:
cmdreq = rx;
bms_uart_state = UART_read_data;
break;
case UART_read_data:
databuf[databuf_i] = rx;
databuf_i++;
if(databuf_i == (length - 2)) {bms_uart_state = UART_read_checksumLSB;} /* Data length without including extras */
break;
case UART_read_checksumLSB:
chksum = (uint16_t)rx;
bms_uart_state = UART_read_checksumMSB;
break;
case UART_read_checksumMSB:
chksum |= ((uint16_t)rx) << 8;
uint16_t chksum_data = 0;
chksum_data = length + 0x25 + 0x01 + cmdreq; /* Need to add extras to checksum */
/* Data length without including extras */
for(uint8_t i = 0; i < (length - 2); i++)
{
chksum_data += databuf[i];
}
if(chksum == (0xFFFF - chksum_data))
{
BMS_UART_ParseData();
}
bms_uart_state = UART_wait_0x55;
break;
default:
asm("bkpt");
break;
}
}
--- End code ---
I've seen some implementations of such a state machine with function pointers, separating each state into a function, although I didn't think it was particularly readable or safe. Are there more ways to do this in a more obvious or maybe safe manner? The only complication I see here is the need to debug these variables in the foreground in a shell-like environment, and the packed structs are not atomic, therefore I need to unpack/copy all these variable into atomic types such as float or uint32_t so that there is no data consistency issues.
krish2487:
which mcu??
One approach I can think of is to use DMA to copy all the incoming data to a buffer and then invoke the parser once the buffer is full..
Then the parsing can happen at a much slower rate than byte invocation..
I dont know about other MCUs but STM32 have a UART IDLE line detection which is essentially an interrupt that fires if there is no data on the UART line for a clock cycle or two. This is nice because you dont need to know the length of the data you need to store.. just wait for the interrupt to fire, set a flag inside the ISR to trigger the parser and do the parsing in the main loop once you return from the ISR..
Siwastaja:
Some pseudocode (not tested), just to give idea how I have done this in recent times:
--- Code: ---// .h
typedef struct PACK
{
...
} msg1_t;
typedef struct PACK
{
...
} msg_2_t;
typedef union
{
struct PACK
{
uint16_t magic;
uint8_t len;
uint8_t msgid;
union // all payload types
{
msg1_t msg1;
msg2_t msg2;
uint8_t pay_u8[MAX_PAYLOAD_LEN];
};
};
uint8_t whole_u8[MAX_PAYLOAD_LEN+4];
} msg_t;
extern msg_t* latest_msg;
// .c
static msg_t msg_a, msg_b;
msg_t* latest_msg;
static msg_t* accum;
init():
accum = msg_a;
latest_msg = msg_b;
accumulation in UART character ISR:
{
accum.whole_u8[idx++] = c;
update_crc(c);
if(idx > offsetof(accum.len)+sizeof(accu .len) // len field already received
&& idx >= accum.len + HEADER_LENS_WHATEVER) // complete packet received
{
if(processing_running) error(57); // previous processing func did not finish in time
if(!verify_crc())
// ignore message and log error
else
{
SWAP(accum, latest_msg);
processing_running = 1;
NVIC->STIR = PROCESSING_IRQn; // with lower priority than current interrupt handler
}
}
}
static volatile int processing_running;
processing_handler()
{
// access latest_msg as you wish.
// if msg_id==1, access latest_msg->msg1, and so on.
// switch-case is fine, or a table which contains msgids and function pointers for the handlers; loop through the table here and call the correct handler.
// last thing to do:
processing_running = 0;
}
--- End code ---
No memory copying here. Not saying it's usually a big problem, but why do it when it's not needed? You still have the same constraints: memory footprint of two buffers, and needing to get processing the data finished before the end of the next packet.
DavidAlfa:
For normal 115.200baud, interrupt-based routine shouldn't be a problem.
Add some sort of timeout handling to avoid stalling!
--- Code: ---typedef struct{
uint8_t len;
uint8_t addr;
uint8_t cmd;
uint8_t buf[255];
uint16_t chksum;
} uart_data_t;
typedef enum{
uart_magic_55 = 0,
uart_magic_AA = 1,
uart_len = 2,
uart_adr = 3,
uart_flag = 4,
uart_cmd = 5,
uart_data = 6,
uart_chksum = 7,
uart_done = 8,
uart_chksum_bytes = 2,
}uart_states_t;
static volatile uart_states_t uart_state;
volatile uart_data_t uart_data;
// |0x55|0xAA|LENGTH|ADDRESS|0x01|CMD|DATA0....DATAn|CHKSUM|.
void reset_state(void){ // Called by the user after parsing the packet, or by the timeout routine.
uart_state = uart_magic_55;
}
void RX_ISR(void){
static uint16_t count;
uint8_t rx = RX_BF;
reset_timeout(); // Reset the timeout as long as data keeps coming.
switch(uart_state){
case uart_magic_55:
if(rx==0x55){
enable_timeout(); // Enable timeout ISR or whatever
uart_state++;
}
break;
case uart_magic_AA:
if(rx==0xAA){
uart_state++;
}
else{
uart_state = uart_magic_55;
}
break;
case uart_len:
uart_data.len = rx;
uart_state++;
break;
case uart_adr:
uart_data.adr = rx;
uart_state++;
break;
case uart_flag:
if(rx == 1){
uart_state++;
}
else{
uart_state = uart_magic_55;
}
break;
case uart_cmd:
uart_data.cmd = rx;
count = 0;
uart_state++;
break;
case uart_data:
uart_data.buffer[count++] = rx;
if (uart_data.len == count){
count = 0;
uart_data.checksum = 0;
uart_state++;
}
break;
case uart_chksum:
uart_data.checksum |= rx<<(8*count++);
if(count == uart_chksum_bytes){
disable_timeout(); // Disable timeout ISR, received full frame, wait for processing
uart_state++;
}
break;
case uart_done: // Do nothing, wait for processing
default:
break;
}
--- End code ---
Doctorandus_P:
I agree with the above.
You can make a union with a bunch of structures in it (Including a byte array to write the "raw" data from the uart to.
C compilers do keep the data in the same order, but they may add padding if your processor has a 16bit or larger word size.
To be able to use the same header file for both my 8-bit uC and my gazillion bit monster PC I had to add an:
#ifdef __linux__
#pragma pack(push)
#pragma pack(1) // Alignment for my big 64 bit monster.
#endif
// Union definition goes here.
#ifdef __linux__
#pragma pack(pop) // Restore alignment to normal
#endif
to make GCC happy on both platforms. The PC needed the pack to prevent padding, while the compiler for the 8-bitter choked on the pack.
Navigation
[0] Message Index
[#] Next page
Go to full version