pbio/drv/usb: Restore subscribe message.

We dropped this when we converted to serial because the DTR serves a similar purpose and is more reliable, especially when it comes to determining disconnecting, which subscribe could only do explicitly, not implicitly if the host closed.

But DTR is immediately asserted by most hosts, which is not ideal for the initial handshake, as the hub begins sending events right away.

This restores subscribe so we get the best of both: subscribe gates the event flood but DTR takes care of automatically deasserting it on the hub side if it becomes disconnected.

Author: laurensvalk

lib/pbio/drv/usb/usb.c

@@ -39,13 +39,25 @@ void pbdrv_usb_set_host_connection_changed_callback(pbio_util_void_callback_t ca
 }
 
 /**
- * Whether the host has opened the serial port (DTR asserted). This is the USB
- * analog of a BLE host subscribing to notifications.
+ * Whether the host has opened the serial port (DTR asserted). This detects
+ * disconnection gracefully, even if the host abruptly goes away (e.g. the
+ * browser tab is closed) without unsubscribing.
  */
 static bool pbdrv_usb_dtr;
 
+/**
+ * Whether the host is subscribed to our outgoing event messages. This is the
+ * USB analog of a BLE host subscribing to notifications.
+ *
+ * Unlike DTR, this is asserted explicitly by the host application rather than
+ * automatically by the OS when the port is opened. It gates the initial event
+ * flood so that we don't bombard the host with events the moment DTR is
+ * (possibly automatically) asserted.
+ */
+static bool pbdrv_usb_subscribed;
+
 bool pbdrv_usb_connection_is_active(void) {
-    return pbdrv_usb_is_ready() && pbdrv_usb_dtr;
+    return pbdrv_usb_is_ready() && pbdrv_usb_dtr && pbdrv_usb_subscribed;
 }
 
 //
