From 97df378147b55c60315f4205ce7ca086b1689736 Mon Sep 17 00:00:00 2001 From: Laurens Valk Date: Sun, 1 Dec 2024 19:34:21 +0100 Subject: [PATCH 1/5] pbio/drv/uart: Refactor async read and write. This squashes the following intermediate commits: pbio/drv/uart: Don't broadcast uart events to all processes. Instead poll subscribers, which is just the legodev process for now. This can be expanded to poll the specific uart instead of all of them. This also drops the uart processes, which were not doing anything, saving some code size. pbio/drv/uart: Replace begin/end with single protothread. UART read and write always used begin/end sequentially, each wrapped in a wait for err != PBIO_ERROR_AGAIN. This replaces the begin/end pattern with one awaitable protothread. This makes the code easier to follow and reduces code size. pbio/drv/uart: Set callback per device. Not all devices may be used by the legodev driver. --- lib/pbio/drv/ioport/ioport_debug_uart.c | 2 + lib/pbio/drv/legodev/legodev_pup.c | 24 ++++ lib/pbio/drv/legodev/legodev_pup_uart.c | 89 +++--------- lib/pbio/drv/uart/uart_stm32f0.c | 169 ++++++++-------------- lib/pbio/drv/uart/uart_stm32f4_ll_irq.c | 180 +++++++++++------------- lib/pbio/drv/uart/uart_stm32l4_ll_dma.c | 149 +++++++++----------- lib/pbio/include/pbdrv/uart.h | 28 ++-- lib/pbio/src/processes.h | 4 - lib/pbio/test/src/test_uartdev.c | 56 +++----- 9 files changed, 297 insertions(+), 404 deletions(-) diff --git a/lib/pbio/drv/ioport/ioport_debug_uart.c b/lib/pbio/drv/ioport/ioport_debug_uart.c index 9fd2cacc5..3837c8df1 100644 --- a/lib/pbio/drv/ioport/ioport_debug_uart.c +++ b/lib/pbio/drv/ioport/ioport_debug_uart.c @@ -5,6 +5,8 @@ #if PBDRV_CONFIG_IOPORT_DEBUG_UART +#error "This driver can currently not be used." + #include #include #include diff --git a/lib/pbio/drv/legodev/legodev_pup.c b/lib/pbio/drv/legodev/legodev_pup.c index 3d75fe1de..4abbb7ad3 100644 --- a/lib/pbio/drv/legodev/legodev_pup.c +++ b/lib/pbio/drv/legodev/legodev_pup.c @@ -14,6 +14,7 @@ #include #include +#include #include @@ -107,6 +108,11 @@ static void legodev_pup_enable_uart(const pbdrv_ioport_pup_pins_t *pins) { pbdrv_gpio_out_low(&pins->uart_buf); } +static void legodev_pup_disable_uart(const pbdrv_ioport_pup_pins_t *pins) { + // REVISIT: Move to ioport. + pbdrv_gpio_out_high(&pins->uart_buf); +} + // This is the device connection manager (dcm). It monitors the ID1 and ID2 pins // on the port to see when devices are connected or disconnected. // It is expected for there to be a 2ms delay between calls to this function. @@ -343,6 +349,7 @@ static PT_THREAD(pbdrv_legodev_pup_thread(ext_dev_t * dev)) { while (true) { // Initially assume nothing is connected. + legodev_pup_disable_uart(dev->pins); dev->dcm.connected_type_id = PBDRV_LEGODEV_TYPE_ID_NONE; dev->dcm.dev_id_match_count = 0; @@ -369,6 +376,7 @@ static PT_THREAD(pbdrv_legodev_pup_thread(ext_dev_t * dev)) { // disconnects, as observed by the UART process not getting valid data. legodev_pup_enable_uart(dev->pins); PT_SPAWN(&dev->pt, &dev->child, pbdrv_legodev_pup_uart_thread(&dev->child, dev->uart_dev)); + } PT_END(&dev->pt); } @@ -405,6 +413,14 @@ PROCESS_THREAD(pbio_legodev_pup_process, ev, data) { PROCESS_END(); } +/** + * We get notified when the uart driver has completed sending or receiving data. + */ +static void uart_poll_callback(pbdrv_uart_dev_t *uart) { + // REVISIT: Only need to poll the specified uart device. + process_poll(&pbio_legodev_pup_process); +} + void pbdrv_legodev_init(void) { #if PBDRV_CONFIG_LEGODEV_PUP_NUM_INT_DEV > 0 for (uint8_t i = 0; i < PBDRV_CONFIG_LEGODEV_PUP_NUM_INT_DEV; i++) { @@ -431,7 +447,15 @@ void pbdrv_legodev_init(void) { pbio_dcmotor_get_dcmotor(legodev, &dcmotor); legodev->ext_dev->uart_dev = pbdrv_legodev_pup_uart_configure(legodev_data->ioport_index, port_data->uart_driver_index, dcmotor); + // legodev driver is started after all other drivers, so we + // assume that we do not need to wait for this to be ready. + pbdrv_uart_dev_t *uart; + pbdrv_uart_get(port_data->uart_driver_index, &uart); + + // Set callback for uart driver. + pbdrv_uart_set_poll_callback(uart, uart_poll_callback); } + process_start(&pbio_legodev_pup_process); } diff --git a/lib/pbio/drv/legodev/legodev_pup_uart.c b/lib/pbio/drv/legodev/legodev_pup_uart.c index 3695183bd..c0aebb712 100644 --- a/lib/pbio/drv/legodev/legodev_pup_uart.c +++ b/lib/pbio/drv/legodev/legodev_pup_uart.c @@ -160,6 +160,8 @@ struct _pbdrv_legodev_pup_uart_dev_t { struct pt pt; /** Protothread for receiving sensor data, running in parallel to the data send thread. */ struct pt recv_pt; + /** Child protothread of the main protothread used for reading data */ + struct pt read_pt; /** Child protothread of the main protothread used for writing data */ struct pt write_pt; /** Timer for sending keepalive messages and other delays. */ @@ -199,8 +201,6 @@ struct _pbdrv_legodev_pup_uart_dev_t { uint32_t err_count; /** Number of bad reads when receiving DATA ludev->msgs. */ uint32_t num_data_err; - /** Time of most recently started transmission. */ - uint32_t tx_start_time; /** Flag that indicates that good DATA ludev->msg has been received since last watchdog timeout. */ bool data_rec; /** Return value for synchronization thread. */ @@ -229,8 +229,6 @@ static pbdrv_legodev_pup_uart_dev_t ludevs[PBDRV_CONFIG_LEGODEV_PUP_UART_NUM_DEV static uint8_t data_read_bufs[PBDRV_CONFIG_LEGODEV_PUP_UART_NUM_DEV][PBDRV_LEGODEV_MAX_DATA_SIZE] __attribute__((aligned(4))); static pbdrv_legodev_pup_uart_data_set_t data_set_bufs[PBDRV_CONFIG_LEGODEV_PUP_UART_NUM_DEV]; -#define PBIO_PT_WAIT_READY(pt, expr) PT_WAIT_UNTIL((pt), (expr) != PBIO_ERROR_AGAIN) - pbdrv_legodev_pup_uart_dev_t *pbdrv_legodev_pup_uart_configure(uint8_t device_index, uint8_t uart_driver_index, pbio_dcmotor_t *dcmotor) { pbdrv_legodev_pup_uart_dev_t *ludev = &ludevs[device_index]; ludev->dcmotor = dcmotor; @@ -694,16 +692,6 @@ static void pbdrv_legodev_pup_uart_reset(pbdrv_legodev_pup_uart_dev_t *ludev) { } } -static PT_THREAD(pbdrv_legodev_pup_uart_send_prepared_msg(pbdrv_legodev_pup_uart_dev_t * ludev, pbio_error_t * err)) { - PT_BEGIN(&ludev->write_pt); - ludev->tx_start_time = pbdrv_clock_get_ms(); - PBIO_PT_WAIT_READY(&ludev->write_pt, *err = pbdrv_uart_write_begin(ludev->uart, ludev->tx_msg, ludev->tx_msg_size, EV3_UART_IO_TIMEOUT)); - if (*err == PBIO_SUCCESS) { - PBIO_PT_WAIT_READY(&ludev->write_pt, *err = pbdrv_uart_write_end(ludev->uart)); - } - PT_END(&ludev->write_pt); -} - /** * The synchronization thread for the LEGO UART device. * @@ -734,7 +722,7 @@ static PT_THREAD(pbdrv_legodev_pup_uart_synchronize_thread(pbdrv_legodev_pup_uar pbdrv_uart_flush(ludev->uart); - PT_SPAWN(&ludev->pt, &ludev->write_pt, pbdrv_legodev_pup_uart_send_prepared_msg(ludev, &ludev->err)); + PT_SPAWN(&ludev->pt, &ludev->write_pt, pbdrv_uart_write(&ludev->write_pt, ludev->uart, ludev->tx_msg, ludev->tx_msg_size, EV3_UART_IO_TIMEOUT, &ludev->err)); if (ludev->err != PBIO_SUCCESS) { DBG_ERR(ludev->last_err = "Speed tx failed"); PT_EXIT(&ludev->pt); @@ -743,13 +731,8 @@ static PT_THREAD(pbdrv_legodev_pup_uart_synchronize_thread(pbdrv_legodev_pup_uar pbdrv_uart_flush(ludev->uart); // read one byte to check for ACK - PBIO_PT_WAIT_READY(&ludev->pt, ludev->err = pbdrv_uart_read_begin(ludev->uart, ludev->rx_msg, 1, 10)); - if (ludev->err != PBIO_SUCCESS) { - DBG_ERR(ludev->last_err = "UART Rx error during baud"); - PT_EXIT(&ludev->pt); - } + PT_SPAWN(&ludev->pt, &ludev->read_pt, pbdrv_uart_read(&ludev->read_pt, ludev->uart, ludev->rx_msg, 1, 10, &ludev->err)); - PBIO_PT_WAIT_READY(&ludev->pt, ludev->err = pbdrv_uart_read_end(ludev->uart)); if ((ludev->err == PBIO_SUCCESS && ludev->rx_msg[0] != LUMP_SYS_ACK) || ludev->err == PBIO_ERROR_TIMEDOUT) { // if we did not get ACK within 100ms, then switch to slow baud rate for sync pbdrv_uart_set_baud_rate(ludev->uart, EV3_UART_SPEED_MIN); @@ -758,18 +741,14 @@ static PT_THREAD(pbdrv_legodev_pup_uart_synchronize_thread(pbdrv_legodev_pup_uar DBG_ERR(ludev->last_err = "UART Rx error during baud"); PT_EXIT(&ludev->pt); } - sync: // To get in sync with the data stream from the sensor, we look for a valid TYPE command. for (;;) { - - PBIO_PT_WAIT_READY(&ludev->pt, ludev->err = pbdrv_uart_read_begin(ludev->uart, ludev->rx_msg, 1, EV3_UART_IO_TIMEOUT)); - if (ludev->err != PBIO_SUCCESS) { - DBG_ERR(ludev->last_err = "UART Rx error during sync"); - PT_EXIT(&ludev->pt); - } - PBIO_PT_WAIT_READY(&ludev->pt, ludev->err = pbdrv_uart_read_end(ludev->uart)); + // If there are multiple bytes waiting to be read, this drains them one + // by one without requiring additional polls. This means we won't need + // exact timing to get in sync. + PT_SPAWN(&ludev->pt, &ludev->read_pt, pbdrv_uart_read(&ludev->read_pt, ludev->uart, ludev->rx_msg, 1, EV3_UART_IO_TIMEOUT, &ludev->err)); if (ludev->err == PBIO_ERROR_TIMEDOUT) { continue; } @@ -784,13 +763,7 @@ static PT_THREAD(pbdrv_legodev_pup_uart_synchronize_thread(pbdrv_legodev_pup_uar } // then read the rest of the message - - PBIO_PT_WAIT_READY(&ludev->pt, ludev->err = pbdrv_uart_read_begin(ludev->uart, ludev->rx_msg + 1, 2, EV3_UART_IO_TIMEOUT)); - if (ludev->err != PBIO_SUCCESS) { - DBG_ERR(ludev->last_err = "UART Rx error while reading type"); - PT_EXIT(&ludev->pt); - } - PBIO_PT_WAIT_READY(&ludev->pt, ludev->err = pbdrv_uart_read_end(ludev->uart)); + PT_SPAWN(&ludev->pt, &ludev->read_pt, pbdrv_uart_read(&ludev->read_pt, ludev->uart, ludev->rx_msg + 1, 2, EV3_UART_IO_TIMEOUT, &ludev->err)); if (ludev->err != PBIO_SUCCESS) { DBG_ERR(ludev->last_err = "UART Rx error while reading type"); PT_EXIT(&ludev->pt); @@ -825,12 +798,7 @@ static PT_THREAD(pbdrv_legodev_pup_uart_synchronize_thread(pbdrv_legodev_pup_uar while (ludev->status == PBDRV_LEGODEV_PUP_UART_STATUS_INFO) { // read the message header - PBIO_PT_WAIT_READY(&ludev->pt, ludev->err = pbdrv_uart_read_begin(ludev->uart, ludev->rx_msg, 1, EV3_UART_IO_TIMEOUT)); - if (ludev->err != PBIO_SUCCESS) { - DBG_ERR(ludev->last_err = "UART Rx begin error during info header"); - PT_EXIT(&ludev->pt); - } - PBIO_PT_WAIT_READY(&ludev->pt, ludev->err = pbdrv_uart_read_end(ludev->uart)); + PT_SPAWN(&ludev->pt, &ludev->read_pt, pbdrv_uart_read(&ludev->read_pt, ludev->uart, ludev->rx_msg, 1, EV3_UART_IO_TIMEOUT, &ludev->err)); if (ludev->err != PBIO_SUCCESS) { DBG_ERR(ludev->last_err = "UART Rx end error during info header"); PT_EXIT(&ludev->pt); @@ -845,12 +813,7 @@ static PT_THREAD(pbdrv_legodev_pup_uart_synchronize_thread(pbdrv_legodev_pup_uar // read the rest of the message if (ludev->rx_msg_size > 1) { - PBIO_PT_WAIT_READY(&ludev->pt, ludev->err = pbdrv_uart_read_begin(ludev->uart, ludev->rx_msg + 1, ludev->rx_msg_size - 1, EV3_UART_IO_TIMEOUT)); - if (ludev->err != PBIO_SUCCESS) { - DBG_ERR(ludev->last_err = "UART Rx begin error during info"); - PT_EXIT(&ludev->pt); - } - PBIO_PT_WAIT_READY(&ludev->pt, ludev->err = pbdrv_uart_read_end(ludev->uart)); + PT_SPAWN(&ludev->pt, &ludev->read_pt, pbdrv_uart_read(&ludev->read_pt, ludev->uart, ludev->rx_msg + 1, ludev->rx_msg_size - 1, EV3_UART_IO_TIMEOUT, &ludev->err)); if (ludev->err != PBIO_SUCCESS) { DBG_ERR(ludev->last_err = "UART Rx end error during info"); PT_EXIT(&ludev->pt); @@ -871,7 +834,7 @@ static PT_THREAD(pbdrv_legodev_pup_uart_synchronize_thread(pbdrv_legodev_pup_uar // reply with ACK ludev->tx_msg[0] = LUMP_SYS_ACK; ludev->tx_msg_size = 1; - PT_SPAWN(&ludev->pt, &ludev->write_pt, pbdrv_legodev_pup_uart_send_prepared_msg(ludev, &ludev->err)); + PT_SPAWN(&ludev->pt, &ludev->write_pt, pbdrv_uart_write(&ludev->write_pt, ludev->uart, ludev->tx_msg, ludev->tx_msg_size, EV3_UART_IO_TIMEOUT, &ludev->err)); if (ludev->err != PBIO_SUCCESS) { DBG_ERR(ludev->last_err = "UART Tx error during ack."); PT_EXIT(&ludev->pt); @@ -942,7 +905,7 @@ static PT_THREAD(pbdrv_legodev_pup_uart_send_thread(pbdrv_legodev_pup_uart_dev_t ludev->data_rec = false; ludev->tx_msg[0] = LUMP_SYS_NACK; ludev->tx_msg_size = 1; - PT_SPAWN(&ludev->pt, &ludev->write_pt, pbdrv_legodev_pup_uart_send_prepared_msg(ludev, &ludev->err)); + PT_SPAWN(&ludev->pt, &ludev->write_pt, pbdrv_uart_write(&ludev->write_pt, ludev->uart, ludev->tx_msg, ludev->tx_msg_size, EV3_UART_IO_TIMEOUT, &ludev->err)); if (ludev->err != PBIO_SUCCESS) { DBG_ERR(ludev->last_err = "Error during keepalive."); PT_EXIT(&ludev->pt); @@ -959,7 +922,7 @@ static PT_THREAD(pbdrv_legodev_pup_uart_send_thread(pbdrv_legodev_pup_uart_dev_t if (ludev->mode_switch.requested) { ludev->mode_switch.requested = false; ev3_uart_prepare_tx_msg(ludev, LUMP_MSG_TYPE_CMD, LUMP_CMD_SELECT, &ludev->mode_switch.desired_mode, 1); - PT_SPAWN(&ludev->pt, &ludev->write_pt, pbdrv_legodev_pup_uart_send_prepared_msg(ludev, &ludev->err)); + PT_SPAWN(&ludev->pt, &ludev->write_pt, pbdrv_uart_write(&ludev->write_pt, ludev->uart, ludev->tx_msg, ludev->tx_msg_size, EV3_UART_IO_TIMEOUT, &ludev->err)); if (ludev->err != PBIO_SUCCESS) { DBG_ERR(ludev->last_err = "Setting requested mode failed."); PT_EXIT(&ludev->pt); @@ -973,7 +936,7 @@ static PT_THREAD(pbdrv_legodev_pup_uart_send_thread(pbdrv_legodev_pup_uart_dev_t ev3_uart_prepare_tx_msg(ludev, LUMP_MSG_TYPE_DATA, ludev->data_set->desired_mode, ludev->data_set->bin_data, ludev->data_set->size); ludev->data_set->size = 0; ludev->data_set->time = pbdrv_clock_get_ms(); - PT_SPAWN(&ludev->pt, &ludev->write_pt, pbdrv_legodev_pup_uart_send_prepared_msg(ludev, &ludev->err)); + PT_SPAWN(&ludev->pt, &ludev->write_pt, pbdrv_uart_write(&ludev->write_pt, ludev->uart, ludev->tx_msg, ludev->tx_msg_size, EV3_UART_IO_TIMEOUT, &ludev->err)); if (ludev->err != PBIO_SUCCESS) { DBG_ERR(ludev->last_err = "Setting requested data failed."); PT_EXIT(&ludev->pt); @@ -1004,19 +967,11 @@ static PT_THREAD(pbdrv_legodev_pup_uart_receive_data_thread(pbdrv_legodev_pup_ua // loose data. For now, the retry after bad message size helps get back into // sync with the data stream. - pbio_error_t err; - PT_BEGIN(&ludev->recv_pt); while (true) { - PBIO_PT_WAIT_READY(&ludev->recv_pt, - err = pbdrv_uart_read_begin(ludev->uart, ludev->rx_msg, 1, EV3_UART_IO_TIMEOUT)); - if (err != PBIO_SUCCESS) { - DBG_ERR(ludev->last_err = "UART Rx data header begin error"); - break; - } - PBIO_PT_WAIT_READY(&ludev->recv_pt, err = pbdrv_uart_read_end(ludev->uart)); - if (err != PBIO_SUCCESS) { + PT_SPAWN(&ludev->recv_pt, &ludev->read_pt, pbdrv_uart_read(&ludev->read_pt, ludev->uart, ludev->rx_msg, 1, EV3_UART_IO_TIMEOUT, &ludev->err)); + if (ludev->err != PBIO_SUCCESS) { DBG_ERR(ludev->last_err = "UART Rx data header end error"); break; } @@ -1035,14 +990,8 @@ static PT_THREAD(pbdrv_legodev_pup_uart_receive_data_thread(pbdrv_legodev_pup_ua continue; } - PBIO_PT_WAIT_READY(&ludev->recv_pt, - err = pbdrv_uart_read_begin(ludev->uart, ludev->rx_msg + 1, ludev->rx_msg_size - 1, EV3_UART_IO_TIMEOUT)); - if (err != PBIO_SUCCESS) { - DBG_ERR(ludev->last_err = "UART Rx data begin error"); - break; - } - PBIO_PT_WAIT_READY(&ludev->recv_pt, err = pbdrv_uart_read_end(ludev->uart)); - if (err != PBIO_SUCCESS) { + PT_SPAWN(&ludev->recv_pt, &ludev->read_pt, pbdrv_uart_read(&ludev->read_pt, ludev->uart, ludev->rx_msg + 1, ludev->rx_msg_size - 1, EV3_UART_IO_TIMEOUT, &ludev->err)); + if (ludev->err != PBIO_SUCCESS) { DBG_ERR(ludev->last_err = "UART Rx data end error"); break; } diff --git a/lib/pbio/drv/uart/uart_stm32f0.c b/lib/pbio/drv/uart/uart_stm32f0.c index cd7ebceb8..5375dca9e 100644 --- a/lib/pbio/drv/uart/uart_stm32f0.c +++ b/lib/pbio/drv/uart/uart_stm32f0.c @@ -43,16 +43,14 @@ typedef struct { uint8_t tx_buf_index; struct etimer rx_timer; struct etimer tx_timer; - volatile pbio_error_t rx_result; - volatile pbio_error_t tx_result; uint8_t irq; bool initialized; + /** Callback to call on read or write completion events */ + pbdrv_uart_poll_callback_t poll_callback; } pbdrv_uart_t; static pbdrv_uart_t pbdrv_uart[PBDRV_CONFIG_UART_STM32F0_NUM_UART]; -PROCESS(pbdrv_uart_process, "UART"); - pbio_error_t pbdrv_uart_get(uint8_t id, pbdrv_uart_dev_t **uart_dev) { if (id >= PBDRV_CONFIG_UART_STM32F0_NUM_UART) { return PBIO_ERROR_INVALID_ARG; @@ -67,115 +65,105 @@ pbio_error_t pbdrv_uart_get(uint8_t id, pbdrv_uart_dev_t **uart_dev) { return PBIO_SUCCESS; } -pbio_error_t pbdrv_uart_read_begin(pbdrv_uart_dev_t *uart_dev, uint8_t *msg, uint8_t length, uint32_t timeout) { +void pbdrv_uart_set_poll_callback(pbdrv_uart_dev_t *uart_dev, pbdrv_uart_poll_callback_t callback) { pbdrv_uart_t *uart = PBIO_CONTAINER_OF(uart_dev, pbdrv_uart_t, uart_dev); + uart->poll_callback = callback; +} +PT_THREAD(pbdrv_uart_read(struct pt *pt, pbdrv_uart_dev_t *uart_dev, uint8_t *msg, uint8_t length, uint32_t timeout, pbio_error_t *err)) { + + pbdrv_uart_t *uart = PBIO_CONTAINER_OF(uart_dev, pbdrv_uart_t, uart_dev); + + PT_BEGIN(pt); if (!msg || !length) { - return PBIO_ERROR_INVALID_ARG; + *err = PBIO_ERROR_INVALID_ARG; + PT_EXIT(pt); } if (uart->rx_buf) { - return PBIO_ERROR_AGAIN; + *err = PBIO_ERROR_BUSY; + PT_EXIT(pt); } uart->rx_buf = msg; uart->rx_buf_size = length; uart->rx_buf_index = 0; - uart->rx_result = PBIO_ERROR_AGAIN; etimer_set(&uart->rx_timer, timeout); - process_poll(&pbdrv_uart_process); - - return PBIO_SUCCESS; -} - -pbio_error_t pbdrv_uart_read_end(pbdrv_uart_dev_t *uart_dev) { - pbdrv_uart_t *uart = PBIO_CONTAINER_OF(uart_dev, pbdrv_uart_t, uart_dev); - pbio_error_t err = uart->rx_result; // read once since interrupt can modify it - - if (uart->rx_buf == NULL) { - // begin was not called first - return PBIO_ERROR_INVALID_OP; - } + // Await completion or timeout. + PT_WAIT_UNTIL(pt, ({ + // On every re-entry to the async read, drain the ring buffer + // into the current read buffer. This ensures that we use + // all available data if there have been multiple polls since our last + // re-entry. If there is already enough data in the buffer, this + // protothread completes right away without yielding once first. + while (uart->rx_ring_buf_head != uart->rx_ring_buf_tail && uart->rx_buf_index < uart->rx_buf_size) { + uart->rx_buf[uart->rx_buf_index++] = uart->rx_ring_buf[uart->rx_ring_buf_tail]; + uart->rx_ring_buf_tail = (uart->rx_ring_buf_tail + 1) & (UART_RING_BUF_SIZE - 1); + } + uart->rx_buf_index == uart->rx_buf_size || etimer_expired(&uart->rx_timer); + })); - if (err != PBIO_ERROR_AGAIN) { + if (etimer_expired(&uart->rx_timer)) { + *err = PBIO_ERROR_TIMEDOUT; + } else { etimer_stop(&uart->rx_timer); - uart->rx_buf = NULL; - } else if (etimer_expired(&uart->rx_timer)) { - err = PBIO_ERROR_TIMEDOUT; - uart->rx_buf = NULL; + *err = PBIO_SUCCESS; } + uart->rx_buf = NULL; - return err; + PT_END(pt); } -void pbdrv_uart_read_cancel(pbdrv_uart_dev_t *uart_dev) { - pbdrv_uart_t *uart = PBIO_CONTAINER_OF(uart_dev, pbdrv_uart_t, uart_dev); +PT_THREAD(pbdrv_uart_write(struct pt *pt, pbdrv_uart_dev_t *uart_dev, uint8_t *msg, uint8_t length, uint32_t timeout, pbio_error_t *err)) { - uart->rx_result = PBIO_ERROR_CANCELED; -} - -pbio_error_t pbdrv_uart_write_begin(pbdrv_uart_dev_t *uart_dev, uint8_t *msg, uint8_t length, uint32_t timeout) { pbdrv_uart_t *uart = PBIO_CONTAINER_OF(uart_dev, pbdrv_uart_t, uart_dev); + PT_BEGIN(pt); + if (!msg || !length) { - return PBIO_ERROR_INVALID_ARG; + *err = PBIO_ERROR_INVALID_ARG; + PT_EXIT(pt); } if (uart->tx_buf) { - return PBIO_ERROR_AGAIN; + *err = PBIO_ERROR_BUSY; + PT_EXIT(pt); } uart->tx_buf = msg; uart->tx_buf_size = length; uart->tx_buf_index = 0; - uart->tx_result = PBIO_ERROR_AGAIN; etimer_set(&uart->tx_timer, timeout); uart->USART->CR1 |= USART_CR1_TXEIE; - return PBIO_SUCCESS; -} - -pbio_error_t pbdrv_uart_write_end(pbdrv_uart_dev_t *uart_dev) { - pbdrv_uart_t *uart = PBIO_CONTAINER_OF(uart_dev, pbdrv_uart_t, uart_dev); - pbio_error_t err = uart->tx_result; // read once since interrupt can modify it + // Await completion or timeout. + PT_WAIT_UNTIL(pt, uart->tx_buf_index == uart->tx_buf_size || etimer_expired(&uart->tx_timer)); - if (uart->tx_buf == NULL) { - // begin was not called first - return PBIO_ERROR_INVALID_OP; - } - - if (err != PBIO_ERROR_AGAIN) { - etimer_stop(&uart->tx_timer); - uart->tx_buf = NULL; - } else if (etimer_expired(&uart->tx_timer)) { + if (etimer_expired(&uart->tx_timer)) { uart->USART->CR1 &= ~(USART_CR1_TXEIE | USART_CR1_TCIE); - err = PBIO_ERROR_TIMEDOUT; - uart->tx_buf = NULL; + *err = PBIO_ERROR_TIMEDOUT; + } else { + etimer_stop(&uart->rx_timer); + *err = PBIO_SUCCESS; } + uart->tx_buf = NULL; - return err; -} - -void pbdrv_uart_write_cancel(pbdrv_uart_dev_t *uart_dev) { - pbdrv_uart_t *uart = PBIO_CONTAINER_OF(uart_dev, pbdrv_uart_t, uart_dev); - - uart->USART->CR1 &= ~(USART_CR1_TXEIE | USART_CR1_TCIE); - uart->tx_result = PBIO_ERROR_CANCELED; + PT_END(pt); } void pbdrv_uart_set_baud_rate(pbdrv_uart_dev_t *uart_dev, uint32_t baud) { pbdrv_uart_t *uart = PBIO_CONTAINER_OF(uart_dev, pbdrv_uart_t, uart_dev); - uart->USART->BRR = PBDRV_CONFIG_SYS_CLOCK_RATE / baud; } void pbdrv_uart_flush(pbdrv_uart_dev_t *uart_dev) { pbdrv_uart_t *uart = PBIO_CONTAINER_OF(uart_dev, pbdrv_uart_t, uart_dev); uart->rx_buf = NULL; + uart->tx_buf = NULL; uart->rx_ring_buf_head = 0; uart->rx_ring_buf_tail = 0; uart->rx_buf_size = 0; @@ -191,7 +179,9 @@ void pbdrv_uart_stm32f0_handle_irq(uint8_t id) { // REVISIT: Do we need to have an overrun error when the ring buffer gets full? uart->rx_ring_buf[uart->rx_ring_buf_head] = uart->USART->RDR; uart->rx_ring_buf_head = (uart->rx_ring_buf_head + 1) & (UART_RING_BUF_SIZE - 1); - process_poll(&pbdrv_uart_process); + if (uart->poll_callback) { + uart->poll_callback(&uart->uart_dev); + } } // transmit next byte @@ -202,43 +192,22 @@ void pbdrv_uart_stm32f0_handle_irq(uint8_t id) { uart->USART->CR1 &= ~USART_CR1_TXEIE; uart->USART->CR1 |= USART_CR1_TCIE; } + // No poll needed here. The interrupt will keep going until + // the whole buffer has been written out. The completion event below + // will poll the parent process. } // transmission complete if (uart->USART->CR1 & USART_CR1_TCIE && isr & USART_ISR_TC) { uart->USART->CR1 &= ~USART_CR1_TCIE; - uart->tx_result = PBIO_SUCCESS; - process_poll(&pbdrv_uart_process); - } -} - -static void handle_poll(void) { - for (int i = 0; i < PBDRV_CONFIG_UART_STM32F0_NUM_UART; i++) { - pbdrv_uart_t *uart = &pbdrv_uart[i]; - - // if receive is pending and we have not received all bytes yet... - if (uart->rx_buf && uart->rx_result == PBIO_ERROR_AGAIN && uart->rx_buf_index < uart->rx_buf_size) { - // copy all available bytes to rx_buf - while (uart->rx_ring_buf_head != uart->rx_ring_buf_tail) { - uart->rx_buf[uart->rx_buf_index++] = uart->rx_ring_buf[uart->rx_ring_buf_tail]; - uart->rx_ring_buf_tail = (uart->rx_ring_buf_tail + 1) & (UART_RING_BUF_SIZE - 1); - // when rx_buf is full, send out a notification - if (uart->rx_buf_index == uart->rx_buf_size) { - uart->rx_result = PBIO_SUCCESS; - process_post(PROCESS_BROADCAST, PROCESS_EVENT_COM, NULL); - break; - } - } - } - - if (uart->tx_buf && uart->tx_buf_index == uart->tx_buf_size) { - // TODO: this should only be sent once per write_begin - process_post(PROCESS_BROADCAST, PROCESS_EVENT_COM, NULL); + if (uart->poll_callback) { + uart->poll_callback(&uart->uart_dev); } } } -static void handle_exit(void) { +// Currently not used +void handle_exit(void) { for (int i = 0; i < PBDRV_CONFIG_UART_STM32F0_NUM_UART; i++) { pbdrv_uart_t *uart = &pbdrv_uart[i]; NVIC_DisableIRQ(uart->irq); @@ -246,16 +215,6 @@ static void handle_exit(void) { } void pbdrv_uart_init(void) { - pbdrv_init_busy_up(); - process_start(&pbdrv_uart_process); -} - -PROCESS_THREAD(pbdrv_uart_process, ev, data) { - PROCESS_POLLHANDLER(handle_poll()); - PROCESS_EXITHANDLER(handle_exit()); - - PROCESS_BEGIN(); - for (int i = 0; i < PBDRV_CONFIG_UART_STM32F0_NUM_UART; i++) { const pbdrv_uart_stm32f0_platform_data_t *pdata = &pbdrv_uart_stm32f0_platform_data[i]; pbdrv_uart_t *uart = &pbdrv_uart[i]; @@ -270,14 +229,6 @@ PROCESS_THREAD(pbdrv_uart_process, ev, data) { uart->initialized = true; } - - pbdrv_init_busy_down(); - - while (true) { - PROCESS_WAIT_EVENT(); - } - - PROCESS_END(); } #endif // PBDRV_CONFIG_UART_STM32F0 diff --git a/lib/pbio/drv/uart/uart_stm32f4_ll_irq.c b/lib/pbio/drv/uart/uart_stm32f4_ll_irq.c index 35035ad86..99bd4ea8c 100644 --- a/lib/pbio/drv/uart/uart_stm32f4_ll_irq.c +++ b/lib/pbio/drv/uart/uart_stm32f4_ll_irq.c @@ -38,24 +38,29 @@ typedef struct { struct etimer read_timer; /** Timer for write timeout. */ struct etimer write_timer; - /** The buffer passed to the read_begin function. */ + /** The buffer of the ongoing async read function. */ uint8_t *read_buf; /** The length of read_buf in bytes. */ uint8_t read_length; /** The current position in read_buf. */ uint8_t read_pos; - /** The buffer passed to the write_begin function. */ + /** The buffer of the ongoing write function. */ uint8_t *write_buf; /** The length of write_buf in bytes. */ uint8_t write_length; /** The current position in write_buf. */ volatile uint8_t write_pos; + /** Callback to call on read or write completion events */ + pbdrv_uart_poll_callback_t poll_callback; } pbdrv_uart_t; static pbdrv_uart_t pbdrv_uart[PBDRV_CONFIG_UART_STM32F4_LL_IRQ_NUM_UART]; static uint8_t pbdrv_uart_rx_data[PBDRV_CONFIG_UART_STM32F4_LL_IRQ_NUM_UART][RX_DATA_SIZE]; -PROCESS(pbdrv_uart_process, "UART"); +void pbdrv_uart_set_poll_callback(pbdrv_uart_dev_t *uart_dev, pbdrv_uart_poll_callback_t callback) { + pbdrv_uart_t *uart = PBIO_CONTAINER_OF(uart_dev, pbdrv_uart_t, uart_dev); + uart->poll_callback = callback; +} pbio_error_t pbdrv_uart_get(uint8_t id, pbdrv_uart_dev_t **uart_dev) { if (id >= PBDRV_CONFIG_UART_STM32F4_LL_IRQ_NUM_UART) { @@ -72,12 +77,16 @@ pbio_error_t pbdrv_uart_get(uint8_t id, pbdrv_uart_dev_t **uart_dev) { return PBIO_SUCCESS; } -pbio_error_t pbdrv_uart_read_begin(pbdrv_uart_dev_t *uart_dev, uint8_t *msg, uint8_t length, uint32_t timeout) { +PT_THREAD(pbdrv_uart_read(struct pt *pt, pbdrv_uart_dev_t *uart_dev, uint8_t *msg, uint8_t length, uint32_t timeout, pbio_error_t *err)) { + pbdrv_uart_t *uart = PBIO_CONTAINER_OF(uart_dev, pbdrv_uart_t, uart_dev); + // Actual protothread starts here. + PT_BEGIN(pt); + if (uart->read_buf) { - // Another read operation is already in progress. - return PBIO_ERROR_AGAIN; + *err = PBIO_ERROR_BUSY; + PT_EXIT(pt); } uart->read_buf = msg; @@ -86,36 +95,44 @@ pbio_error_t pbdrv_uart_read_begin(pbdrv_uart_dev_t *uart_dev, uint8_t *msg, uin etimer_set(&uart->read_timer, timeout); - return PBIO_SUCCESS; -} - -pbio_error_t pbdrv_uart_read_end(pbdrv_uart_dev_t *uart_dev) { - pbdrv_uart_t *uart = PBIO_CONTAINER_OF(uart_dev, pbdrv_uart_t, uart_dev); - - // If read_pos is less that read_length then we have not read everything yet - if (uart->read_pos < uart->read_length) { - if (etimer_expired(&uart->read_timer)) { - uart->read_buf = NULL; - return PBIO_ERROR_TIMEDOUT; + // Await completion or timeout. + PT_WAIT_UNTIL(pt, ({ + // On every re-entry to the async read, drain the ring buffer + // into the current read buffer. This ensures that we use + // all available data if there have been multiple polls since our last + // re-entry. If there is already enough data in the buffer, this + // protothread completes right away without yielding once first. + while (uart->read_pos < uart->read_length) { + int c = ringbuf_get(&uart->rx_buf); + if (c == -1) { + break; + } + uart->read_buf[uart->read_pos++] = c; } - return PBIO_ERROR_AGAIN; - } + uart->read_pos == uart->read_length || etimer_expired(&uart->read_timer); + })); - etimer_stop(&uart->read_timer); + // Set exit status based on completion condition. + if (etimer_expired(&uart->read_timer)) { + *err = PBIO_ERROR_TIMEDOUT; + } else { + etimer_stop(&uart->read_timer); + *err = PBIO_SUCCESS; + } + uart->read_buf = NULL; - return PBIO_SUCCESS; + PT_END(pt); } -void pbdrv_uart_read_cancel(pbdrv_uart_dev_t *uart_dev) { - // TODO -} +PT_THREAD(pbdrv_uart_write(struct pt *pt, pbdrv_uart_dev_t *uart_dev, uint8_t *msg, uint8_t length, uint32_t timeout, pbio_error_t *err)) { -pbio_error_t pbdrv_uart_write_begin(pbdrv_uart_dev_t *uart_dev, uint8_t *msg, uint8_t length, uint32_t timeout) { pbdrv_uart_t *uart = PBIO_CONTAINER_OF(uart_dev, pbdrv_uart_t, uart_dev); + PT_BEGIN(pt); + if (uart->write_buf) { - // Another write operation is already in progress. - return PBIO_ERROR_AGAIN; + *err = PBIO_ERROR_BUSY; + PT_EXIT(pt); } uart->write_buf = msg; @@ -126,30 +143,22 @@ pbio_error_t pbdrv_uart_write_begin(pbdrv_uart_dev_t *uart_dev, uint8_t *msg, ui LL_USART_EnableIT_TXE(uart->pdata->uart); - return PBIO_SUCCESS; -} - -pbio_error_t pbdrv_uart_write_end(pbdrv_uart_dev_t *uart_dev) { - pbdrv_uart_t *uart = PBIO_CONTAINER_OF(uart_dev, pbdrv_uart_t, uart_dev); + // Await completion or timeout. + PT_WAIT_UNTIL(pt, uart->write_pos == uart->write_length || etimer_expired(&uart->write_timer)); - // If write_pos is less that write_length then we have not written everything yet. - if (uart->write_pos < uart->write_length) { - if (etimer_expired(&uart->write_timer)) { - LL_USART_DisableIT_TXE(uart->pdata->uart); - LL_USART_DisableIT_TC(uart->pdata->uart); - uart->write_buf = NULL; - return PBIO_ERROR_TIMEDOUT; - } - return PBIO_ERROR_AGAIN; + // Set exit status based on completion condition. + if (etimer_expired(&uart->write_timer)) { + LL_USART_DisableIT_TXE(uart->pdata->uart); + LL_USART_DisableIT_TC(uart->pdata->uart); + *err = PBIO_ERROR_TIMEDOUT; + } else { + etimer_stop(&uart->write_timer); + *err = PBIO_SUCCESS; } - etimer_stop(&uart->write_timer); + uart->write_buf = NULL; - return PBIO_SUCCESS; -} - -void pbdrv_uart_write_cancel(pbdrv_uart_dev_t *uart_dev) { - // TODO + PT_END(pt); } void pbdrv_uart_set_baud_rate(pbdrv_uart_dev_t *uart_dev, uint32_t baud) { @@ -179,6 +188,20 @@ void pbdrv_uart_set_baud_rate(pbdrv_uart_dev_t *uart_dev, uint32_t baud) { } void pbdrv_uart_flush(pbdrv_uart_dev_t *uart_dev) { + pbdrv_uart_t *uart = PBIO_CONTAINER_OF(uart_dev, pbdrv_uart_t, uart_dev); + // If a process was exited while an operation was in progress this is + // normally an error, and the process may call flush when it is restarted + // to clear the state. + uart->write_buf = NULL; + uart->write_length = 0; + uart->write_pos = 0; + uart->read_buf = NULL; + uart->read_length = 0; + uart->read_pos = 0; + // Discard all received bytes. + while (ringbuf_get(&uart->rx_buf) != -1) { + ; + } } void pbdrv_uart_stm32f4_ll_irq_handle_irq(uint8_t id) { @@ -188,7 +211,11 @@ void pbdrv_uart_stm32f4_ll_irq_handle_irq(uint8_t id) { if (sr & USART_SR_RXNE) { ringbuf_put(&uart->rx_buf, LL_USART_ReceiveData8(USARTx)); - process_poll(&pbdrv_uart_process); + // Poll parent process for each received byte, since the IRQ handler + // has no awareness of the expected length of the read operation. + if (uart->poll_callback) { + uart->poll_callback(&uart->uart_dev); + } } if (sr & USART_SR_ORE) { @@ -196,51 +223,29 @@ void pbdrv_uart_stm32f4_ll_irq_handle_irq(uint8_t id) { LL_USART_ReceiveData8(USARTx); } - if (USARTx->CR1 & USART_CR1_TXEIE && sr & USART_SR_TXE) { + if (USARTx->CR1 & USART_CR1_TXEIE && sr & USART_SR_TXE && uart->write_buf) { LL_USART_TransmitData8(USARTx, uart->write_buf[uart->write_pos++]); // When all bytes have been written, wait for the Tx complete interrupt. if (uart->write_pos == uart->write_length) { LL_USART_DisableIT_TXE(USARTx); LL_USART_EnableIT_TC(USARTx); } + // No need to poll parent process here: the interrupts will keep it + // going until the whole buffer has been written out. The completion + // event below will trigger the poll parent process. } if (USARTx->CR1 & USART_CR1_TCIE && sr & USART_SR_TC) { LL_USART_DisableIT_TC(USARTx); - process_poll(&pbdrv_uart_process); - } -} - -static void handle_poll(void) { - for (int i = 0; i < PBDRV_CONFIG_UART_STM32F4_LL_IRQ_NUM_UART; i++) { - pbdrv_uart_t *uart = &pbdrv_uart[i]; - - // if receive is pending and we have not received all bytes yet - while (uart->read_buf && uart->read_pos < uart->read_length) { - int c = ringbuf_get(&uart->rx_buf); - if (c == -1) { - break; - } - uart->read_buf[uart->read_pos++] = c; - } - - // broadcast when read_buf is full - if (uart->read_buf && uart->read_pos == uart->read_length) { - // clearing read_buf to prevent multiple broadcasts - uart->read_buf = NULL; - process_post(PROCESS_BROADCAST, PROCESS_EVENT_COM, NULL); - } - - // broadcast when write_buf is drained - if (uart->write_buf && uart->write_pos == uart->write_length) { - // clearing write_buf to prevent multiple broadcasts - uart->write_buf = NULL; - process_post(PROCESS_BROADCAST, PROCESS_EVENT_COM, NULL); + // Poll parent process to indicate the write operation is complete. + if (uart->poll_callback) { + uart->poll_callback(&uart->uart_dev); } } } -static void handle_exit(void) { +// Currently not used +void handle_exit(void) { for (int i = 0; i < PBDRV_CONFIG_UART_STM32F4_LL_IRQ_NUM_UART; i++) { const pbdrv_uart_stm32f4_ll_irq_platform_data_t *pdata = &pbdrv_uart_stm32f4_ll_irq_platform_data[i]; LL_USART_Disable(pdata->uart); @@ -249,15 +254,6 @@ static void handle_exit(void) { } void pbdrv_uart_init(void) { - pbdrv_init_busy_up(); - process_start(&pbdrv_uart_process); -} - -PROCESS_THREAD(pbdrv_uart_process, ev, data) { - PROCESS_POLLHANDLER(handle_poll()); - PROCESS_EXITHANDLER(handle_exit()); - - PROCESS_BEGIN(); for (int i = 0; i < PBDRV_CONFIG_UART_STM32F4_LL_IRQ_NUM_UART; i++) { const pbdrv_uart_stm32f4_ll_irq_platform_data_t *pdata = &pbdrv_uart_stm32f4_ll_irq_platform_data[i]; @@ -286,14 +282,6 @@ PROCESS_THREAD(pbdrv_uart_process, ev, data) { // start receiving as soon as everything is configured LL_USART_Enable(pdata->uart); } - - pbdrv_init_busy_down(); - - while (true) { - PROCESS_WAIT_EVENT(); - } - - PROCESS_END(); } #endif // PBDRV_CONFIG_UART_STM32F4_LL_IRQ diff --git a/lib/pbio/drv/uart/uart_stm32l4_ll_dma.c b/lib/pbio/drv/uart/uart_stm32l4_ll_dma.c index 7d75f5a30..8f10db46a 100644 --- a/lib/pbio/drv/uart/uart_stm32l4_ll_dma.c +++ b/lib/pbio/drv/uart/uart_stm32l4_ll_dma.c @@ -39,12 +39,17 @@ typedef struct { uint8_t rx_tail; uint8_t *read_buf; uint8_t read_length; + /** Callback to call on read or write completion events */ + pbdrv_uart_poll_callback_t poll_callback; } pbdrv_uart_t; static pbdrv_uart_t pbdrv_uart[PBDRV_CONFIG_UART_STM32L4_LL_DMA_NUM_UART]; static volatile uint8_t pbdrv_uart_rx_data[PBDRV_CONFIG_UART_STM32L4_LL_DMA_NUM_UART][RX_DATA_SIZE]; -PROCESS(pbdrv_uart_process, "UART"); +void pbdrv_uart_set_poll_callback(pbdrv_uart_dev_t *uart_dev, pbdrv_uart_poll_callback_t callback) { + pbdrv_uart_t *uart = PBIO_CONTAINER_OF(uart_dev, pbdrv_uart_t, uart_dev); + uart->poll_callback = callback; +} pbio_error_t pbdrv_uart_get(uint8_t id, pbdrv_uart_dev_t **uart_dev) { if (id >= PBDRV_CONFIG_UART_STM32L4_LL_DMA_NUM_UART) { @@ -61,21 +66,6 @@ pbio_error_t pbdrv_uart_get(uint8_t id, pbdrv_uart_dev_t **uart_dev) { return PBIO_SUCCESS; } -pbio_error_t pbdrv_uart_read_begin(pbdrv_uart_dev_t *uart_dev, uint8_t *msg, uint8_t length, uint32_t timeout) { - pbdrv_uart_t *uart = PBIO_CONTAINER_OF(uart_dev, pbdrv_uart_t, uart_dev); - - if (uart->read_buf) { - return PBIO_ERROR_AGAIN; - } - - uart->read_buf = msg; - uart->read_length = length; - - etimer_set(&uart->rx_timer, timeout); - - return PBIO_SUCCESS; -} - static void volatile_copy(volatile uint8_t *src, uint8_t *dst, uint8_t size) { for (int i = 0; i < size; i++) { dst[i] = src[i]; @@ -202,24 +192,39 @@ static bool dma_is_ht(DMA_TypeDef *DMAx, uint32_t channel) { } } -pbio_error_t pbdrv_uart_read_end(pbdrv_uart_dev_t *uart_dev) { +static uint32_t pbdrv_uart_get_num_available(pbdrv_uart_t *uart) { + // Head is the last position that DMA wrote to. + uint32_t rx_head = RX_DATA_SIZE - LL_DMA_GetDataLength(uart->pdata->rx_dma, uart->pdata->rx_dma_ch); + return (rx_head - uart->rx_tail) & (RX_DATA_SIZE - 1); +} + +PT_THREAD(pbdrv_uart_read(struct pt *pt, pbdrv_uart_dev_t *uart_dev, uint8_t *msg, uint8_t length, uint32_t timeout, pbio_error_t *err)) { + pbdrv_uart_t *uart = PBIO_CONTAINER_OF(uart_dev, pbdrv_uart_t, uart_dev); - const pbdrv_uart_stm32l4_ll_dma_platform_data_t *pdata = uart->pdata; - uint32_t rx_head; - // head is the last position that DMA wrote to - rx_head = RX_DATA_SIZE - LL_DMA_GetDataLength(pdata->rx_dma, pdata->rx_dma_ch); + PT_BEGIN(pt); - uint32_t available = (rx_head - uart->rx_tail) & (RX_DATA_SIZE - 1); - if (available < uart->read_length) { - if (etimer_expired(&uart->rx_timer)) { - uart->read_buf = NULL; - uart->read_length = 0; - return PBIO_ERROR_TIMEDOUT; - } - return PBIO_ERROR_AGAIN; + if (uart->read_buf) { + *err = PBIO_ERROR_BUSY; + PT_EXIT(pt); } + uart->read_buf = msg; + uart->read_length = length; + + etimer_set(&uart->rx_timer, timeout); + + // Wait until we have enough data or timeout. If there is enough data + // already, this completes right away without yielding once first. + PT_WAIT_UNTIL(pt, pbdrv_uart_get_num_available(uart) >= uart->read_length || etimer_expired(&uart->rx_timer)); + if (etimer_expired(&uart->rx_timer)) { + uart->read_buf = NULL; + uart->read_length = 0; + *err = PBIO_ERROR_TIMEDOUT; + PT_EXIT(pt); + } + + // Copy from ring buffer to user buffer, taking care of wrap-around. if (uart->rx_tail + uart->read_length > RX_DATA_SIZE) { uint32_t partial_size = RX_DATA_SIZE - uart->rx_tail; volatile_copy(&uart->rx_data[uart->rx_tail], &uart->read_buf[0], partial_size); @@ -233,20 +238,21 @@ pbio_error_t pbdrv_uart_read_end(pbdrv_uart_dev_t *uart_dev) { uart->read_length = 0; etimer_stop(&uart->rx_timer); + *err = PBIO_SUCCESS; - return PBIO_SUCCESS; + PT_END(pt); } -void pbdrv_uart_read_cancel(pbdrv_uart_dev_t *uart_dev) { - // TODO -} +PT_THREAD(pbdrv_uart_write(struct pt *pt, pbdrv_uart_dev_t *uart_dev, uint8_t *msg, uint8_t length, uint32_t timeout, pbio_error_t *err)) { -pbio_error_t pbdrv_uart_write_begin(pbdrv_uart_dev_t *uart_dev, uint8_t *msg, uint8_t length, uint32_t timeout) { pbdrv_uart_t *uart = PBIO_CONTAINER_OF(uart_dev, pbdrv_uart_t, uart_dev); const pbdrv_uart_stm32l4_ll_dma_platform_data_t *pdata = uart->pdata; + PT_BEGIN(pt); + if (LL_USART_IsEnabledDMAReq_TX(pdata->uart)) { - return PBIO_ERROR_AGAIN; + *err = PBIO_ERROR_BUSY; + PT_EXIT(pt); } LL_DMA_DisableChannel(pdata->tx_dma, pdata->tx_dma_ch); @@ -261,28 +267,16 @@ pbio_error_t pbdrv_uart_write_begin(pbdrv_uart_dev_t *uart_dev, uint8_t *msg, ui etimer_set(&uart->tx_timer, timeout); - return PBIO_SUCCESS; -} - -pbio_error_t pbdrv_uart_write_end(pbdrv_uart_dev_t *uart_dev) { - pbdrv_uart_t *uart = PBIO_CONTAINER_OF(uart_dev, pbdrv_uart_t, uart_dev); - const pbdrv_uart_stm32l4_ll_dma_platform_data_t *pdata = uart->pdata; - - if (LL_USART_IsEnabledDMAReq_TX(pdata->uart)) { - if (etimer_expired(&uart->tx_timer)) { - LL_USART_DisableDMAReq_TX(pdata->uart); - return PBIO_ERROR_TIMEDOUT; - } - return PBIO_ERROR_AGAIN; + PT_WAIT_WHILE(pt, LL_USART_IsEnabledDMAReq_TX(pdata->uart) && !etimer_expired(&uart->tx_timer)); + if (etimer_expired(&uart->tx_timer)) { + LL_USART_DisableDMAReq_TX(pdata->uart); + *err = PBIO_ERROR_TIMEDOUT; + } else { + etimer_stop(&uart->tx_timer); + *err = PBIO_SUCCESS; } - etimer_stop(&uart->tx_timer); - - return PBIO_SUCCESS; -} - -void pbdrv_uart_write_cancel(pbdrv_uart_dev_t *uart_dev) { - // TODO + PT_END(pt); } void pbdrv_uart_set_baud_rate(pbdrv_uart_dev_t *uart_dev, uint32_t baud) { @@ -326,6 +320,15 @@ void pbdrv_uart_flush(pbdrv_uart_dev_t *uart_dev) { pbdrv_uart_t *uart = PBIO_CONTAINER_OF(uart_dev, pbdrv_uart_t, uart_dev); uart->read_buf = NULL; uart->read_length = 0; + // Clears the ring buffer by setting tail equal to head. + uart->rx_tail = RX_DATA_SIZE - LL_DMA_GetDataLength(uart->pdata->rx_dma, uart->pdata->rx_dma_ch); +} + +static void poll_process_by_id(uint8_t id) { + pbdrv_uart_t *uart = &pbdrv_uart[id]; + if (uart->poll_callback) { + uart->poll_callback(&uart->uart_dev); + } } void pbdrv_uart_stm32l4_ll_dma_handle_tx_dma_irq(uint8_t id) { @@ -333,7 +336,7 @@ void pbdrv_uart_stm32l4_ll_dma_handle_tx_dma_irq(uint8_t id) { if (LL_DMA_IsEnabledIT_TC(pdata->tx_dma, pdata->tx_dma_ch) && dma_is_tc(pdata->tx_dma, pdata->tx_dma_ch)) { dma_clear_tc(pdata->tx_dma, pdata->tx_dma_ch); LL_USART_DisableDMAReq_TX(pdata->uart); - process_poll(&pbdrv_uart_process); + poll_process_by_id(id); } } @@ -342,12 +345,12 @@ void pbdrv_uart_stm32l4_ll_dma_handle_rx_dma_irq(uint8_t id) { if (LL_DMA_IsEnabledIT_HT(pdata->rx_dma, pdata->rx_dma_ch) && dma_is_ht(pdata->rx_dma, pdata->rx_dma_ch)) { dma_clear_ht(pdata->rx_dma, pdata->rx_dma_ch); - process_poll(&pbdrv_uart_process); + poll_process_by_id(id); } if (LL_DMA_IsEnabledIT_TC(pdata->rx_dma, pdata->rx_dma_ch) && dma_is_tc(pdata->rx_dma, pdata->rx_dma_ch)) { dma_clear_tc(pdata->rx_dma, pdata->rx_dma_ch); - process_poll(&pbdrv_uart_process); + poll_process_by_id(id); } } @@ -357,21 +360,17 @@ void pbdrv_uart_stm32l4_ll_dma_handle_uart_irq(uint8_t id) { if (LL_USART_IsEnabledIT_TC(pdata->uart) && LL_USART_IsActiveFlag_TC(pdata->uart)) { LL_USART_DisableIT_TC(pdata->uart); LL_USART_ClearFlag_TC(pdata->uart); - process_poll(&pbdrv_uart_process); + poll_process_by_id(id); } if (LL_USART_IsEnabledIT_IDLE(pdata->uart) && LL_USART_IsActiveFlag_IDLE(pdata->uart)) { LL_USART_ClearFlag_IDLE(pdata->uart); - process_poll(&pbdrv_uart_process); + poll_process_by_id(id); } } -static void handle_poll(void) { - // TODO: only broadcast when read or write is complete - process_post(PROCESS_BROADCAST, PROCESS_EVENT_COM, NULL); -} - -static void handle_exit(void) { +// Currently not used +void handle_exit(void) { for (int i = 0; i < PBDRV_CONFIG_UART_STM32L4_LL_DMA_NUM_UART; i++) { const pbdrv_uart_stm32l4_ll_dma_platform_data_t *pdata = &pbdrv_uart_stm32l4_ll_dma_platform_data[i]; LL_USART_Disable(pdata->uart); @@ -384,16 +383,6 @@ static void handle_exit(void) { } void pbdrv_uart_init(void) { - pbdrv_init_busy_up(); - process_start(&pbdrv_uart_process); -} - -PROCESS_THREAD(pbdrv_uart_process, ev, data) { - PROCESS_POLLHANDLER(handle_poll()); - PROCESS_EXITHANDLER(handle_exit()); - - PROCESS_BEGIN(); - for (int i = 0; i < PBDRV_CONFIG_UART_STM32L4_LL_DMA_NUM_UART; i++) { const pbdrv_uart_stm32l4_ll_dma_platform_data_t *pdata = &pbdrv_uart_stm32l4_ll_dma_platform_data[i]; volatile uint8_t *rx_data = pbdrv_uart_rx_data[i]; @@ -474,14 +463,6 @@ PROCESS_THREAD(pbdrv_uart_process, ev, data) { LL_DMA_EnableChannel(pdata->rx_dma, pdata->rx_dma_ch); LL_USART_Enable(pdata->uart); } - - pbdrv_init_busy_down(); - - while (true) { - PROCESS_WAIT_EVENT(); - } - - PROCESS_END(); } #endif // PBDRV_CONFIG_UART_STM32L4_LL_DMA diff --git a/lib/pbio/include/pbdrv/uart.h b/lib/pbio/include/pbdrv/uart.h index 32e5d2d2e..59d8ac60e 100644 --- a/lib/pbio/include/pbdrv/uart.h +++ b/lib/pbio/include/pbdrv/uart.h @@ -14,11 +14,14 @@ #include #include #include +#include typedef struct { } pbdrv_uart_dev_t; +typedef void (*pbdrv_uart_poll_callback_t)(pbdrv_uart_dev_t *uart); + #if PBDRV_CONFIG_UART pbio_error_t pbdrv_uart_get(uint8_t id, pbdrv_uart_dev_t **uart_dev); @@ -30,13 +33,11 @@ pbio_error_t pbdrv_uart_get(uint8_t id, pbdrv_uart_dev_t **uart_dev); * @return ::PBIO_SUCCESS if the baud rate was set or */ void pbdrv_uart_set_baud_rate(pbdrv_uart_dev_t *uart, uint32_t baud); -pbio_error_t pbdrv_uart_read_begin(pbdrv_uart_dev_t *uart, uint8_t *msg, uint8_t length, uint32_t timeout); -pbio_error_t pbdrv_uart_read_end(pbdrv_uart_dev_t *uart); -void pbdrv_uart_read_cancel(pbdrv_uart_dev_t *uart); -pbio_error_t pbdrv_uart_write_begin(pbdrv_uart_dev_t *uart, uint8_t *msg, uint8_t length, uint32_t timeout); -pbio_error_t pbdrv_uart_write_end(pbdrv_uart_dev_t *uart); -void pbdrv_uart_write_cancel(pbdrv_uart_dev_t *uart); void pbdrv_uart_flush(pbdrv_uart_dev_t *uart); +void pbdrv_uart_set_poll_callback(pbdrv_uart_dev_t *uart_dev, pbdrv_uart_poll_callback_t callback); + +PT_THREAD(pbdrv_uart_read(struct pt *pt, pbdrv_uart_dev_t *uart_dev, uint8_t *msg, uint8_t length, uint32_t timeout, pbio_error_t *err)); +PT_THREAD(pbdrv_uart_write(struct pt *pt, pbdrv_uart_dev_t *uart_dev, uint8_t *msg, uint8_t length, uint32_t timeout, pbio_error_t *err)); #else // PBDRV_CONFIG_UART @@ -52,8 +53,6 @@ static inline pbio_error_t pbdrv_uart_read_begin(pbdrv_uart_dev_t *uart, uint8_t static inline pbio_error_t pbdrv_uart_read_end(pbdrv_uart_dev_t *uart) { return PBIO_ERROR_NOT_SUPPORTED; } -static inline void pbdrv_uart_read_cancel(pbdrv_uart_dev_t *uart) { -} static inline pbio_error_t pbdrv_uart_write_begin(pbdrv_uart_dev_t *uart, uint8_t *msg, uint8_t length, uint32_t timeout) { return PBIO_ERROR_NOT_SUPPORTED; } @@ -64,7 +63,20 @@ static inline void pbdrv_uart_write_cancel(pbdrv_uart_dev_t *uart) { } static inline void pbdrv_uart_flush(pbdrv_uart_dev_t *uart) { } +static inline void pbdrv_uart_set_poll_callback(pbdrv_uart_poll_callback_t callback) { +} +static inline PT_THREAD(pbdrv_uart_write(struct pt *pt, pbdrv_uart_dev_t *uart_dev, uint8_t *msg, uint8_t length, uint32_t timeout, pbio_error_t *err)) { + PT_BEGIN(pt); + *err = PBIO_ERROR_NOT_SUPPORTED; + PT_END(pt); +} + +static inline PT_THREAD(pbdrv_uart_read(struct pt *pt, pbdrv_uart_dev_t *uart_dev, uint8_t *msg, uint8_t length, uint32_t timeout, pbio_error_t *err)) { + PT_BEGIN(pt); + *err = PBIO_ERROR_NOT_SUPPORTED; + PT_END(pt); +} #endif // PBDRV_CONFIG_UART diff --git a/lib/pbio/src/processes.h b/lib/pbio/src/processes.h index 34a66a040..b46e7b272 100644 --- a/lib/pbio/src/processes.h +++ b/lib/pbio/src/processes.h @@ -16,8 +16,4 @@ PROCESS_NAME(pbdrv_adc_process); #endif -#if PBDRV_CONFIG_UART -PROCESS_NAME(pbdrv_uart_process); -#endif - #endif // _PBIO_PROCESSES_H_ diff --git a/lib/pbio/test/src/test_uartdev.c b/lib/pbio/test/src/test_uartdev.c index d14eb8cb0..a3af8d8f6 100644 --- a/lib/pbio/test/src/test_uartdev.c +++ b/lib/pbio/test/src/test_uartdev.c @@ -1063,68 +1063,58 @@ extern bool pbio_legodev_test_process_auto_start; void pbdrv_uart_init(void) { } -pbio_error_t pbdrv_uart_read_begin(pbdrv_uart_dev_t *uart, uint8_t *msg, uint8_t length, uint32_t timeout) { - if (test_uart_dev.rx_msg) { - return PBIO_ERROR_AGAIN; - } +PT_THREAD(pbdrv_uart_read(struct pt *pt, pbdrv_uart_dev_t *uart_dev, uint8_t *msg, uint8_t length, uint32_t timeout, pbio_error_t *err)) { + + PT_BEGIN(pt); + + PT_WAIT_WHILE(pt, test_uart_dev.rx_msg); test_uart_dev.rx_msg = msg; test_uart_dev.rx_msg_length = length; test_uart_dev.rx_msg_result = PBIO_ERROR_AGAIN; etimer_set(&test_uart_dev.rx_timer, timeout); - return PBIO_SUCCESS; -} - -pbio_error_t pbdrv_uart_read_end(pbdrv_uart_dev_t *uart) { - assert(test_uart_dev.rx_msg); - - if (test_uart_dev.rx_msg_result == PBIO_ERROR_AGAIN && etimer_expired(&test_uart_dev.rx_timer)) { + // If read_pos is less that read_length then we have not read everything yet + PT_WAIT_WHILE(pt, test_uart_dev.rx_msg_result == PBIO_ERROR_AGAIN && !etimer_expired(&test_uart_dev.rx_timer)); + if (etimer_expired(&test_uart_dev.rx_timer)) { test_uart_dev.rx_msg_result = PBIO_ERROR_TIMEDOUT; + } else { + etimer_stop(&test_uart_dev.rx_timer); } if (test_uart_dev.rx_msg_result != PBIO_ERROR_AGAIN) { test_uart_dev.rx_msg = NULL; } - return test_uart_dev.rx_msg_result; + *err = test_uart_dev.rx_msg_result; + + PT_END(pt); } -void pbdrv_uart_read_cancel(pbdrv_uart_dev_t *uart) { +PT_THREAD(pbdrv_uart_write(struct pt *pt, pbdrv_uart_dev_t *uart_dev, uint8_t *msg, uint8_t length, uint32_t timeout, pbio_error_t *err)) { -} + PT_BEGIN(pt); -pbio_error_t pbdrv_uart_write_begin(pbdrv_uart_dev_t *uart, uint8_t *msg, uint8_t length, uint32_t timeout) { - if (test_uart_dev.tx_msg) { - return PBIO_ERROR_AGAIN; - } + // Wait while other write operation already in progress. + PT_WAIT_WHILE(pt, test_uart_dev.tx_msg); test_uart_dev.tx_msg = msg; test_uart_dev.tx_msg_length = length; test_uart_dev.tx_msg_result = PBIO_ERROR_AGAIN; etimer_set(&test_uart_dev.tx_timer, timeout); - return PBIO_SUCCESS; -} - -pbio_error_t pbdrv_uart_write_end(pbdrv_uart_dev_t *uart) { - if (!test_uart_dev.tx_msg) { - // Write end called without begin - // REVISIT: This passes but not very clean. - return PBIO_ERROR_INVALID_OP; - } - - if (test_uart_dev.tx_msg_result == PBIO_ERROR_AGAIN && etimer_expired(&test_uart_dev.tx_timer)) { + PT_WAIT_WHILE(pt, test_uart_dev.tx_msg_result == PBIO_ERROR_AGAIN && !etimer_expired(&test_uart_dev.tx_timer)); + if (etimer_expired(&test_uart_dev.tx_timer)) { test_uart_dev.tx_msg_result = PBIO_ERROR_TIMEDOUT; + } else { + etimer_stop(&test_uart_dev.tx_timer); } if (test_uart_dev.tx_msg_result != PBIO_ERROR_AGAIN) { test_uart_dev.tx_msg = NULL; } - return test_uart_dev.tx_msg_result; -} - -void pbdrv_uart_write_cancel(pbdrv_uart_dev_t *uart) { + *err = test_uart_dev.tx_msg_result; + PT_END(pt); } From e88360abe5a4107f151f47cecf50bd07020c925b Mon Sep 17 00:00:00 2001 From: Laurens Valk Date: Mon, 25 Nov 2024 11:54:25 +0100 Subject: [PATCH 2/5] pbio/drv/ioport_debug_uart: Follow uart updates. This can be used to debug non-blocking protothreads like the Bluetooth driver. We need to explicitly call back to the relevant process since the uart drivers are no longer broadcasting to every process. --- lib/pbio/drv/bluetooth/bluetooth_btstack.c | 3 +- .../drv/bluetooth/bluetooth_stm32_cc2640.c | 32 +++++---- lib/pbio/drv/ioport/ioport_debug_uart.c | 72 +++---------------- lib/pbio/drv/ioport/ioport_debug_uart.h | 35 ++++----- 4 files changed, 41 insertions(+), 101 deletions(-) diff --git a/lib/pbio/drv/bluetooth/bluetooth_btstack.c b/lib/pbio/drv/bluetooth/bluetooth_btstack.c index 179175972..910fdacc3 100644 --- a/lib/pbio/drv/bluetooth/bluetooth_btstack.c +++ b/lib/pbio/drv/bluetooth/bluetooth_btstack.c @@ -40,8 +40,7 @@ #endif #if 0 -#include -#define DEBUG_PRINT pbdrv_ioport_debug_uart_printf +#define DEBUG_PRINT(...) #else #define DEBUG_PRINT(...) #endif diff --git a/lib/pbio/drv/bluetooth/bluetooth_stm32_cc2640.c b/lib/pbio/drv/bluetooth/bluetooth_stm32_cc2640.c index eefe05c44..92c83b22d 100644 --- a/lib/pbio/drv/bluetooth/bluetooth_stm32_cc2640.c +++ b/lib/pbio/drv/bluetooth/bluetooth_stm32_cc2640.c @@ -43,26 +43,22 @@ #include "./bluetooth_stm32_cc2640.h" -#define DEBUG_LL (0x01) -#define DEBUG_PT (0x02) +PROCESS(pbdrv_bluetooth_spi_process, "Bluetooth SPI"); -// Choose either/or DEBUG_LL | DEBUG_PT -#define DEBUG (0) +#define DEBUG_ON_LAST_UART_PORT (0) -#if DEBUG +#if DEBUG_ON_LAST_UART_PORT #include -#endif -#if (DEBUG & DEBUG_LL) -#define DBG pbdrv_ioport_debug_uart_printf -#else #define DBG(...) -#endif -#if (DEBUG & DEBUG_PT) -#define DEBUG_PRINT pbdrv_ioport_debug_uart_printf +#define DEBUG_PRINT(...) #define DEBUG_PRINT_PT PBDRV_IOPORT_DEBUG_UART_PT_PRINTF +static void uart_poll_callback(pbdrv_uart_dev_t *uart) { + process_poll(&pbdrv_bluetooth_spi_process); +} #else -#define DEBUG_PRINT_PT(...) +#define DBG(...) #define DEBUG_PRINT(...) +#define DEBUG_PRINT_PT(...) #endif // hub name goes in special section so that it can be modified when flashing firmware @@ -156,8 +152,6 @@ static uint16_t uart_service_handle, uart_service_end_handle, uart_rx_char_handl // Nordic UART tx notifications enabled static bool uart_tx_notify_en; -PROCESS(pbdrv_bluetooth_spi_process, "Bluetooth SPI"); - LIST(task_queue); static bool bluetooth_ready; static pbdrv_bluetooth_on_event_t bluetooth_on_event; @@ -2277,6 +2271,14 @@ PROCESS_THREAD(pbdrv_bluetooth_spi_process, ev, data) { PROCESS_BEGIN(); + #if DEBUG_ON_LAST_UART_PORT + // Wait for the UART to be ready for debugging. + while (pbdrv_ioport_debug_uart_init(uart_poll_callback) != PBIO_SUCCESS) { + etimer_set(&timer, 100); + PROCESS_WAIT_EVENT_UNTIL(ev == PROCESS_EVENT_TIMER && etimer_expired(&timer)); + } + #endif // DEBUG_ON_LAST_UART_PORT + start: // take Bluetooth chip out of reset bluetooth_reset(RESET_STATE_OUT_HIGH); diff --git a/lib/pbio/drv/ioport/ioport_debug_uart.c b/lib/pbio/drv/ioport/ioport_debug_uart.c index 3837c8df1..2de0060b0 100644 --- a/lib/pbio/drv/ioport/ioport_debug_uart.c +++ b/lib/pbio/drv/ioport/ioport_debug_uart.c @@ -5,8 +5,6 @@ #if PBDRV_CONFIG_IOPORT_DEBUG_UART -#error "This driver can currently not be used." - #include #include #include @@ -22,16 +20,13 @@ #error "PBDRV_CONFIG_IOPORT_DEBUG_UART requires exactly one port" #endif -static char buffer[64]; -static pbdrv_uart_dev_t *debug_uart; - -#define UART_TIMEOUT (250) +// Use globals for simplified debug macro inside protothreads. +struct pt debug_printf_thread; +char debug_buffer[64]; +pbdrv_uart_dev_t *debug_uart; +pbio_error_t debug_err; -static pbio_error_t pbdrv_ioport_init(void) { - // UART already initialized. - if (debug_uart) { - return PBIO_SUCCESS; - } +pbio_error_t pbdrv_ioport_debug_uart_init(pbdrv_uart_poll_callback_t callback) { // Get the debug uart from last port. const pbdrv_ioport_pup_port_platform_data_t *port_data = &pbdrv_ioport_pup_platform_data.ports[PBDRV_CONFIG_IOPORT_NUM_DEV - 1]; @@ -41,6 +36,8 @@ static pbio_error_t pbdrv_ioport_init(void) { return err; } + pbdrv_uart_set_poll_callback(debug_uart, callback); + // Enable and configure uart. const pbdrv_ioport_pup_pins_t *pins = &port_data->pins; pbdrv_gpio_alt(&pins->uart_rx, pins->uart_alt); @@ -52,60 +49,11 @@ static pbio_error_t pbdrv_ioport_init(void) { return PBIO_SUCCESS; } -pbio_error_t pbdrv_ioport_debug_uart_printf(const char *format, ...) { - - // Get the debug uart. - pbio_error_t err = pbdrv_ioport_init(); - if (err != PBIO_SUCCESS) { - return err; - } - - // If still writing, just ignore this debug message. - err = pbdrv_uart_write_end(debug_uart); - if (err != PBIO_SUCCESS) { - return err; - } - - // Buffer result. - va_list args; - va_start(args, format); - vsnprintf(buffer, sizeof(buffer), format, args); - va_end(args); - - // Write to uart. - return pbdrv_uart_write_begin(debug_uart, (uint8_t *)buffer, strlen(buffer), UART_TIMEOUT); -} - -// Protothread set up here for simplified debug macro inside protothreads. -struct pt printf_thread; - -PT_THREAD(pbdrv_ioport_debug_uart_printf_thread(struct pt *pt, const char *format, ...)) { - static pbio_error_t err; - - PT_BEGIN(pt); - - // Only print if port initialized. - err = pbdrv_ioport_init(); - if (err != PBIO_SUCCESS) { - PT_EXIT(pt); - } - - // Buffer result. +void pbdrv_ioport_debug_uart_printf_buffer(const char *format, ...) { va_list args; va_start(args, format); - vsnprintf(buffer, sizeof(buffer), format, args); + vsnprintf(debug_buffer, sizeof(debug_buffer), format, args); va_end(args); - - // Wait for uart to be ready then write. - PT_WAIT_UNTIL((pt), (err = pbdrv_uart_write_begin(debug_uart, (uint8_t *)buffer, strlen(buffer), UART_TIMEOUT)) != PBIO_ERROR_AGAIN); - if (err != PBIO_SUCCESS) { - PT_EXIT(pt); - } - - // Wait for uart to finish. - PT_WAIT_UNTIL((pt), (err = pbdrv_uart_write_end(debug_uart)) != PBIO_ERROR_AGAIN); - - PT_END(pt); } #endif // PBDRV_CONFIG_IOPORT_DEBUG_UART diff --git a/lib/pbio/drv/ioport/ioport_debug_uart.h b/lib/pbio/drv/ioport/ioport_debug_uart.h index c327528e6..12921d3d0 100644 --- a/lib/pbio/drv/ioport/ioport_debug_uart.h +++ b/lib/pbio/drv/ioport/ioport_debug_uart.h @@ -9,28 +9,18 @@ #include #include +#include #if PBDRV_CONFIG_IOPORT_DEBUG_UART -extern struct pt printf_thread; +extern char debug_buffer[]; +extern struct pt debug_printf_thread; +extern pbdrv_uart_dev_t *debug_uart; +extern pbio_error_t debug_err; -/** - * Prints debug information on the last ioport. - * - * Will not print anything if the debug uart is already in use by another - * debug message, to avoid blocking. - * - * Useful outside of a protothread or when timing is more critical while - * complete information is not. - * - * @param [in] format Format string. - * @param [in] ... Arguments. - * - * @return Error code. - */ -pbio_error_t pbdrv_ioport_debug_uart_printf(const char *format, ...); +PT_THREAD(pbdrv_ioport_debug_uart_debug_printf_thread(struct pt *pt, const char *format, ...)); -PT_THREAD(pbdrv_ioport_debug_uart_printf_thread(struct pt *pt, const char *format, ...)); +void pbdrv_ioport_debug_uart_printf_buffer(const char *format, ...); /** * Spawns a task to print debug information on the last ioport. @@ -42,13 +32,14 @@ PT_THREAD(pbdrv_ioport_debug_uart_printf_thread(struct pt *pt, const char *forma * @param [in] ... Format string and arguments. */ #define PBDRV_IOPORT_DEBUG_UART_PT_PRINTF(pt, ...) \ - PT_SPAWN(pt, &printf_thread, pbdrv_ioport_debug_uart_printf_thread(&printf_thread, __VA_ARGS__)) + pbdrv_ioport_debug_uart_printf_buffer(__VA_ARGS__); \ + if (debug_uart) { \ + PT_SPAWN(pt, &debug_printf_thread, pbdrv_uart_write(&debug_printf_thread, debug_uart, (uint8_t *)debug_buffer, strlen(debug_buffer), 250, &debug_err)); \ + } \ -#else // PBDRV_CONFIG_IOPORT_DEBUG_UART +pbio_error_t pbdrv_ioport_debug_uart_init(pbdrv_uart_poll_callback_t callback); -static inline pbio_error_t pbdrv_ioport_debug_uart_printf(const char *format, ...) { - return PBIO_ERROR_NOT_SUPPORTED; -} +#else // PBDRV_CONFIG_IOPORT_DEBUG_UART #define PBDRV_IOPORT_DEBUG_UART_PT_PRINTF(pt, ...) From ff18d745c6a801a1b81d1f681c74ad7f44df8a30 Mon Sep 17 00:00:00 2001 From: Laurens Valk Date: Tue, 19 Nov 2024 11:16:42 +0100 Subject: [PATCH 3/5] pbio/drv/core: Drop process autostart. This was a longstanding chore that we have done gradually over time. The ADC driver was the last to be tranformed to the manual start format. To ensure that this isn't a potentially breaking change in itself, we move the call to initialize the ADC driver to the end of pbdrv_init, which is where the process was previously auto-started. --- lib/pbio/drv/adc/adc.h | 21 +++++++++++++++++++++ lib/pbio/drv/adc/adc_stm32_hal.c | 15 ++++++++++----- lib/pbio/drv/adc/adc_stm32f0.c | 6 +++--- lib/pbio/drv/core.c | 5 +++++ lib/pbio/drv/uart/uart_stm32f0.c | 1 - lib/pbio/drv/uart/uart_stm32f4_ll_irq.c | 1 - lib/pbio/drv/uart/uart_stm32l4_ll_dma.c | 1 - lib/pbio/src/main.c | 13 ------------- lib/pbio/src/processes.h | 19 ------------------- lib/pbio/test/src/test_drivebase.c | 1 - lib/pbio/test/src/test_servo.c | 1 - lib/pbio/test/src/test_uartdev.c | 1 - lib/pbio/test/test-pbio.c | 2 -- 13 files changed, 39 insertions(+), 48 deletions(-) create mode 100644 lib/pbio/drv/adc/adc.h delete mode 100644 lib/pbio/src/processes.h diff --git a/lib/pbio/drv/adc/adc.h b/lib/pbio/drv/adc/adc.h new file mode 100644 index 000000000..d42d38383 --- /dev/null +++ b/lib/pbio/drv/adc/adc.h @@ -0,0 +1,21 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2024 The Pybricks Authors + +// Internal common adc functions. + +#ifndef _INTERNAL_PBDRV_ADC_H_ +#define _INTERNAL_PBDRV_ADC_H_ + +#include + +#if PBDRV_CONFIG_ADC + +void pbdrv_adc_init(void); + +#else // PBDRV_CONFIG_ADC + +#define pbdrv_adc_init() + +#endif // PBDRV_CONFIG_ADC + +#endif // _INTERNAL_PBDRV_ADC_H_ diff --git a/lib/pbio/drv/adc/adc_stm32_hal.c b/lib/pbio/drv/adc/adc_stm32_hal.c index 5667d1b3b..9e4e1071c 100644 --- a/lib/pbio/drv/adc/adc_stm32_hal.c +++ b/lib/pbio/drv/adc/adc_stm32_hal.c @@ -81,11 +81,7 @@ static void pbdrv_adc_exit(void) { HAL_DMA_DeInit(&pbdrv_adc_hdma); } -PROCESS_THREAD(pbdrv_adc_process, ev, data) { - PROCESS_POLLHANDLER(pbdrv_adc_poll()); - PROCESS_EXITHANDLER(pbdrv_adc_exit()); - - PROCESS_BEGIN(); +void pbdrv_adc_init(void) { // Timer to trigger ADC @@ -149,6 +145,15 @@ PROCESS_THREAD(pbdrv_adc_process, ev, data) { HAL_ADC_Start_DMA(&pbdrv_adc_hadc, pbdrv_adc_dma_buffer, PBIO_ARRAY_SIZE(pbdrv_adc_dma_buffer)); HAL_TIM_Base_Start(&pbdrv_adc_htim); + process_start(&pbdrv_adc_process); +} + +PROCESS_THREAD(pbdrv_adc_process, ev, data) { + PROCESS_POLLHANDLER(pbdrv_adc_poll()); + PROCESS_EXITHANDLER(pbdrv_adc_exit()); + + PROCESS_BEGIN(); + while (true) { PROCESS_WAIT_EVENT(); } diff --git a/lib/pbio/drv/adc/adc_stm32f0.c b/lib/pbio/drv/adc/adc_stm32f0.c index d028b3aee..0bd99b507 100644 --- a/lib/pbio/drv/adc/adc_stm32f0.c +++ b/lib/pbio/drv/adc/adc_stm32f0.c @@ -35,7 +35,7 @@ static void pbdrv_adc_calibrate(void) { } } -static void pbdrv_adc_init(void) { +void pbdrv_adc_init(void) { // enable power domain RCC->APB2ENR |= RCC_APB2ENR_ADCEN; @@ -61,6 +61,8 @@ static void pbdrv_adc_init(void) { // TODO: LEGO firmware reads CH 3 during init 10 times and averages it. // Not sure what this is measuring or what it would be used for. Perhaps // some kind of ID resistor? + + process_start(&pbdrv_adc_process); } // does a single conversion for the specified channel @@ -91,8 +93,6 @@ PROCESS_THREAD(pbdrv_adc_process, ev, data) { PROCESS_BEGIN(); - pbdrv_adc_init(); - while (true) { PROCESS_WAIT_EVENT(); } diff --git a/lib/pbio/drv/core.c b/lib/pbio/drv/core.c index 9c1b1fdcf..ad57863be 100644 --- a/lib/pbio/drv/core.c +++ b/lib/pbio/drv/core.c @@ -6,6 +6,7 @@ #include #include "core.h" +#include "adc/adc.h" #include "battery/battery.h" #include "block_device/block_device.h" #include "bluetooth/bluetooth.h" @@ -78,4 +79,8 @@ void pbdrv_init(void) { // devices behaves the same as if unknown devices are still syncing up after // just being plugged in. pbdrv_legodev_init(); + + // REVISIT: verify that the order does not matter and that we can start this + // in the same place as the other drivers above. + pbdrv_adc_init(); } diff --git a/lib/pbio/drv/uart/uart_stm32f0.c b/lib/pbio/drv/uart/uart_stm32f0.c index 5375dca9e..41b7a19ec 100644 --- a/lib/pbio/drv/uart/uart_stm32f0.c +++ b/lib/pbio/drv/uart/uart_stm32f0.c @@ -21,7 +21,6 @@ #include #include -#include "../../src/processes.h" #include "../core.h" #include "stm32f0xx.h" diff --git a/lib/pbio/drv/uart/uart_stm32f4_ll_irq.c b/lib/pbio/drv/uart/uart_stm32f4_ll_irq.c index 99bd4ea8c..4c97311b9 100644 --- a/lib/pbio/drv/uart/uart_stm32f4_ll_irq.c +++ b/lib/pbio/drv/uart/uart_stm32f4_ll_irq.c @@ -23,7 +23,6 @@ #include "../core.h" #include "./uart_stm32f4_ll_irq.h" -#include "../../src/processes.h" #define RX_DATA_SIZE 64 // must be power of 2 for ring buffer! diff --git a/lib/pbio/drv/uart/uart_stm32l4_ll_dma.c b/lib/pbio/drv/uart/uart_stm32l4_ll_dma.c index 8f10db46a..e2a87a61c 100644 --- a/lib/pbio/drv/uart/uart_stm32l4_ll_dma.c +++ b/lib/pbio/drv/uart/uart_stm32l4_ll_dma.c @@ -20,7 +20,6 @@ #include #include "./uart_stm32l4_ll_dma.h" -#include "../../src/processes.h" #include "../core.h" #include "stm32l4xx_ll_dma.h" diff --git a/lib/pbio/src/main.c b/lib/pbio/src/main.c index 0f8a47c12..2e1e2a944 100644 --- a/lib/pbio/src/main.c +++ b/lib/pbio/src/main.c @@ -23,15 +23,6 @@ #include #include "light/animation.h" -#include "processes.h" - -// DO NOT ADD NEW PROCESSES HERE! -// We are trying to remove the use of autostart. -AUTOSTART_PROCESSES( -#if PBDRV_CONFIG_ADC - &pbdrv_adc_process, -#endif - NULL); /** * Initialize the Pybricks I/O Library. This function must be called once, @@ -41,10 +32,6 @@ AUTOSTART_PROCESSES( void pbio_init(void) { pbdrv_init(); - // TODO: remove autostart - this currently starts legacy drivers like analog - // it has to be after pbdrv_init() but before anything else - autostart_start(autostart_processes); - #if PBIO_CONFIG_MOTOR_PROCESS_AUTO_START pbio_motor_process_start(); #endif diff --git a/lib/pbio/src/processes.h b/lib/pbio/src/processes.h deleted file mode 100644 index b46e7b272..000000000 --- a/lib/pbio/src/processes.h +++ /dev/null @@ -1,19 +0,0 @@ -// SPDX-License-Identifier: MIT -// Copyright (c) 2019-2020 The Pybricks Authors - -#ifndef _PBIO_PROCESSES_H_ -#define _PBIO_PROCESSES_H_ - -#include - -#include -#include - -// DO NOT ADD NEW PROCESSES HERE! -// we are trying to get rid of this file. - -#if PBDRV_CONFIG_ADC -PROCESS_NAME(pbdrv_adc_process); -#endif - -#endif // _PBIO_PROCESSES_H_ diff --git a/lib/pbio/test/src/test_drivebase.c b/lib/pbio/test/src/test_drivebase.c index d3382d137..6d78b8eba 100644 --- a/lib/pbio/test/src/test_drivebase.c +++ b/lib/pbio/test/src/test_drivebase.c @@ -26,7 +26,6 @@ #include #include -#include "../src/processes.h" #include "../drv/core.h" #include "../drv/clock/clock_test.h" #include "../drv/motor_driver/motor_driver_virtual_simulation.h" diff --git a/lib/pbio/test/src/test_servo.c b/lib/pbio/test/src/test_servo.c index ba3a17003..c2cf80474 100644 --- a/lib/pbio/test/src/test_servo.c +++ b/lib/pbio/test/src/test_servo.c @@ -25,7 +25,6 @@ #include #include -#include "../src/processes.h" #include "../drv/core.h" #include "../drv/clock/clock_test.h" #include "../drv/motor_driver/motor_driver_virtual_simulation.h" diff --git a/lib/pbio/test/src/test_uartdev.c b/lib/pbio/test/src/test_uartdev.c index a3af8d8f6..a83129f5b 100644 --- a/lib/pbio/test/src/test_uartdev.c +++ b/lib/pbio/test/src/test_uartdev.c @@ -23,7 +23,6 @@ #include "../drv/legodev/legodev_pup_uart.h" #include "../drv/legodev/legodev_test.h" -#include "../src/processes.h" #include "../drv/clock/clock_test.h" // TODO: submit this upstream diff --git a/lib/pbio/test/test-pbio.c b/lib/pbio/test/test-pbio.c index 9683b4682..885c11453 100644 --- a/lib/pbio/test/test-pbio.c +++ b/lib/pbio/test/test-pbio.c @@ -15,8 +15,6 @@ #include -#include "src/processes.h" - #define PBIO_TEST_TIMEOUT 1 // seconds void pbio_test_run_thread(void *env) { From 6a606d304285038ecaa9815194373e2145dc23bd Mon Sep 17 00:00:00 2001 From: Laurens Valk Date: Tue, 19 Nov 2024 11:18:43 +0100 Subject: [PATCH 4/5] pbio/drv/core: Move ADC init to normal start location. This completes the transition started in the previous commit. The ADC does not depend on anything other than clocks, so we can move it along with the other drivers. --- lib/pbio/drv/core.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/pbio/drv/core.c b/lib/pbio/drv/core.c index ad57863be..899edc938 100644 --- a/lib/pbio/drv/core.c +++ b/lib/pbio/drv/core.c @@ -39,6 +39,7 @@ void pbdrv_init(void) { process_start(&etimer_process); // the rest of the drivers should be implemented so that init order doesn't matter + pbdrv_adc_init(); pbdrv_battery_init(); pbdrv_block_device_init(); pbdrv_bluetooth_init(); @@ -79,8 +80,4 @@ void pbdrv_init(void) { // devices behaves the same as if unknown devices are still syncing up after // just being plugged in. pbdrv_legodev_init(); - - // REVISIT: verify that the order does not matter and that we can start this - // in the same place as the other drivers above. - pbdrv_adc_init(); } From 4f2ed8e1709046b82481f09f8cae57f874cb0a81 Mon Sep 17 00:00:00 2001 From: Laurens Valk Date: Tue, 3 Dec 2024 15:04:04 +0100 Subject: [PATCH 5/5] pbio/platform/contiki-conf: Drop autostart enable. This is no longer being used. --- lib/pbio/platform/city_hub/contiki-conf.h | 1 - lib/pbio/platform/debug/contiki-conf.h | 1 - lib/pbio/platform/essential_hub/contiki-conf.h | 1 - lib/pbio/platform/ev3/contiki-conf.h | 1 - lib/pbio/platform/ev3dev_stretch/contiki-conf.h | 1 - lib/pbio/platform/ev3rt/contiki-conf.h | 1 - lib/pbio/platform/move_hub/contiki-conf.h | 1 - lib/pbio/platform/nxt/contiki-conf.h | 1 - lib/pbio/platform/prime_hub/contiki-conf.h | 1 - lib/pbio/platform/technic_hub/contiki-conf.h | 1 - lib/pbio/platform/virtual_hub/contiki-conf.h | 1 - 11 files changed, 11 deletions(-) diff --git a/lib/pbio/platform/city_hub/contiki-conf.h b/lib/pbio/platform/city_hub/contiki-conf.h index c1dedd936..483a2c679 100644 --- a/lib/pbio/platform/city_hub/contiki-conf.h +++ b/lib/pbio/platform/city_hub/contiki-conf.h @@ -8,7 +8,6 @@ #define CCIF #define CLIF -#define AUTOSTART_ENABLE 1 typedef uint32_t clock_time_t; #define CLOCK_CONF_SECOND 1000 diff --git a/lib/pbio/platform/debug/contiki-conf.h b/lib/pbio/platform/debug/contiki-conf.h index c1dedd936..483a2c679 100644 --- a/lib/pbio/platform/debug/contiki-conf.h +++ b/lib/pbio/platform/debug/contiki-conf.h @@ -8,7 +8,6 @@ #define CCIF #define CLIF -#define AUTOSTART_ENABLE 1 typedef uint32_t clock_time_t; #define CLOCK_CONF_SECOND 1000 diff --git a/lib/pbio/platform/essential_hub/contiki-conf.h b/lib/pbio/platform/essential_hub/contiki-conf.h index c1dedd936..483a2c679 100644 --- a/lib/pbio/platform/essential_hub/contiki-conf.h +++ b/lib/pbio/platform/essential_hub/contiki-conf.h @@ -8,7 +8,6 @@ #define CCIF #define CLIF -#define AUTOSTART_ENABLE 1 typedef uint32_t clock_time_t; #define CLOCK_CONF_SECOND 1000 diff --git a/lib/pbio/platform/ev3/contiki-conf.h b/lib/pbio/platform/ev3/contiki-conf.h index 3585e853a..40159f606 100644 --- a/lib/pbio/platform/ev3/contiki-conf.h +++ b/lib/pbio/platform/ev3/contiki-conf.h @@ -8,7 +8,6 @@ #define CCIF #define CLIF -#define AUTOSTART_ENABLE 1 typedef uint32_t clock_time_t; #define CLOCK_CONF_SECOND 1000 diff --git a/lib/pbio/platform/ev3dev_stretch/contiki-conf.h b/lib/pbio/platform/ev3dev_stretch/contiki-conf.h index 8c0ed1bf6..0f175da17 100644 --- a/lib/pbio/platform/ev3dev_stretch/contiki-conf.h +++ b/lib/pbio/platform/ev3dev_stretch/contiki-conf.h @@ -8,7 +8,6 @@ #define CCIF #define CLIF -#define AUTOSTART_ENABLE 1 // TODO: could use 64-bit time struct typedef uint32_t clock_time_t; diff --git a/lib/pbio/platform/ev3rt/contiki-conf.h b/lib/pbio/platform/ev3rt/contiki-conf.h index 3585e853a..40159f606 100644 --- a/lib/pbio/platform/ev3rt/contiki-conf.h +++ b/lib/pbio/platform/ev3rt/contiki-conf.h @@ -8,7 +8,6 @@ #define CCIF #define CLIF -#define AUTOSTART_ENABLE 1 typedef uint32_t clock_time_t; #define CLOCK_CONF_SECOND 1000 diff --git a/lib/pbio/platform/move_hub/contiki-conf.h b/lib/pbio/platform/move_hub/contiki-conf.h index 3585e853a..40159f606 100644 --- a/lib/pbio/platform/move_hub/contiki-conf.h +++ b/lib/pbio/platform/move_hub/contiki-conf.h @@ -8,7 +8,6 @@ #define CCIF #define CLIF -#define AUTOSTART_ENABLE 1 typedef uint32_t clock_time_t; #define CLOCK_CONF_SECOND 1000 diff --git a/lib/pbio/platform/nxt/contiki-conf.h b/lib/pbio/platform/nxt/contiki-conf.h index c1dedd936..483a2c679 100644 --- a/lib/pbio/platform/nxt/contiki-conf.h +++ b/lib/pbio/platform/nxt/contiki-conf.h @@ -8,7 +8,6 @@ #define CCIF #define CLIF -#define AUTOSTART_ENABLE 1 typedef uint32_t clock_time_t; #define CLOCK_CONF_SECOND 1000 diff --git a/lib/pbio/platform/prime_hub/contiki-conf.h b/lib/pbio/platform/prime_hub/contiki-conf.h index c1dedd936..483a2c679 100644 --- a/lib/pbio/platform/prime_hub/contiki-conf.h +++ b/lib/pbio/platform/prime_hub/contiki-conf.h @@ -8,7 +8,6 @@ #define CCIF #define CLIF -#define AUTOSTART_ENABLE 1 typedef uint32_t clock_time_t; #define CLOCK_CONF_SECOND 1000 diff --git a/lib/pbio/platform/technic_hub/contiki-conf.h b/lib/pbio/platform/technic_hub/contiki-conf.h index c1dedd936..483a2c679 100644 --- a/lib/pbio/platform/technic_hub/contiki-conf.h +++ b/lib/pbio/platform/technic_hub/contiki-conf.h @@ -8,7 +8,6 @@ #define CCIF #define CLIF -#define AUTOSTART_ENABLE 1 typedef uint32_t clock_time_t; #define CLOCK_CONF_SECOND 1000 diff --git a/lib/pbio/platform/virtual_hub/contiki-conf.h b/lib/pbio/platform/virtual_hub/contiki-conf.h index aee3c316f..c3c18d5f3 100644 --- a/lib/pbio/platform/virtual_hub/contiki-conf.h +++ b/lib/pbio/platform/virtual_hub/contiki-conf.h @@ -8,7 +8,6 @@ #define CCIF #define CLIF -#define AUTOSTART_ENABLE 1 typedef uint32_t clock_time_t; #define CLOCK_CONF_SECOND 1000