pbio/drv/bluetooth: Broadcast on the Move Hub while connected.

The BlueNRG-MS GAP layer refuses to enter any advertising mode while a
computer is connected, so aci_gap_set_non_connectable() answered
BLE_STATUS_FAILED and nothing was broadcast. The driver mapped that one
error to success so that programs using broadcasting could at least run.

Drive the link layer directly instead, with the set advertising
parameters, set advertising data and set advertise enable sequence from
UM1865 section 3.3. The chip is already configured for role mode 4,
which supports advertising alongside connections and scanning, and it
accepts all three commands while a computer is connected. Asking the
chip first was no help: HCI_LE_Read_Supported_States answers with bit 20
clear, but it also claims not to support advertising while scanning and
not to support scanning while a slave, both of which demonstrably work,
so its answer understates what this stack can do.

Advertising is now enabled behind the GAP layer's back, so stopping has
to disable it the same way, and the discoverable mode has to be left
before the parameters can be set. The link layer advertises exactly the
payload it is given, so the service UUID and TX power AD types left over
from discovery no longer have to be deleted to make room. Broadcasts
therefore no longer carry the Flags AD type, which is what the BTstack
hubs already send.

The advertising interval has to be requested as a range. The chip fits a
new activity to the anchor period it is already running for the
connection and for any scanning, and answers 0x85
BLE_STATUS_INTERVAL_TOO_LARGE when no multiple of that period falls
between the requested minimum and maximum. A fixed 100ms therefore
failed the moment the hub was also observing or connected, which is
precisely the case being fixed here. The GAP layer picked its own range
internally and so never ran into it. 100ms is the fastest this Bluetooth
4.1 controller allows for non-connectable advertising and is the bottom
of the range; 200ms at the top measures the same as the GAP layer
managed, 2.45 against 2.53 broadcasts a second reaching a third hub,
which is inside the run to run spread.

Drop the note about broadcasting interfering with observing as well.
Measured on a Move Hub with the 50% scan duty cycle, reception is
unchanged by broadcasting at the same time: 13.4 advertisements a second
from a reference transmitter either way, with the same distribution of
gaps. That measurement depends on the duty cycle, so this belongs after
that change.

Fixes pybricks/support#2826

Author: dlech

CHANGELOG.md

