Skip to content

Commit b412ac3

Browse files
committed
Moved packet handler state to caller and updated unit test for packet handler
1 parent 25f4967 commit b412ac3

File tree

10 files changed

+90
-142
lines changed

10 files changed

+90
-142
lines changed

applications/app_uartcomm.c

+8-8
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,6 @@
2828

2929
// Settings
3030
#define BAUDRATE 115200
31-
#define PACKET_HANDLER 1
32-
#define PACKET_HANDLER_P 2
3331

3432
// Threads
3533
static THD_FUNCTION(packet_process_thread, arg);
@@ -40,6 +38,7 @@ static volatile bool thread_is_running = false;
4038
static volatile bool uart_is_running = false;
4139
static mutex_t send_mutex;
4240
static bool send_mutex_init_done = false;
41+
static PACKET_STATE_t packet_state_external;
4342

4443
#ifdef HW_UART_P_DEV
4544
static mutex_t send_mutex_p;
@@ -53,6 +52,7 @@ static void send_packet(unsigned char *data, unsigned int len);
5352
#ifdef HW_UART_P_DEV
5453
static void process_packet_p(unsigned char *data, unsigned int len);
5554
static void send_packet_p(unsigned char *data, unsigned int len);
55+
static PACKET_STATE_t packet_state_permanent;
5656
#endif
5757

