proper timeouts for ble write, now working.

include/config.h

@@ -19,6 +19,8 @@
 #ifndef __CONFIG_H__
 #define __CONFIG_H__
 
+#define WATCHDOG_PERIOD_MS 1000
+
 // ASCII 0x18 = CAN (cancel)
 #define ENTER_BOOTLOADER_MAGIC 0x18
 

src/ble.c

@@ -20,12 +20,17 @@
  */
 
 #include "pico/cyw43_arch.h"
+#include "hardware/watchdog.h"
 
 #include "config.h"
 #include "log.h"
 #include "util.h"
 #include "ble.h"
 
+#define BLE_READ_TIMEOUT_MS 500
+#define BLE_SRVC_TIMEOUT_MS 500
+#define BLE_CHAR_TIMEOUT_MS 2000
+#define BLE_WRTE_TIMEOUT_MS 500
 #define BLE_MAX_SCAN_AGE_MS (10 * 1000)
 #define BLE_MAX_SERVICES 8
 #define BLE_MAX_CHARACTERISTICS 8
@@ -485,10 +490,18 @@ int32_t ble_read(const uint8_t *characteristic, uint8_t *buff, uint16_t buff_len
     read_len = 0;
     cyw43_thread_exit();
 
+    uint32_t start_time = to_ms_since_boot(get_absolute_time());
     while (1) {
         sleep_ms(1);
 
-        // TODO timeout
+        uint32_t now = to_ms_since_boot(get_absolute_time());
+        if ((now - start_time) >= BLE_READ_TIMEOUT_MS) {
+            debug("timeout waiting for read");
+            return -3;
+        }
+#if BLE_READ_TIMEOUT_MS >= (WATCHDOG_PERIOD_MS / 2)
+        watchdog_update();
+#endif
 
         cyw43_thread_enter();
         enum ble_state state_cached = state;
@@ -506,7 +519,7 @@ int32_t ble_read(const uint8_t *characteristic, uint8_t *buff, uint16_t buff_len
     if (read_len > buff_len) {
         debug("buffer too short (%d < %d)", buff_len, read_len);
         cyw43_thread_exit();
-        return -3;
+        return -4;
     }
 
     memcpy(buff, data_buff, read_len);
@@ -566,10 +579,19 @@ int8_t ble_write(const uint8_t *service, const uint8_t *characteristic,
         cyw43_thread_exit();
 
         debug("waiting for service discovery");
+
+        uint32_t start_time = to_ms_since_boot(get_absolute_time());
         while (1) {
             sleep_ms(1);
 
-            // TODO timeout
+            uint32_t now = to_ms_since_boot(get_absolute_time());
+            if ((now - start_time) >= BLE_SRVC_TIMEOUT_MS) {
+                debug("timeout waiting for service");
+                return -3;
+            }
+#if BLE_SRVC_TIMEOUT_MS >= (WATCHDOG_PERIOD_MS / 2)
+            watchdog_update();
+#endif
 
             cyw43_thread_enter();
             enum ble_state state_cached = state;
@@ -580,6 +602,11 @@ int8_t ble_write(const uint8_t *service, const uint8_t *characteristic,
                 break;
             }
         }
+
+        // allow some time for service discovery
+        watchdog_update();
+
+        cyw43_thread_enter();
     }
 
     // check if characteristic has already been discovered
@@ -616,7 +643,7 @@ int8_t ble_write(const uint8_t *service, const uint8_t *characteristic,
         if (r != ERROR_CODE_SUCCESS) {
             cyw43_thread_exit();
             debug("gatt characteristic discovery failed %d", r);
-            return -3;
+            return -4;
         }
 
         state = TC_W4_CHARACTERISTIC;
@@ -624,10 +651,19 @@ int8_t ble_write(const uint8_t *service, const uint8_t *characteristic,
         cyw43_thread_exit();
 
         debug("waiting for characteristic discovery");
+
+        uint32_t start_time = to_ms_since_boot(get_absolute_time());
         while (1) {
             sleep_ms(1);
 
-            // TODO timeout
+            uint32_t now = to_ms_since_boot(get_absolute_time());
+            if ((now - start_time) >= BLE_CHAR_TIMEOUT_MS) {
+                debug("timeout waiting for characteristic");
+                return -5;
+            }
+#if BLE_CHAR_TIMEOUT_MS >= (WATCHDOG_PERIOD_MS / 2)
+            watchdog_update();
+#endif
 
             cyw43_thread_enter();
             enum ble_state state_cached = state;
@@ -638,6 +674,11 @@ int8_t ble_write(const uint8_t *service, const uint8_t *characteristic,
                 break;
             }
         }
+
+        // allow some time for characteristic discovery
+        watchdog_update();
+
+        cyw43_thread_enter();
     }
 
     if (buff_len > BLE_MAX_VALUE_LEN) {
@@ -652,17 +693,26 @@ int8_t ble_write(const uint8_t *service, const uint8_t *characteristic,
     if (r != ERROR_CODE_SUCCESS) {
         cyw43_thread_exit();
         debug("gatt write failed %d", r);
-        return -4;
+        return -6;
     }
 
     state = TC_W4_WRITE;
     cyw43_thread_exit();
 
     debug("waiting for write");
+
+    uint32_t start_time = to_ms_since_boot(get_absolute_time());
     while (1) {
         sleep_ms(1);
 
-        // TODO timeout
+        uint32_t now = to_ms_since_boot(get_absolute_time());
+        if ((now - start_time) >= BLE_WRTE_TIMEOUT_MS) {
+            debug("timeout waiting for write");
+            return -7;
+        }
+#if BLE_WRTE_TIMEOUT_MS >= (WATCHDOG_PERIOD_MS / 2)
+        watchdog_update();
+#endif
 
         cyw43_thread_enter();
         enum ble_state state_cached = state;
@@ -676,7 +726,7 @@ int8_t ble_write(const uint8_t *service, const uint8_t *characteristic,
 
     cyw43_thread_enter();
 
-    int8_t ret = (state == TC_WRITE_COMPLETE) ? 0 : -1;
+    int8_t ret = (state == TC_WRITE_COMPLETE) ? 0 : -8;
     state = TC_READY;
 
     cyw43_thread_exit();

src/main.c

@@ -76,8 +76,7 @@ int main(void) {
     debug("debug_disk_init");
     debug_disk_init();
 
-    // trigger after 1000ms
-    watchdog_enable(1000, 1);
+    watchdog_enable(WATCHDOG_PERIOD_MS, 1);
 
     debug("init done");
     lcd_set_backlight(0x8000);

src/serial.c

@@ -52,6 +52,19 @@ static struct ring_buffer tx = RB_INIT(tx_buff, sizeof(tx_buff));
 
 static bool reroute_serial_debug = false;
 static bool tx_irq_state = false;
+static bool rx_irq_state = false;
+#define SET_TX_IRQ(v) {                 \
+    tx_irq_state = v;                   \
+    uart_set_irq_enables(UART_ID,       \
+                         rx_irq_state,  \
+                         tx_irq_state); \
+}
+#define SET_RX_IRQ(v) {                 \
+    rx_irq_state = v;                   \
+    uart_set_irq_enables(UART_ID,       \
+                         rx_irq_state,  \
+                         tx_irq_state); \
+}
 
 static void serial_irq(void) {
     // Rx - read from UART FIFO to local buffer
@@ -65,8 +78,7 @@ static void serial_irq(void) {
         if (rb_len(&tx) > 0) {
             uart_putc_raw(UART_ID, rb_pop(&tx));
         } else {
-            uart_set_irq_enables(UART_ID, true, false);
-            tx_irq_state = false;
+            SET_TX_IRQ(false);
             break;
         }
     }
@@ -86,13 +98,12 @@ void serial_init(void) {
     irq_set_exclusive_handler(UART_IRQ, serial_irq);
     irq_set_enabled(UART_IRQ, true);
 
-    uart_set_irq_enables(UART_ID, true, false);
-    tx_irq_state = false;
+    SET_RX_IRQ(true);
+    SET_TX_IRQ(false);
 }
 
 void serial_write(const uint8_t *buf, size_t count) {
-    uart_set_irq_enables(UART_ID, true, false);
-    tx_irq_state = false;
+    SET_TX_IRQ(false);
 
     while ((rb_len(&tx) == 0) && uart_is_writable(UART_ID) && (count > 0)) {
         uart_putc_raw(UART_ID, *buf++);
@@ -112,21 +123,17 @@ void serial_write(const uint8_t *buf, size_t count) {
         count -= space;
         off += space;
 
-        uart_set_irq_enables(UART_ID, true, true);
-        tx_irq_state = true;
+        SET_TX_IRQ(true);
 
         sleep_ms(1);
 
-        uart_set_irq_enables(UART_ID, true, false);
-        tx_irq_state = false;
+        SET_TX_IRQ(false);
     }
-
 #endif // SERIAL_WRITES_BLOCK_WHEN_BUFFER_FULL
 
     rb_add(&tx, buf + off, count);
 
-    uart_set_irq_enables(UART_ID, true, true);
-    tx_irq_state = true;
+    SET_TX_IRQ(true);
 }
 
 void serial_set_reroute(bool reroute) {
@@ -134,9 +141,8 @@ void serial_set_reroute(bool reroute) {
 }
 
 void serial_run(void) {
-    uart_set_irq_enables(UART_ID, false, tx_irq_state);
+    SET_RX_IRQ(false);
 
-    // Rx - pass local buffer to further processing
     if (rb_len(&rx) >= 1) {
         if (rb_peek(&rx) == ENTER_BOOTLOADER_MAGIC) {
             reset_to_bootloader();
@@ -147,5 +153,5 @@ void serial_run(void) {
         }
     }
 
-    uart_set_irq_enables(UART_ID, true, tx_irq_state);
+    SET_RX_IRQ(true);
 }