@@ -13,8 +13,10 @@
 - Fixed slow broadcasting while observing at the same time ([support#2822]).
 - Fixed slow observing while connected to a computer or app ([support#2822]).
 - Fixed connected remote preventing Move Hub from advertising.
+- Fixed Move Hub not broadcasting while connected to a PC ([support#2826]).
 
 [support#2822]: https://github.com/orgs/pybricks/discussions/2822
+[support#2826]: https://github.com/pybricks/support/issues/2826
 
 [Unreleased]: https://github.com/pybricks/pybricks-micropython/compare/v4.1.0b3...HEAD
 

lib/pbio/drv/bluetooth/bluetooth_stm32_bluenrg.c

@@ -236,6 +236,25 @@ const char *pbdrv_bluetooth_get_fw_version(void) {
     return pbdrv_bluetooth_fw_version;
 }
 
+/**
+ * Sends a link layer command with constant parameters.
+ *
+ * The BlueNRG library's hci_le_*() functions for these commands wait for the
+ * response, which this driver cannot do, so the command is packed here.
+ *
+ * @param [in]  ocf     The opcode command field.
+ * @param [in]  params  The command parameters.
+ * @param [in]  plen    The size of @p params in bytes.
+ */
+static void hci_send_le_command(uint16_t ocf, const uint8_t *params, uint8_t plen) {
+    struct hci_request rq = {
+        .opcode = cmd_opcode_pack(OGF_LE_CTL, ocf),
+        .cparam = (void *)params,
+        .clen = plen,
+    };
+    hci_send_req(&rq);
+}
+
 /**
  * Sets advertising data and enables advertisements.
  */
@@ -292,10 +311,21 @@ pbio_error_t pbdrv_bluetooth_stop_advertising_func(pbio_os_state_t *state, void
 
     // REVISIT: might need to delete advertising data here
 
-    PBIO_OS_AWAIT_WHILE(state, write_xfer_size);
-    aci_gap_set_non_discoverable_begin();
-    PBIO_OS_AWAIT_UNTIL(state, hci_command_complete);
-    // aci_gap_set_non_discoverable_end();
+    if (pbdrv_bluetooth_advertising_state == PBDRV_BLUETOOTH_ADVERTISING_STATE_BROADCASTING) {
+        // The GAP layer does not know about advertising that was enabled with
+        // a link layer command, so it has to be stopped the same way.
+        PBIO_OS_AWAIT_WHILE(state, write_xfer_size);
+        {
+            static const uint8_t disable = 0;
+            hci_send_le_command(OCF_LE_SET_ADVERTISE_ENABLE, &disable, sizeof(disable));
+        }
+        PBIO_OS_AWAIT_UNTIL(state, hci_command_complete);
+    } else {
+        PBIO_OS_AWAIT_WHILE(state, write_xfer_size);
+        aci_gap_set_non_discoverable_begin();
+        PBIO_OS_AWAIT_UNTIL(state, hci_command_complete);
+        // aci_gap_set_non_discoverable_end();
+    }
 
     // This protothread is also shared with stop broadcasting. Either way,
     // nothing is advertising or broadcasting after this, so reset that state.
@@ -599,49 +629,72 @@ pbio_error_t pbdrv_bluetooth_start_broadcasting_func(pbio_os_state_t *state, voi
 
     PBIO_OS_ASYNC_BEGIN(state);
 
+    // The GAP layer refuses to enter any advertising mode while a computer is
+    // connected, so the link layer is driven directly instead.
     if (pbdrv_bluetooth_advertising_state != PBDRV_BLUETOOTH_ADVERTISING_STATE_BROADCASTING) {
-        PBIO_OS_AWAIT_WHILE(state, write_xfer_size);
-        // This chip cannot broadcast faster than every 100ms, so other hubs
-        // receive from it more slowly than they do from each other.
-        aci_gap_set_non_connectable_begin(ADV_NONCONN_IND, STATIC_RANDOM_ADDR);
-        PBIO_OS_AWAIT_UNTIL(state, hci_command_complete);
-        status = aci_gap_set_non_connectable_end();
 
-        if (status != BLE_STATUS_SUCCESS) {
-            pbio_error_t err = ble_error_to_pbio_error(status);
-            // Broadcasting does not work while connected to the computer. But
-            // returning an error means that Move Hub programs with
-            // broadcasting can never run while connected, which makes it very
-            // impractical to test any program. So mark as success.
-            if (err == PBIO_ERROR_INVALID_OP) {
-                return PBIO_SUCCESS;
-            }
-            return err;
+        // Advertising parameters can only be set while advertising is stopped,
+        // so leave the GAP discoverable mode if it is still active.
+        if (pbdrv_bluetooth_advertising_state == PBDRV_BLUETOOTH_ADVERTISING_STATE_ADVERTISING_PYBRICKS) {
+            PBIO_OS_AWAIT_WHILE(state, write_xfer_size);
+            aci_gap_set_non_discoverable_begin();
+            PBIO_OS_AWAIT_UNTIL(state, hci_command_complete);
+            // aci_gap_set_non_discoverable_end();
         }
 
-        // These AD types are left over from connectable discovery and need
-        // to be deleted _after_ starting non-connectable advertising.
-
         PBIO_OS_AWAIT_WHILE(state, write_xfer_size);
-        aci_gap_delete_ad_type_begin(AD_TYPE_128_BIT_SERV_UUID);
+        {
+            static const uint8_t params[] = {
+                // 100ms is the fastest this controller allows. The range has
+                // to be wide, because the chip fits a new activity to the
+                // anchor period it is already running for the connection and
+                // any scanning, and rejects an interval that is not a multiple
+                // of it.
+                0xa0, 0x00, // Advertising_Interval_Min, 100ms
+                0x40, 0x01, // Advertising_Interval_Max, 200ms
+                ADV_NONCONN_IND,
+                STATIC_RANDOM_ADDR,
+                0, // Peer_Address_Type
+                0, 0, 0, 0, 0, 0, // Peer_Address
+                0x07, // Advertising_Channel_Map
+                NO_WHITE_LIST_USE,
+            };
+            hci_send_le_command(OCF_LE_SET_ADV_PARAMETERS, params, sizeof(params));
+        }
         PBIO_OS_AWAIT_UNTIL(state, hci_command_complete);
+        status = hci_le_command_end();
 
-        PBIO_OS_AWAIT_WHILE(state, write_xfer_size);
-        aci_gap_delete_ad_type_begin(AD_TYPE_TX_POWER_LEVEL);
-        PBIO_OS_AWAIT_UNTIL(state, hci_command_complete);
+        if (status != BLE_STATUS_SUCCESS) {
+            return ble_error_to_pbio_error(status);
+        }
+    }
 
-        // Errors from deleting are ignored since we should only get an error
-        // if the AD does not exist, which is OK.
+    // The link layer advertises exactly this payload.
+    PBIO_OS_AWAIT_WHILE(state, write_xfer_size);
+    hci_le_set_advertising_data_begin(pbdrv_bluetooth_broadcast_data_size, pbdrv_bluetooth_broadcast_data);
+    PBIO_OS_AWAIT_UNTIL(state, hci_command_complete);
+    status = hci_le_set_advertising_data_end();
 
-        pbdrv_bluetooth_advertising_state = PBDRV_BLUETOOTH_ADVERTISING_STATE_BROADCASTING;
+    if (status != BLE_STATUS_SUCCESS) {
+        return ble_error_to_pbio_error(status);
     }
 
-    // This has to be done _after_ other data is delete to make sure it fits.
+    if (pbdrv_bluetooth_advertising_state == PBDRV_BLUETOOTH_ADVERTISING_STATE_BROADCASTING) {
+        // Already advertising, so updating the data was all that was needed.
+        return PBIO_SUCCESS;
+    }
 
     PBIO_OS_AWAIT_WHILE(state, write_xfer_size);
-    aci_gap_update_adv_data_begin(pbdrv_bluetooth_broadcast_data_size, pbdrv_bluetooth_broadcast_data);
+    {
+        static const uint8_t enable = 1;
+        hci_send_le_command(OCF_LE_SET_ADVERTISE_ENABLE, &enable, sizeof(enable));
+    }
     PBIO_OS_AWAIT_UNTIL(state, hci_command_complete);
-    status = aci_gap_update_adv_data_end();
+    status = hci_le_command_end();
+
+    if (status == BLE_STATUS_SUCCESS) {
+        pbdrv_bluetooth_advertising_state = PBDRV_BLUETOOTH_ADVERTISING_STATE_BROADCASTING;
+    }
 
     PBIO_OS_ASYNC_END(ble_error_to_pbio_error(status));
 }

pybricks/messaging/pb_type_ble_radio.c

@@ -275,9 +275,6 @@ static mp_obj_t pb_module_ble_broadcast(size_t n_args, const mp_obj_t *pos_args,
     PB_PARSE_ARGS_METHOD(n_args, pos_args, kw_args,
         pb_obj_BLE_t, self,
         PB_ARG_REQUIRED(data));
-    // On Move Hub, nothing is broadcast if it is called while the
-    // move hub is connected to Pybricks Code. Also, broadcasting interferes
-    // with observing even when not connected to Pybricks Code.
 
     mp_obj_t self_in = MP_OBJ_FROM_PTR(self);