From c8f672fcc30743efabf4014327476b759f2147e1 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Fri, 31 Mar 2023 15:21:08 -0500 Subject: [PATCH] drv/bluetooth_stm32_cc2640: strict ordering of tasks Similar to efda5d02, this avoids bugs by only allowing the Bluetooth chip to do one thing at a time and blocks other requested tasks until the previous ones have completed. This helps prevent the Bluetooth chip from getting in a bad state and fixes some existing known bugs. We can also now drop the workaround for [1] from 7d7434f (which in addition to the changes in this commit is also now avoided by a01772bd). [1]: https://github.com/pybricks/support/issues/736 Fixes: https://github.com/pybricks/support/issues/567 --- CHANGELOG.md | 3 + .../drv/bluetooth/bluetooth_stm32_cc2640.c | 65 ++++++++++--------- 2 files changed, 38 insertions(+), 30 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 50799f61f..d6d52e2c7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,9 @@ ### Fixed - Fixed iterator for `Matrix` objects giving bad values. +- Fixed Bluetooth sometimes locking up on Technic/City hubs ([support#567]). + +[support#567]: https://github.com/pybricks/support/issues/567 ## [3.3.0b3] - 2023-03-28 diff --git a/lib/pbio/drv/bluetooth/bluetooth_stm32_cc2640.c b/lib/pbio/drv/bluetooth/bluetooth_stm32_cc2640.c index 247a7337e..ab10daaaf 100644 --- a/lib/pbio/drv/bluetooth/bluetooth_stm32_cc2640.c +++ b/lib/pbio/drv/bluetooth/bluetooth_stm32_cc2640.c @@ -105,9 +105,6 @@ static uint8_t read_buf[RX_BUFFER_SIZE]; // value is set to 0 when Tx is complete static uint8_t write_xfer_size; -// set to true when ATT_HandleValueNoti is pending -static bool notification_in_progress; - // reflects state of SRDY signal volatile bool spi_srdy; // count of SRDY signal falling edges @@ -175,6 +172,12 @@ static pbio_error_t ble_error_to_pbio_error(HCI_StatusCodes_t status) { } } +static void start_task(pbio_task_t *task, pbio_task_thread_t thread, void *context) { + pbio_task_init(task, thread, context); + list_add(task_queue, task); + process_poll(&pbdrv_bluetooth_spi_process); +} + /** * Gets a vendor-specific event payload for @p handle. * @param [in] handle The connection handle. @@ -330,8 +333,7 @@ static PT_THREAD(set_discoverable(struct pt *pt, pbio_task_t *task)) { void pbdrv_bluetooth_start_advertising(void) { static pbio_task_t task; - pbio_task_init(&task, set_discoverable, NULL); - pbio_task_queue_add(task_queue, &task); + start_task(&task, set_discoverable, NULL); } static PT_THREAD(set_non_discoverable(struct pt *pt, pbio_task_t *task)) { @@ -352,8 +354,7 @@ static PT_THREAD(set_non_discoverable(struct pt *pt, pbio_task_t *task)) { void pbdrv_bluetooth_stop_advertising(void) { static pbio_task_t task; - pbio_task_init(&task, set_non_discoverable, NULL); - pbio_task_queue_add(task_queue, &task); + start_task(&task, set_non_discoverable, NULL); } bool pbdrv_bluetooth_is_connected(pbdrv_bluetooth_connection_t connection) { @@ -420,9 +421,7 @@ static PT_THREAD(send_value_notification(struct pt *pt, pbio_task_t *task)) req.pValue = send->data; ATT_HandleValueNoti(conn_handle, &req); } - notification_in_progress = true; PT_WAIT_UNTIL(pt, hci_command_status); - notification_in_progress = false; HCI_StatusCodes_t status = read_buf[8]; @@ -445,8 +444,7 @@ static PT_THREAD(send_value_notification(struct pt *pt, pbio_task_t *task)) void pbdrv_bluetooth_send(pbdrv_bluetooth_send_context_t *context) { static pbio_task_t task; - pbio_task_init(&task, send_value_notification, context); - pbio_task_queue_add(task_queue, &task); + start_task(&task, send_value_notification, context); } void pbdrv_bluetooth_set_receive_handler(pbdrv_bluetooth_receive_handler_t handler) { @@ -466,7 +464,7 @@ static PT_THREAD(scan_and_connect_task(struct pt *pt, pbio_task_t *task)) { // calling GAP_DeviceDiscoveryRequest can cause notifications to be dropped, // so we need to ensure than none are pending before we proceeded - PT_WAIT_WHILE(pt, write_xfer_size || notification_in_progress); + PT_WAIT_WHILE(pt, write_xfer_size); GAP_DeviceDiscoveryRequest(GAP_DEVICE_DISCOVERY_MODE_ALL, 1, GAP_FILTER_POLICY_SCAN_ANY_CONNECT_ANY); PT_WAIT_UNTIL(pt, hci_command_status); @@ -674,8 +672,7 @@ static PT_THREAD(scan_and_connect_task(struct pt *pt, pbio_task_t *task)) { } void pbdrv_bluetooth_scan_and_connect(pbio_task_t *task, pbdrv_bluetooth_scan_and_connect_context_t *context) { - pbio_task_init(task, scan_and_connect_task, context); - pbio_task_queue_add(task_queue, task); + start_task(task, scan_and_connect_task, context); } static PT_THREAD(write_remote_task(struct pt *pt, pbio_task_t *task)) { @@ -748,8 +745,7 @@ static PT_THREAD(write_remote_task(struct pt *pt, pbio_task_t *task)) { } void pbdrv_bluetooth_write_remote(pbio_task_t *task, pbdrv_bluetooth_value_t *value) { - pbio_task_init(task, write_remote_task, value); - pbio_task_queue_add(task_queue, task); + start_task(task, write_remote_task, value); } static PT_THREAD(disconnect_remote_task(struct pt *pt, pbio_task_t *task)) { @@ -768,8 +764,7 @@ static PT_THREAD(disconnect_remote_task(struct pt *pt, pbio_task_t *task)) { void pbdrv_bluetooth_disconnect_remote(void) { static pbio_task_t task; - pbio_task_init(&task, disconnect_remote_task, NULL); - pbio_task_queue_add(task_queue, &task); + start_task(&task, disconnect_remote_task, NULL); } // Driver interrupt callbacks @@ -1762,13 +1757,29 @@ PROCESS_THREAD(pbdrv_bluetooth_spi_process, ev, data) { PROCESS_WAIT_EVENT_UNTIL(ev == PROCESS_EVENT_TIMER && etimer_expired(&timer)); static pbio_task_t task; - pbio_task_init(&task, init_task, NULL); - pbio_task_queue_add(task_queue, &task); + start_task(&task, init_task, NULL); - while (true) { - static uint8_t real_write_xfer_size; + for (;;) { + PROCESS_WAIT_UNTIL({ + for (;;) { + pbio_task_t *current_task = list_head(task_queue); - PROCESS_WAIT_UNTIL(spi_srdy || write_xfer_size); + if (!current_task) { + break; + } + + if (current_task->status != PBIO_ERROR_AGAIN || pbio_task_run_once(current_task)) { + // remove the task from the queue only if the task is complete + list_remove(task_queue, current_task); + // then start the next task + continue; + } + + break; + } + + spi_srdy || write_xfer_size; + }); spi_set_mrdy(true); @@ -1777,6 +1788,7 @@ PROCESS_THREAD(pbdrv_bluetooth_spi_process, ev, data) { // to a non-zero value to indicate that write_buf is being used. But we // still need the real size later to figure out the actual size of the // SPI transfer. + static uint8_t real_write_xfer_size; real_write_xfer_size = write_xfer_size; // we can either read, write, or read and write at the same time @@ -1830,11 +1842,6 @@ PROCESS_THREAD(pbdrv_bluetooth_spi_process, ev, data) { handle_event(&read_buf[NPI_SPI_HEADER_LEN + 1]); } } - - pbio_task_queue_run_once(task_queue); - - // clear the event packet type in case tasks are run outside of this loop - read_buf[NPI_SPI_HEADER_LEN] = 0; } PROCESS_END(); @@ -1867,8 +1874,6 @@ HCI_StatusCodes_t HCI_sendHCICommand(uint16_t opcode, uint8_t *pData, uint8_t da hci_command_status = false; hci_command_complete = false; - process_poll(&pbdrv_bluetooth_spi_process); - return bleSUCCESS; }