Merge branch 'reset-ble-2' into staging/ble

Author: benma

src/reset.c

@@ -48,18 +48,18 @@ static void _show_reset_label(bool status)
 }
 #endif
 
-#if !defined(TESTING)
-static void _ble_reset(void)
+void reset_ble(void)
 {
+#if !defined(TESTING)
     struct ringbuffer uart_queue;
     uint8_t uart_queue_buf[64];
     ringbuffer_init(&uart_queue, &uart_queue_buf[0], sizeof(uart_queue_buf));
     da14531_reset(&uart_queue);
     while (ringbuffer_num(&uart_queue)) {
         uart_poll(NULL, 0, NULL, &uart_queue);
     }
-}
 #endif
+}
 
 void reset_reset(bool status)
 {
@@ -98,7 +98,7 @@ void reset_reset(bool status)
 
     // The ble chip needs to be restarted to load the new secrets.
     if (memory_get_platform() == MEMORY_PLATFORM_BITBOX02_PLUS) {
-        _ble_reset();
+        reset_ble();
     }
 
     reboot();

src/reset.h

@@ -17,6 +17,12 @@
 
 #include <stdbool.h>
 
+/**
+ * Restarts the Bluetooth chip. This also means it will re-load the Bluetooth firmware from SPI
+ * memory.
+ */
+void reset_ble(void);
+
 /**
  * Resets the device:
  * - Updates secure chip KDF keys.

src/rust/bitbox02-rust/src/async_usb.rs

@@ -101,6 +101,9 @@ where
 
             *state = UsbTaskState::Running(Some(task), WaitingForNextRequestState::Idle);
         }
+        // This panic could happen e.g. if someone reconnects to the BitBox while a task is running,
+        // before the 500ms timeout cancels the task. The proper way to handle would be to let the
+        // host know we are busy so the host can re-retry after some time.
         _ => panic!("spawn: wrong state"),
     }
 }
@@ -149,7 +152,13 @@ pub fn spin() {
         _ => None,
     };
     if let Some(ref mut task) = popped_task {
-        match spin_task(task) {
+        let spin_result = spin_task(task);
+        if matches!(*USB_TASK_STATE.0.borrow(), UsbTaskState::Nothing) {
+            // The task was cancelled while it was running, so there is nothing to do with the
+            // result.
+            return;
+        }
+        match spin_result {
             Poll::Ready(result) => {
                 *USB_TASK_STATE.0.borrow_mut() = UsbTaskState::ResultAvailable(result);
             }
@@ -208,6 +217,11 @@ pub fn copy_response(dst: &mut [u8]) -> Result<usize, CopyResponseErr> {
 
 /// Cancel and drop a running task. Returns true if a task was cancelled, false if no task was
 /// running.
+///
+/// Call this inside a running task only if you expect that the host may not be able to read the
+/// result (e.g. when resetting the BLE chip as part of a task), so another task can spawn
+/// afterwards immediately (before the timeout auto-cancles it), which currently would run into a
+/// panic until the response was read and the current task concluded. See the comment in `spawn()`
 pub fn cancel() -> bool {
     let mut state = USB_TASK_STATE.0.borrow_mut();
     if let UsbTaskState::Running(_, _) = *state {

src/rust/bitbox02-rust/src/hww/api/bluetooth.rs

@@ -163,6 +163,13 @@ async fn process_upgrade(
 
     if response.is_ok() {
         hal.ui().status("Upgrade\nsuccessful", true).await;
+        bitbox02::reset_ble();
+        if bitbox02::communication_mode_ble_enabled() {
+            // Since the Bluetooth host will not be there anymore to read this response, this task
+            // will not be cleared by the executor. We do it manually to make space for the next
+            // task upon reconnection.
+            crate::async_usb::cancel();
+        }
     } else {
         hal.ui().status("Upgrade failed", false).await;
     }

src/rust/bitbox02-sys/build.rs

@@ -120,6 +120,7 @@ const ALLOWLIST_FNS: &[&str] = &[
     "random_mock_reset",
     "reboot_to_bootloader",
     "reset_reset",
+    "reset_ble",
     "screen_print_debug",
     "screen_process",
     "screen_saver_disable",

src/rust/bitbox02/src/lib.rs

@@ -113,6 +113,10 @@ pub fn reset(status: bool) {
     unsafe { bitbox02_sys::reset_reset(status) }
 }
 
+pub fn reset_ble() {
+    unsafe { bitbox02_sys::reset_ble() }
+}
+
 pub struct Tm {
     tm: bitbox02_sys::tm,
 }