@@ -226,10 +238,35 @@ void pbdrv_usb_on_dtr_changed(bool dtr) {
     pbdrv_usb_rx_frame_len = 0;
     pbdrv_usb_rx_overflow = false;
 
-    if (dtr) {
-        // Host just opened the port. Send the current status right away, like
-        // the first notification after a BLE host subscribes. Device info is
-        // not pushed; the host reads it on demand via read requests.
+    if (!dtr) {
+        // Host closed the port. The subscription implicitly falls with DTR. In
+        // practice the host rarely unsubscribes explicitly; it just lets DTR
+        // drop, which we detect even if the host abruptly goes away.
+        pbdrv_usb_subscribed = false;
+    }
+
+    if (pbdrv_usb_host_connection_changed_callback) {
+        pbdrv_usb_host_connection_changed_callback();
+    }
+
+    pbio_os_request_poll();
+}
+
+/**
+ * Sets whether the host is subscribed to event notifications and notifies
+ * listeners of the connection state change.
+ */
+static void pbdrv_usb_set_subscribed(bool subscribed) {
+    if (subscribed == pbdrv_usb_subscribed) {
+        return;
+    }
+
+    pbdrv_usb_subscribed = subscribed;
+
+    if (subscribed) {
+        // Host just subscribed. Send the current status right away, like the
+        // first notification after a BLE host subscribes. Device info is not
+        // pushed; the host reads it on demand via read requests.
         pbdrv_usb_status_data_pending = true;
     }
 
@@ -444,7 +481,11 @@ static void pbdrv_usb_handle_data_in(void) {
 
             // The first byte is the host-to-hub message type and the rest is
             // its payload.
-            if (msg_size >= 2 && msg[0] == PBIO_PYBRICKS_OUT_EP_MSG_COMMAND && pbdrv_usb_receive_handler) {
+            if (msg_size >= 2 && msg[0] == PBIO_PYBRICKS_OUT_EP_MSG_SUBSCRIBE) {
+                // Subscribe or unsubscribe to event notifications. The payload
+                // is a single byte: 1 to subscribe, 0 to unsubscribe.
+                pbdrv_usb_set_subscribed(msg[1]);
+            } else if (msg_size >= 2 && msg[0] == PBIO_PYBRICKS_OUT_EP_MSG_COMMAND && pbdrv_usb_receive_handler) {
                 // Compute the response synchronously and queue it immediately.
                 pbio_set_uint32_le(&pbdrv_usb_command_response_buf[1],
                     pbdrv_usb_receive_handler(&msg[1], msg_size - 1));
@@ -475,6 +516,7 @@ static void pbdrv_usb_handle_data_in(void) {
 
 static void pbdrv_usb_reset_state(void) {
     pbdrv_usb_on_dtr_changed(false);
+    pbdrv_usb_subscribed = false;
     pbdrv_usb_command_response_pending = false;
     pbdrv_usb_read_reply_pending = false;
     pbdrv_usb_status_data_pending = false;

lib/pbio/drv/usb/usb_simulation.c

@@ -97,6 +97,14 @@ static pbio_error_t pbdrv_usb_test_process_thread(pbio_os_state_t *state, void *
 
     PBIO_OS_ASYNC_BEGIN(state);
 
+    // Fake a subscribe message so the common driver starts sending events,
+    // just as a real host would after opening the port. It is COBS-encoded
+    // like real host traffic.
+    static const uint8_t subscribe_msg[] = {
+        PBIO_PYBRICKS_OUT_EP_MSG_SUBSCRIBE, 1,
+    };
+    usb_in_size = pbdrv_usb_cobs_encode(subscribe_msg, sizeof(subscribe_msg), usb_in_buf);
+
     #ifdef PBDRV_CONFIG_RUN_ON_CI
     // CI and MicroPython test suite have lots of problems with stdin. It is
     // only needed for the REPL and interactive input, so don't bother on CI.
@@ -130,7 +138,8 @@ void pbdrv_usb_init_device(void) {
     static pbio_os_process_t pbdrv_usb_test_process;
     pbio_os_process_start(&pbdrv_usb_test_process, pbdrv_usb_test_process_thread, NULL);
 
-    // No physical port to open, so report the connection as active right away.
+    // No physical port to open, so assert DTR right away. The process thread
+    // also fakes a subscribe message, so the connection becomes active.
     pbdrv_usb_on_dtr_changed(true);
 }
 

lib/pbio/drv/usb/usb_simulation_pico.c

@@ -110,6 +110,15 @@ uint32_t pbdrv_usb_get_data_and_start_receive(uint8_t *data) {
 static pbio_error_t pbdrv_usb_test_process_thread(pbio_os_state_t *state, void *context) {
     PBIO_OS_ASYNC_BEGIN(state);
 
+    // Fake a subscribe message so the common driver starts sending events,
+    // just as a real host would after opening the port. It is COBS-encoded
+    // like real host traffic.
+    static const uint8_t subscribe_msg[] = {
+        PBIO_PYBRICKS_OUT_EP_MSG_SUBSCRIBE, 1,
+    };
+    pbdrv_usb_simulation_pico_in_size = pbdrv_usb_cobs_encode(
+        subscribe_msg, sizeof(subscribe_msg), pbdrv_usb_simulation_pico_in_buf);
+
     for (;;) {
         static size_t available;
         PBIO_OS_AWAIT_UNTIL(state, pbdrv_usb_simulation_pico_in_size == 0 &&
@@ -160,8 +169,8 @@ void pbdrv_usb_init_device(void) {
     static pbio_os_process_t pbdrv_usb_test_process;
     pbio_os_process_start(&pbdrv_usb_test_process, pbdrv_usb_test_process_thread, NULL);
 
-    // No physical port to detect, so report the host connection as active
-    // right away (USB analog of a BLE host subscribing).
+    // No physical port to detect, so assert DTR right away. The process thread
+    // also fakes a subscribe message, so the connection becomes active.
     pbdrv_usb_on_dtr_changed(true);
 }
 

lib/pbio/include/pbio/protocol.h

@@ -513,7 +513,7 @@ typedef enum {
      * error code, analogous to a BLE write response.
      */
     PBIO_PYBRICKS_IN_EP_MSG_RESPONSE = 1,
-    /** Analog to BLE notification. Emitted while a host is connected. */
+    /** Analog to BLE notification. Only emitted if subscribed. */
     PBIO_PYBRICKS_IN_EP_MSG_EVENT = 2,
     /**
      * Reply to a ::PBIO_PYBRICKS_OUT_EP_MSG_READ. The payload is
@@ -532,19 +532,29 @@ typedef enum {
  */
 typedef enum {
     /**
-     * A characteristic read request. The payload is
-     * `[service, char_id_lo, char_id_hi]`, where service is one of the
-     * ::PBIO_PYBRICKS_USB_INTERFACE_READ_CHARACTERISTIC_GATT values. The hub
-     * replies with a ::PBIO_PYBRICKS_IN_EP_MSG_READ_REPLY. This is the serial
-     * analog of a BLE host reading a characteristic by UUID.
+     * Subscribe to events. The payload is a single byte: 1 to subscribe, 0 to
+     * unsubscribe. The hub will only send events if the host is subscribed.
+     *
+     * Analog of BLE Client Characteristic Configuration Descriptor (CCCD).
      */
-    PBIO_PYBRICKS_OUT_EP_MSG_READ = 1,
+    PBIO_PYBRICKS_OUT_EP_MSG_SUBSCRIBE = 1,
     /**
      * A Pybricks command (see ::pbio_pybricks_command_t), carried in the
      * remaining payload bytes. The hub replies with a
      * ::PBIO_PYBRICKS_IN_EP_MSG_RESPONSE.
+     *
+     * Analog of BLE Client Characteristic Write with response.
      */
     PBIO_PYBRICKS_OUT_EP_MSG_COMMAND = 2,
+    /**
+     * A characteristic read request. The payload is
+     * `[service, char_id_lo, char_id_hi]`, where service is one of the
+     * ::PBIO_PYBRICKS_USB_INTERFACE_READ_CHARACTERISTIC_GATT values. The hub
+     * replies with a ::PBIO_PYBRICKS_IN_EP_MSG_READ_REPLY.
+     *
+     * Analog of a BLE host reading a characteristic by UUID.
+     */
+    PBIO_PYBRICKS_OUT_EP_MSG_READ = 3,
 } pbio_pybricks_usb_out_ep_msg_t;
 
 #endif // _PBIO_PROTOCOL_H_