5858
static SerialConfig uart_cfg = {
@@ -102,7 +102,7 @@ static void send_packet_p(unsigned char *data, unsigned int len) {
102102
#endif
103103

104104
void app_uartcomm_start(void) {
105-
packet_init(send_packet, process_packet, PACKET_HANDLER);
105+
packet_init(send_packet, process_packet, &packet_state_external);
106106

107107
if (!thread_is_running) {
108108
chThdCreateStatic(packet_process_thread_wa, sizeof(packet_process_thread_wa),
@@ -123,7 +123,7 @@ void app_uartcomm_start(void) {
123123

124124
void app_uartcomm_start_permanent(void) {
125125
#ifdef HW_UART_P_DEV
126-
packet_init(send_packet_p, process_packet_p, PACKET_HANDLER_P);
126+
packet_init(send_packet_p, process_packet_p, &packet_state_permanent);
127127

128128
if (!thread_is_running) {
129129
chThdCreateStatic(packet_process_thread_wa, sizeof(packet_process_thread_wa),
@@ -166,7 +166,7 @@ void app_uartcomm_send_packet(unsigned char *data, unsigned int len) {
166166
}
167167

168168
chMtxLock(&send_mutex);
169-
packet_send_packet(data, len, PACKET_HANDLER);
169+
packet_send_packet(data, len, &packet_state_external);
170170
chMtxUnlock(&send_mutex);
171171
}
172172

@@ -178,7 +178,7 @@ void app_uartcomm_send_packet_p(unsigned char *data, unsigned int len) {
178178
}
179179

180180
chMtxLock(&send_mutex_p);
181-
packet_send_packet(data, len, PACKET_HANDLER_P);
181+
packet_send_packet(data, len, &packet_state_permanent);
182182
chMtxUnlock(&send_mutex_p);
183183
#else
184184
(void)data;
@@ -236,7 +236,7 @@ static THD_FUNCTION(packet_process_thread, arg) {
236236
#ifdef HW_UART_P_DEV
237237
from_p_uart = false;
238238
#endif
239-
packet_process_byte(res, PACKET_HANDLER);
239+
packet_process_byte(res, &packet_state_external);
240240
rx = true;
241241
}
242242
}
@@ -245,7 +245,7 @@ static THD_FUNCTION(packet_process_thread, arg) {
245245
msg_t res = sdGetTimeout(&HW_UART_P_DEV, TIME_IMMEDIATE);
246246
if (res != MSG_TIMEOUT) {
247247
from_p_uart = true;
248-
packet_process_byte(res, PACKET_HANDLER_P);
248+
packet_process_byte(res, &packet_state_permanent);
249249
rx = true;
250250
}
251251
#endif

comm_usb.c

+4-6
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,6 @@
2424
#include "comm_usb_serial.h"
2525
#include "commands.h"
2626

27-
// Settings
28-
#define PACKET_HANDLER 0
29-
3027
// Private variables
3128
#define SERIAL_RX_BUFFER_SIZE 2048
3229
static uint8_t serial_rx_buffer[SERIAL_RX_BUFFER_SIZE];
@@ -38,6 +35,7 @@ static mutex_t send_mutex;
3835
static thread_t *process_tp;
3936
static volatile unsigned int write_timeout_cnt = 0;
4037
static volatile bool was_timeout = false;
38+
static PACKET_STATE_t packet_state;
4139

4240
// Private functions
4341
static void process_packet(unsigned char *data, unsigned int len);
@@ -82,7 +80,7 @@ static THD_FUNCTION(serial_process_thread, arg) {
8280
chEvtWaitAny((eventmask_t) 1);
8381

8482
while (serial_rx_read_pos != serial_rx_write_pos) {
85-
packet_process_byte(serial_rx_buffer[serial_rx_read_pos++], PACKET_HANDLER);
83+
packet_process_byte(serial_rx_buffer[serial_rx_read_pos++], &packet_state);
8684

8785
if (serial_rx_read_pos == SERIAL_RX_BUFFER_SIZE) {
8886
serial_rx_read_pos = 0;
@@ -118,7 +116,7 @@ static void send_packet_raw(unsigned char *buffer, unsigned int len) {
118116

119117
void comm_usb_init(void) {
120118
comm_usb_serial_init();
121-
packet_init(send_packet_raw, process_packet, PACKET_HANDLER);
119+
packet_init(send_packet_raw, process_packet, &packet_state);
122120

123121
chMtxObjectInit(&send_mutex);
124122

@@ -129,7 +127,7 @@ void comm_usb_init(void) {
129127

130128
void comm_usb_send_packet(unsigned char *data, unsigned int len) {
131129
chMtxLock(&send_mutex);
132-
packet_send_packet(data, len, PACKET_HANDLER);
130+
packet_send_packet(data, len, &packet_state);
133131
chMtxUnlock(&send_mutex);
134132
}
135133

crc.c

+4
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,9 @@
1818
*/
1919

2020
#include "crc.h"
21+
#ifndef NO_STM32
2122
#include "stm32f4xx.h"
23+
#endif
2224

2325
// CRC Table
2426
const unsigned short crc16_tab[] = { 0x0000, 0x1021, 0x2042, 0x3063, 0x4084,
@@ -60,6 +62,7 @@ unsigned short crc16(unsigned char *buf, unsigned int len) {
6062
return cksum;
6163
}
6264

65+
#ifndef NO_STM32
6366
/**
6467
* @brief Computes the 32-bit CRC of a given buffer of data word(32-bit) using
6568
* Hardware Acceleration.
@@ -86,3 +89,4 @@ void crc32_reset(void) {
8689
/* Reset CRC generator */
8790
CRC->CR |= CRC_CR_RESET;
8891
}
92+
#endif

main.c

-14
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,6 @@
7777

7878
// Private variables
7979
static THD_WORKING_AREA(periodic_thread_wa, 1024);
80-
static THD_WORKING_AREA(timer_thread_wa, 128);
8180
static THD_WORKING_AREA(flash_integrity_check_thread_wa, 256);
8281

8382
static THD_FUNCTION(flash_integrity_check_thread, arg) {
@@ -179,18 +178,6 @@ static THD_FUNCTION(periodic_thread, arg) {
179178
}
180179
}
181180

182-
static THD_FUNCTION(timer_thread, arg) {
183-
(void)arg;
184-
185-
chRegSetThreadName("msec_timer");
186-
187-
for(;;) {
188-
packet_timerfunc();
189-
timeout_feed_WDT(THREAD_TIMER);
190-
chThdSleepMilliseconds(1);
191-
}
192-
}
193-
194181
// When assertions enabled halve PWM frequency. The control loop ISR runs 40% slower
195182
void assert_failed(uint8_t* file, uint32_t line) {
196183
commands_printf("Wrong parameters value: file %s on line %d\r\n", file, line);
@@ -285,7 +272,6 @@ int main(void) {
285272

286273
// Threads
287274
chThdCreateStatic(periodic_thread_wa, sizeof(periodic_thread_wa), NORMALPRIO, periodic_thread, NULL);
288-
chThdCreateStatic(timer_thread_wa, sizeof(timer_thread_wa), NORMALPRIO, timer_thread, NULL);
289275
chThdCreateStatic(flash_integrity_check_thread_wa, sizeof(flash_integrity_check_thread_wa), LOWPRIO, flash_integrity_check_thread, NULL);
290276

291277
#if WS2811_TEST

packet.c

+46-88
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
Copyright 2016 - 2019 Benjamin Vedder [email protected]
2+
Copyright 2016 - 2021 Benjamin Vedder [email protected]
33
44
This file is part of the VESC firmware.
55
@@ -21,135 +21,93 @@
2121
#include "packet.h"
2222
#include "crc.h"
2323

24-
/**
25-
* The latest update aims at achieving optimal re-synchronization in the
26-
* case if lost data, at the cost of some performance.
27-
*/
28-
29-
// Defines
30-
#define BUFFER_LEN (PACKET_MAX_PL_LEN + 8)
31-
32-
// Private types
33-
typedef struct {
34-
volatile unsigned short rx_timeout;
35-
void(*send_func)(unsigned char *data, unsigned int len);
36-
void(*process_func)(unsigned char *data, unsigned int len);
37-
unsigned int rx_read_ptr;
38-
unsigned int rx_write_ptr;
39-
int bytes_left;
40-
unsigned char rx_buffer[BUFFER_LEN];
41-
unsigned char tx_buffer[BUFFER_LEN];
42-
} PACKET_STATE_t;
43-
44-
// Private variables
45-
static PACKET_STATE_t m_handler_states[PACKET_HANDLERS];
46-
4724
// Private functions
4825
static int try_decode_packet(unsigned char *buffer, unsigned int in_len,
4926
void(*process_func)(unsigned char *data, unsigned int len), int *bytes_left);
5027

5128
void packet_init(void (*s_func)(unsigned char *data, unsigned int len),
52-
void (*p_func)(unsigned char *data, unsigned int len), int handler_num) {
53-
memset(&m_handler_states[handler_num], 0, sizeof(PACKET_STATE_t));
54-
m_handler_states[handler_num].send_func = s_func;
55-
m_handler_states[handler_num].process_func = p_func;
29+
void (*p_func)(unsigned char *data, unsigned int len), PACKET_STATE_t *state) {
30+
memset(state, 0, sizeof(PACKET_STATE_t));
31+
state->send_func = s_func;
32+
state->process_func = p_func;
5633
}
5734

58-
void packet_reset(int handler_num) {
59-
m_handler_states[handler_num].rx_read_ptr = 0;
60-
m_handler_states[handler_num].rx_write_ptr = 0;
61-
m_handler_states[handler_num].bytes_left = 0;
35+
void packet_reset(PACKET_STATE_t *state) {
36+
state->rx_read_ptr = 0;
37+
state->rx_write_ptr = 0;
38+
state->bytes_left = 0;
6239
}
6340

64-
void packet_send_packet(unsigned char *data, unsigned int len, int handler_num) {
41+
void packet_send_packet(unsigned char *data, unsigned int len, PACKET_STATE_t *state) {
6542
if (len == 0 || len > PACKET_MAX_PL_LEN) {
6643
return;
6744
}
6845

6946
int b_ind = 0;
70-
PACKET_STATE_t *handler = &m_handler_states[handler_num];
7147

7248
if (len <= 255) {
73-
handler->tx_buffer[b_ind++] = 2;
74-
handler->tx_buffer[b_ind++] = len;
49+
state->tx_buffer[b_ind++] = 2;
50+
state->tx_buffer[b_ind++] = len;
7551
} else if (len <= 65535) {
76-
handler->tx_buffer[b_ind++] = 3;
77-
handler->tx_buffer[b_ind++] = len >> 8;
78-
handler->tx_buffer[b_ind++] = len & 0xFF;
52+
state->tx_buffer[b_ind++] = 3;
53+
state->tx_buffer[b_ind++] = len >> 8;
54+
state->tx_buffer[b_ind++] = len & 0xFF;
7955
} else {
80-
handler->tx_buffer[b_ind++] = 4;
81-
handler->tx_buffer[b_ind++] = len >> 16;
82-
handler->tx_buffer[b_ind++] = (len >> 8) & 0x0F;
83-
handler->tx_buffer[b_ind++] = len & 0xFF;
56+
state->tx_buffer[b_ind++] = 4;
57+
state->tx_buffer[b_ind++] = len >> 16;
58+
state->tx_buffer[b_ind++] = (len >> 8) & 0x0F;
59+
state->tx_buffer[b_ind++] = len & 0xFF;
8460
}
8561

86-
memcpy(handler->tx_buffer + b_ind, data, len);
62+
memcpy(state->tx_buffer + b_ind, data, len);
8763
b_ind += len;
8864

8965
unsigned short crc = crc16(data, len);
90-
handler->tx_buffer[b_ind++] = (uint8_t)(crc >> 8);
91-
handler->tx_buffer[b_ind++] = (uint8_t)(crc & 0xFF);
92-
handler->tx_buffer[b_ind++] = 3;
93-
94-
if (handler->send_func) {
95-
handler->send_func(handler->tx_buffer, b_ind);
96-
}
97-
}
66+
state->tx_buffer[b_ind++] = (uint8_t)(crc >> 8);
67+
state->tx_buffer[b_ind++] = (uint8_t)(crc & 0xFF);
68+
state->tx_buffer[b_ind++] = 3;
9869

99-
/**
100-
* Call this function every millisecond. This is not strictly necessary
101-
* if the timeout is unimportant.
102-
*/
103-
void packet_timerfunc(void) {
104-
for (int i = 0;i < PACKET_HANDLERS;i++) {
105-
if (m_handler_states[i].rx_timeout) {
106-
m_handler_states[i].rx_timeout--;
107-
} else {
108-
packet_reset(i);
109-
}
70+
if (state->send_func) {
71+
state->send_func(state->tx_buffer, b_ind);
11072
}
11173
}
11274

113-
void packet_process_byte(uint8_t rx_data, int handler_num) {
114-
PACKET_STATE_t *handler = &m_handler_states[handler_num];
115-
116-
handler->rx_timeout = PACKET_RX_TIMEOUT;
117-
118-
unsigned int data_len = handler->rx_write_ptr - handler->rx_read_ptr;
75+
void packet_process_byte(uint8_t rx_data, PACKET_STATE_t *state) {
76+
unsigned int data_len = state->rx_write_ptr - state->rx_read_ptr;
11977

12078
// Out of space (should not happen)
121-
if (data_len >= BUFFER_LEN) {
122-
handler->rx_write_ptr = 0;
123-
handler->rx_read_ptr = 0;
124-
handler->bytes_left = 0;
125-
handler->rx_buffer[handler->rx_write_ptr++] = rx_data;
79+
if (data_len >= PACKET_BUFFER_LEN) {
80+
state->rx_write_ptr = 0;
81+
state->rx_read_ptr = 0;
82+
state->bytes_left = 0;
83+
state->rx_buffer[state->rx_write_ptr++] = rx_data;
12684
return;
12785
}
12886

12987
// Everything has to be aligned, so shift buffer if we are out of space.
13088
// (as opposed to using a circular buffer)
131-
if (handler->rx_write_ptr >= BUFFER_LEN) {
132-
memmove(handler->rx_buffer,
133-
handler->rx_buffer + handler->rx_read_ptr,
89+
if (state->rx_write_ptr >= PACKET_BUFFER_LEN) {
90+
memmove(state->rx_buffer,
91+
state->rx_buffer + state->rx_read_ptr,
13492
data_len);
13593

136-
handler->rx_read_ptr = 0;
137-
handler->rx_write_ptr = data_len;
94+
state->rx_read_ptr = 0;
95+
state->rx_write_ptr = data_len;
13896
}
13997

140-
handler->rx_buffer[handler->rx_write_ptr++] = rx_data;
98+
state->rx_buffer[state->rx_write_ptr++] = rx_data;
14199
data_len++;
142100

143-
if (handler->bytes_left > 1) {
144-
handler->bytes_left--;
101+
if (state->bytes_left > 1) {
102+
state->bytes_left--;
145103
return;
146104
}
147105

148106
// Try decoding the packet at various offsets until it succeeds, or
149107
// until we run out of data.
150108
for (;;) {
151-
int res = try_decode_packet(handler->rx_buffer + handler->rx_read_ptr,
152-
data_len, handler->process_func, &handler->bytes_left);
109+
int res = try_decode_packet(state->rx_buffer + state->rx_read_ptr,
110+
data_len, state->process_func, &state->bytes_left);
153111

154112
// More data is needed
155113
if (res == -2) {
@@ -158,18 +116,18 @@ void packet_process_byte(uint8_t rx_data, int handler_num) {
158116

159117
if (res > 0) {
160118
data_len -= res;
161-
handler->rx_read_ptr += res;
119+
state->rx_read_ptr += res;
162120
} else if (res == -1) {
163121
// Something went wrong. Move pointer forward and try again.
164-
handler->rx_read_ptr++;
122+
state->rx_read_ptr++;
165123
data_len--;
166124
}
167125
}
168126

169127
// Nothing left, move pointers to avoid memmove
170128
if (data_len == 0) {
171-
handler->rx_read_ptr = 0;
172-
handler->rx_write_ptr = 0;
129+
state->rx_read_ptr = 0;
130+
state->rx_write_ptr = 0;
173131
}
174132
}
175133

0 commit comments

Comments
 (0)