Skip to content

Commit 50c1746

Browse files
committed
pbdrv/usb_stm32: Implement event subscription
Implement the Pybricks Profile event subscription flag. This saves us from trying to send messages to the host when there is no app running to receive them. Previously, trying to write to stdout would block forever because of this. The 5 millisecond timeout is arbitrary. Wireshark shows that the OUT endpoint is being serviced in less than 1 ms, so this should be plenty of time even if some hosts are an order of magnitude slower. We don't want to make it too big because it will delay user programs for this long and we don't want to mess up someone's control loop too badly.
1 parent 40724db commit 50c1746

File tree

1 file changed

+60
-23
lines changed

1 file changed

+60
-23
lines changed

lib/pbio/drv/usb/usb_stm32.c

Lines changed: 60 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ static volatile uint32_t usb_response_sz;
3939
static volatile uint32_t usb_status_sz;
4040
static volatile uint32_t usb_stdout_sz;
4141
static volatile bool transmitting;
42+
static volatile bool pbdrv_usb_stm32_is_events_subscribed;
4243

4344
static USBD_HandleTypeDef husbd;
4445
static PCD_HandleTypeDef hpcd;
@@ -156,12 +157,14 @@ pbio_error_t pbdrv_usb_stdout_tx(const uint8_t *data, uint32_t *size) {
156157
return PBIO_ERROR_NOT_IMPLEMENTED;
157158
#endif
158159

160+
if (!pbdrv_usb_stm32_is_events_subscribed) {
161+
// If the app hasn't subscribed to events, we can't send stdout.
162+
return PBIO_ERROR_INVALID_OP;
163+
}
164+
159165
uint8_t *ptr = usb_stdout_buf;
160166
uint32_t ptr_len = sizeof(usb_stdout_buf);
161167

162-
// TODO: return PBIO_ERROR_INVALID_OP if app flag is not set. Also need a
163-
// timeout in case the app crashes and doesn't clear the flag on exit.
164-
165168
if (usb_stdout_sz) {
166169
*size = 0;
167170
return PBIO_ERROR_AGAIN;
@@ -184,7 +187,9 @@ pbio_error_t pbdrv_usb_stdout_tx(const uint8_t *data, uint32_t *size) {
184187
}
185188

186189
uint32_t pbdrv_usb_stdout_tx_available(void) {
187-
// TODO: return UINT32_MAX if app flag is not set.
190+
if (!pbdrv_usb_stm32_is_events_subscribed) {
191+
return UINT32_MAX;
192+
}
188193

189194
if (usb_stdout_sz) {
190195
return 0;
@@ -201,6 +206,14 @@ bool pbdrv_usb_stdout_tx_is_idle(void) {
201206
return usb_stdout_sz == 0;
202207
}
203208

209+
static void pbdrv_usb_stm32_reset_tx_state(void) {
210+
usb_response_sz = 0;
211+
usb_status_sz = 0;
212+
usb_stdout_sz = 0;
213+
transmitting = false;
214+
pbdrv_usb_stm32_is_events_subscribed = false;
215+
}
216+
204217
/**
205218
* @brief Pybricks_Itf_Init
206219
* Initializes the Pybricks media low layer
@@ -210,10 +223,7 @@ bool pbdrv_usb_stdout_tx_is_idle(void) {
210223
static USBD_StatusTypeDef Pybricks_Itf_Init(void) {
211224
USBD_Pybricks_SetRxBuffer(&husbd, usb_in_buf);
212225
usb_in_sz = 0;
213-
usb_response_sz = 0;
214-
usb_status_sz = 0;
215-
usb_stdout_sz = 0;
216-
transmitting = false;
226+
pbdrv_usb_stm32_reset_tx_state();
217227

218228
return USBD_OK;
219229
}
@@ -225,6 +235,7 @@ static USBD_StatusTypeDef Pybricks_Itf_Init(void) {
225235
* @retval Result of the operation: USBD_OK if all operations are OK else USBD_FAIL
226236
*/
227237
static USBD_StatusTypeDef Pybricks_Itf_DeInit(void) {
238+
pbdrv_usb_stm32_reset_tx_state();
228239
return USBD_OK;
229240
}
230241

@@ -316,7 +327,8 @@ PROCESS_THREAD(pbdrv_usb_process, ev, data) {
316327
static PBIO_ONESHOT(pwrdn_oneshot);
317328
static bool bcd_busy;
318329
static pbio_pybricks_error_t result;
319-
static struct etimer timer;
330+
static struct etimer status_timer;
331+
static struct etimer transmit_timer;
320332
static uint32_t prev_status_flags = ~0;
321333
static uint32_t new_status_flags;
322334

@@ -337,7 +349,7 @@ PROCESS_THREAD(pbdrv_usb_process, ev, data) {
337349

338350
PROCESS_BEGIN();
339351

340-
etimer_set(&timer, 500);
352+
etimer_set(&status_timer, 500);
341353

342354
for (;;) {
343355
PROCESS_WAIT_EVENT();
@@ -369,6 +381,12 @@ PROCESS_THREAD(pbdrv_usb_process, ev, data) {
369381

370382
if (usb_in_sz) {
371383
switch (usb_in_buf[0]) {
384+
case USBD_PYBRICKS_OUT_EP_MSG_SUBSCRIBE:
385+
pbdrv_usb_stm32_is_events_subscribed = usb_in_buf[1];
386+
usb_response_buf[0] = USBD_PYBRICKS_IN_EP_MSG_RESPONSE;
387+
pbio_set_uint32_le(&usb_response_buf[1], PBIO_PYBRICKS_ERROR_OK);
388+
usb_response_sz = sizeof(usb_response_buf);
389+
break;
372390
case USBD_PYBRICKS_OUT_EP_MSG_COMMAND:
373391
if (usb_response_sz == 0) {
374392
result = pbsys_command(usb_in_buf + 1, usb_in_sz - 1);
@@ -385,6 +403,13 @@ PROCESS_THREAD(pbdrv_usb_process, ev, data) {
385403
}
386404

387405
if (transmitting) {
406+
if (etimer_expired(&transmit_timer)) {
407+
// Transmission has taken too long, so reset the state to allow
408+
// new transmissions. This can happen if the host stops reading
409+
// data for some reason.
410+
pbdrv_usb_stm32_reset_tx_state();
411+
}
412+
388413
continue;
389414
}
390415

@@ -394,20 +419,32 @@ PROCESS_THREAD(pbdrv_usb_process, ev, data) {
394419
if (usb_response_sz) {
395420
transmitting = true;
396421
USBD_Pybricks_TransmitPacket(&husbd, usb_response_buf, usb_response_sz);
397-
} else if ((new_status_flags != prev_status_flags) || etimer_expired(&timer)) {
398-
usb_status_buf[0] = USBD_PYBRICKS_IN_EP_MSG_EVENT;
399-
_Static_assert(sizeof(usb_status_buf) + 1 >= PBIO_PYBRICKS_EVENT_STATUS_REPORT_SIZE,
400-
"size of status report does not match size of event");
401-
usb_status_sz = 1 + pbsys_status_get_status_report(&usb_status_buf[1]);
402-
403-
etimer_restart(&timer);
404-
prev_status_flags = new_status_flags;
422+
} else if (pbdrv_usb_stm32_is_events_subscribed) {
423+
if ((new_status_flags != prev_status_flags) || etimer_expired(&status_timer)) {
424+
usb_status_buf[0] = USBD_PYBRICKS_IN_EP_MSG_EVENT;
425+
_Static_assert(sizeof(usb_status_buf) + 1 >= PBIO_PYBRICKS_EVENT_STATUS_REPORT_SIZE,
426+
"size of status report does not match size of event");
427+
usb_status_sz = 1 + pbsys_status_get_status_report(&usb_status_buf[1]);
428+
429+
// REVISIT: we really shouldn't need a status timer on USB since
430+
// it's not a lossy transport. We just need to make sure we send
431+
// status updates when they change and send the current status
432+
// immediately after subscribing to events.
433+
etimer_restart(&status_timer);
434+
prev_status_flags = new_status_flags;
435+
436+
transmitting = true;
437+
USBD_Pybricks_TransmitPacket(&husbd, usb_status_buf, usb_status_sz);
438+
} else if (usb_stdout_sz) {
439+
transmitting = true;
440+
USBD_Pybricks_TransmitPacket(&husbd, usb_stdout_buf, usb_stdout_sz);
441+
}
442+
}
405443

406-
transmitting = true;
407-
USBD_Pybricks_TransmitPacket(&husbd, usb_status_buf, usb_status_sz);
408-
} else if (usb_stdout_sz) {
409-
transmitting = true;
410-
USBD_Pybricks_TransmitPacket(&husbd, usb_stdout_buf, usb_stdout_sz);
444+
if (transmitting) {
445+
// If the FIFO isn't emptied quickly, then there probably isn't an
446+
// app anymore. This timer is used to detect such a condition.
447+
etimer_set(&transmit_timer, 5);
411448
}
412449
}
413450

0 commit comments

Comments
 (0)