Skip to content

Commit 1a888fe

Browse files
benpiccobenpicco
committed
drivers/slipdev: make use of chunked ringbuffer
1 parent 73bde97 commit 1a888fe

File tree

6 files changed

+114
-133
lines changed

6 files changed

+114
-133
lines changed

‎drivers/include/slipdev.h‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@
4141
#include "cib.h"
4242
#include "net/netdev.h"
4343
#include "periph/uart.h"
44-
#include "tsrb.h"
44+
#include "chunked_ringbuffer.h"
4545

4646
#ifdef __cplusplus
4747
extern "C" {
@@ -57,8 +57,6 @@ extern "C" {
5757
*
5858
* Reduce this value if your expected traffic does not include full IPv6 MTU
5959
* sized packets.
60-
*
61-
* @pre Needs to be power of two and `<= INT_MAX`
6260
*/
6361
#ifdef CONFIG_SLIPDEV_BUFSIZE_EXP
6462
#define CONFIG_SLIPDEV_BUFSIZE (1<<CONFIG_SLIPDEV_BUFSIZE_EXP)
@@ -83,10 +81,18 @@ enum {
8381
* @brief Device writes handles data as network device
8482
*/
8583
SLIPDEV_STATE_NET,
84+
/**
85+
* @brief Device writes handles data as network device, next byte is escaped
86+
*/
87+
SLIPDEV_STATE_NET_ESC,
8688
/**
8789
* @brief Device writes received data to stdin
8890
*/
8991
SLIPDEV_STATE_STDIN,
92+
/**
93+
* @brief Device writes received data to stdin, next byte is escaped
94+
*/
95+
SLIPDEV_STATE_STDIN_ESC,
9096
/**
9197
* @brief Device is in standby, will wake up when sending data
9298
*/
@@ -114,15 +120,18 @@ typedef struct {
114120
typedef struct {
115121
netdev_t netdev; /**< parent class */
116122
slipdev_params_t config; /**< configuration parameters */
117-
tsrb_t inbuf; /**< RX buffer */
123+
chunk_ringbuf_t rb; /**< Ringbuffer to store received frames. */
124+
/* Written to from interrupts (with irq_disable */
125+
/* to prevent any simultaneous writes), */
126+
/* consumed exclusively in the network stack's */
127+
/* loop at _isr. */
128+
118129
uint8_t rxmem[CONFIG_SLIPDEV_BUFSIZE]; /**< memory used by RX buffer */
119130
/**
120131
* @brief Device state
121132
* @see [Device state definitions](@ref drivers_slipdev_states)
122133
*/
123134
uint8_t state;
124-
uint8_t rx_queued; /**< pkt queued in inbuf */
125-
uint8_t rx_done; /**< pkt processed */
126135
} slipdev_t;
127136

128137
/**

‎drivers/slipdev/Kconfig‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ menuconfig MODULE_SLIPDEV
1111
depends on HAS_PERIPH_UART
1212
depends on TEST_KCONFIG
1313
select MODULE_NETDEV_LEGACY_API
14+
select MODULE_CHUNKED_RINGBUFFER
1415
select MODULE_PERIPH_UART
15-
select MODULE_TSRB
1616

1717
menuconfig KCONFIG_USEMODULE_SLIPDEV
1818
bool "Configure SLIPDEV driver"

‎drivers/slipdev/Makefile.dep‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1+
USEMODULE += chunked_ringbuffer
12
USEMODULE += eui_provider
23
USEMODULE += netdev_legacy_api
34
USEMODULE += netdev_register
4-
USEMODULE += tsrb
55
FEATURES_REQUIRED += periph_uart
66

77
ifneq (,$(filter slipdev_stdio,$(USEMODULE)))

‎drivers/slipdev/include/slipdev_internal.h‎

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -79,23 +79,6 @@ static inline void slipdev_write_byte(uart_t uart, uint8_t byte)
7979
*/
8080
void slipdev_write_bytes(uart_t uart, const uint8_t *data, size_t len);
8181

82-
/**
83-
* @brief Unstuffs a (SLIP-escaped) byte.
84-
*
85-
* @param[out] buf The buffer to write to. It must at least be able to
86-
* receive 1 byte.
87-
* @param[in] byte The byte to unstuff.
88-
* @param[in,out] escaped When set to `false` on in, @p byte will be read as
89-
* though it was not escaped, when set to `true` it
90-
* will be read as though it was escaped. On out it
91-
* will be `false` unless @p byte was `SLIPDEV_ESC`.
92-
*
93-
* @return 0, when @p byte did not resolve to an actual byte
94-
* @return 1, when @p byte resolves to an actual byte (or @p escaped was set to
95-
* true on in and resolves to a byte that was previously escaped).
96-
*/
97-
unsigned slipdev_unstuff_readbyte(uint8_t *buf, uint8_t byte, bool *escaped);
98-
9982
#ifdef __cplusplus
10083
}
10184
#endif

‎drivers/slipdev/slipdev.c‎

Lines changed: 87 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
*
1212
* @file
1313
* @author Martine Lenders <[email protected]>
14+
* @author Benjamin Valentin <[email protected]>
1415
*/
1516

1617
#include <assert.h>
@@ -52,27 +53,89 @@ static void _slip_rx_cb(void *arg, uint8_t byte)
5253
{
5354
slipdev_t *dev = arg;
5455

55-
if (IS_USED(MODULE_SLIPDEV_STDIO)) {
56-
if (dev->state == SLIPDEV_STATE_STDIN) {
56+
switch (dev->state) {
57+
#if IS_USED(MODULE_SLIPDEV_STDIO)
58+
case SLIPDEV_STATE_STDIN:
59+
switch (byte) {
60+
case SLIPDEV_ESC:
61+
dev->state = SLIPDEV_STATE_STDIN_ESC;
62+
break;
63+
case SLIPDEV_END:
64+
dev->state = SLIPDEV_STATE_NONE;
65+
byte = 0;
66+
/* fall-through */
67+
default:
5768
isrpipe_write_one(&slipdev_stdio_isrpipe, byte);
58-
goto check_end;
69+
break;
5970
}
60-
else if ((byte == SLIPDEV_STDIO_START) &&
61-
(dev->config.uart == STDIO_UART_DEV) &&
62-
(dev->state == SLIPDEV_STATE_NONE)) {
71+
return;
72+
case SLIPDEV_STATE_STDIN_ESC:
73+
switch (byte) {
74+
case SLIPDEV_END_ESC:
75+
byte = SLIPDEV_END;
76+
break;
77+
case SLIPDEV_ESC_ESC:
78+
byte = SLIPDEV_ESC;
79+
break;
80+
}
81+
dev->state = SLIPDEV_STATE_STDIN;
82+
isrpipe_write_one(&slipdev_stdio_isrpipe, byte);
83+
return;
84+
#endif
85+
case SLIPDEV_STATE_NONE:
86+
/* is diagnostic frame? */
87+
if (IS_USED(MODULE_SLIPDEV_STDIO) &&
88+
(byte == SLIPDEV_STDIO_START) &&
89+
(dev->config.uart == STDIO_UART_DEV)) {
6390
dev->state = SLIPDEV_STATE_STDIN;
6491
return;
6592
}
66-
}
67-
dev->state = SLIPDEV_STATE_NET;
68-
tsrb_add_one(&dev->inbuf, byte);
69-
check_end:
70-
if (byte == SLIPDEV_END) {
71-
if (dev->state == SLIPDEV_STATE_NET) {
72-
dev->rx_queued++;
93+
94+
/* ignore empty frame */
95+
if (byte == SLIPDEV_END) {
96+
return;
97+
}
98+
99+
/* try to create new frame */
100+
if (!crb_start_chunk(&dev->rb)) {
101+
return;
102+
}
103+
dev->state = SLIPDEV_STATE_NET;
104+
/* fall-through */
105+
case SLIPDEV_STATE_NET:
106+
switch (byte) {
107+
case SLIPDEV_ESC:
108+
dev->state = SLIPDEV_STATE_NET_ESC;
109+
return;
110+
case SLIPDEV_END:
111+
crb_end_chunk(&dev->rb, true);
73112
netdev_trigger_event_isr(&dev->netdev);
113+
dev->state = SLIPDEV_STATE_NONE;
114+
return;
74115
}
116+
break;
117+
/* escaped byte received */
118+
case SLIPDEV_STATE_NET_ESC:
119+
switch (byte) {
120+
case SLIPDEV_END_ESC:
121+
byte = SLIPDEV_END;
122+
break;
123+
case SLIPDEV_ESC_ESC:
124+
byte = SLIPDEV_ESC;
125+
break;
126+
}
127+
dev->state = SLIPDEV_STATE_NET;
128+
break;
129+
}
130+
131+
assert(dev->state == SLIPDEV_STATE_NET);
132+
133+
/* discard frame if byte can't be added */
134+
if (!crb_add_byte(&dev->rb, byte)) {
135+
DEBUG("slipdev: rx buffer full, drop frame\n");
136+
crb_end_chunk(&dev->rb, false);
75137
dev->state = SLIPDEV_STATE_NONE;
138+
return;
76139
}
77140
}
78141

@@ -100,7 +163,7 @@ static int _init(netdev_t *netdev)
100163
DEBUG("slipdev: initializing device %p on UART %i with baudrate %" PRIu32 "\n",
101164
(void *)dev, dev->config.uart, dev->config.baudrate);
102165
/* initialize buffers */
103-
tsrb_init(&dev->inbuf, dev->rxmem, sizeof(dev->rxmem));
166+
crb_init(&dev->rb, dev->rxmem, sizeof(dev->rxmem));
104167
if (uart_init(dev->config.uart, dev->config.baudrate, _slip_rx_cb,
105168
dev) != UART_OK) {
106169
LOG_ERROR("slipdev: error initializing UART %i with baudrate %" PRIu32 "\n",
@@ -134,41 +197,6 @@ void slipdev_write_bytes(uart_t uart, const uint8_t *data, size_t len)
134197
}
135198
}
136199

137-
static unsigned _copy_byte(uint8_t *buf, uint8_t byte, bool *escaped)
138-
{
139-
*buf = byte;
140-
*escaped = false;
141-
return 1U;
142-
}
143-
144-
unsigned slipdev_unstuff_readbyte(uint8_t *buf, uint8_t byte, bool *escaped)
145-
{
146-
unsigned res = 0U;
147-
148-
switch (byte) {
149-
case SLIPDEV_ESC:
150-
*escaped = true;
151-
/* Intentionally falls through */
152-
case SLIPDEV_END:
153-
break;
154-
case SLIPDEV_END_ESC:
155-
if (*escaped) {
156-
return _copy_byte(buf, SLIPDEV_END, escaped);
157-
}
158-
/* Intentionally falls through */
159-
/* to default when !(*escaped) */
160-
case SLIPDEV_ESC_ESC:
161-
if (*escaped) {
162-
return _copy_byte(buf, SLIPDEV_ESC, escaped);
163-
}
164-
/* Intentionally falls through */
165-
/* to default when !(*escaped) */
166-
default:
167-
return _copy_byte(buf, byte, escaped);
168-
}
169-
return res;
170-
}
171-
172200
static int _check_state(slipdev_t *dev)
173201
{
174202
/* power states not supported when multiplexing stdio */
@@ -212,64 +240,33 @@ static int _send(netdev_t *netdev, const iolist_t *iolist)
212240
static int _recv(netdev_t *netdev, void *buf, size_t len, void *info)
213241
{
214242
slipdev_t *dev = (slipdev_t *)netdev;
215-
int res = 0;
243+
size_t res = 0;
216244

217245
(void)info;
218246
if (buf == NULL) {
219247
if (len > 0) {
220248
/* remove data */
221-
for (; len > 0; len--) {
222-
int byte = tsrb_get_one(&dev->inbuf);
223-
if ((byte == (int)SLIPDEV_END) || (byte < 0)) {
224-
/* end early if end of packet or ringbuffer is reached;
225-
* len might be larger than the actual packet */
226-
break;
227-
}
228-
}
249+
crb_consume_chunk(&dev->rb, NULL, len);
229250
} else {
230251
/* the user was warned not to use a buffer size > `INT_MAX` ;-) */
231-
res = (int)tsrb_avail(&dev->inbuf);
252+
crb_get_chunk_size(&dev->rb, &res);
232253
}
233254
}
234255
else {
235-
int byte;
236-
bool escaped = false;
237-
uint8_t *ptr = buf;
238-
239-
do {
240-
int tmp;
241-
242-
if ((byte = tsrb_get_one(&dev->inbuf)) < 0) {
243-
/* something went wrong, return error */
244-
return -EIO;
245-
}
246-
247-
/* frame is larger than expected - lost end marker */
248-
if ((unsigned)res >= len) {
249-
while (byte != SLIPDEV_END) {
250-
/* clear out unreceived packet */
251-
byte = tsrb_get_one(&dev->inbuf);
252-
}
253-
return -ENOBUFS;
254-
}
255-
256-
tmp = slipdev_unstuff_readbyte(ptr, byte, &escaped);
257-
ptr += tmp;
258-
res += tmp;
259-
} while (byte != SLIPDEV_END);
260-
261-
if (++dev->rx_done != dev->rx_queued) {
262-
DEBUG("slipdev: pkt still in queue");
263-
netdev_trigger_event_isr(&dev->netdev);
264-
}
256+
crb_consume_chunk(&dev->rb, buf, len);
257+
res = len;
265258
}
266259
return res;
267260
}
268261

269262
static void _isr(netdev_t *netdev)
270263
{
264+
slipdev_t *dev = (slipdev_t *)netdev;
265+
271266
DEBUG("slipdev: handling ISR event\n");
272-
if (netdev->event_callback != NULL) {
267+
268+
size_t len;
269+
while (crb_get_chunk_size(&dev->rb, &len)) {
273270
DEBUG("slipdev: event handler set, issuing RX_COMPLETE event\n");
274271
netdev->event_callback(netdev, NETDEV_EVENT_RX_COMPLETE);
275272
}

‎drivers/slipdev/stdio.c‎

Lines changed: 10 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -39,35 +39,27 @@ void stdio_init(void)
3939
/* intentionally overwritten in netdev init so we have stdio before
4040
* the network device is initialized is initialized */
4141
uart_init(slipdev_params[0].uart, slipdev_params[0].baudrate,
42-
(uart_rx_cb_t)_isrpipe_write, &slipdev_stdio_isrpipe);
42+
_isrpipe_write, &slipdev_stdio_isrpipe);
4343
}
4444

4545
ssize_t stdio_read(void *buffer, size_t len)
4646
{
4747
uint8_t *ptr = buffer;
48-
bool escaped = false;
49-
uint8_t byte;
5048

51-
do {
52-
int read = isrpipe_read(&slipdev_stdio_isrpipe, &byte, 1);
53-
int tmp;
49+
while (len) {
50+
int read = isrpipe_read(&slipdev_stdio_isrpipe, ptr, 1);
5451

5552
if (read == 0) {
5653
continue;
5754
}
58-
else if (len == 0) {
59-
return -ENOBUFS;
60-
}
61-
tmp = slipdev_unstuff_readbyte(ptr, byte, &escaped);
62-
ptr += tmp;
63-
if ((unsigned)(ptr - (uint8_t *)buffer) > len) {
64-
while (byte != SLIPDEV_END) {
65-
/* clear out unreceived packet */
66-
isrpipe_read(&slipdev_stdio_isrpipe, &byte, 1);
67-
}
68-
return -ENOBUFS;
55+
56+
if (*ptr == 0) {
57+
break;
6958
}
70-
} while (byte != SLIPDEV_END);
59+
60+
++ptr;
61+
--len;
62+
}
7163
return ptr - (uint8_t *)buffer;
7264
}
7365

0 commit comments

Comments
 (0)