fix(ble,usb): three bugs from BLE audit

1. USB takeover opcode mismatch
   ZephyrCompanionUSB.cpp checked payload[0] == 0x03 with a comment
   claiming CMD_APP_START, but CMD_APP_START is 0x01 (0x03 is
   CMD_SEND_CHANNEL_TXT_MSG). The USB handshake silently dropped the
   companion app's first frame on every connection; the app appeared
   broken over USB until the user happened to send a channel message.

2. CMD_SET_ADVERT_NAME didn't propagate to BLE adv data
   Name changes were persisted to prefs but the advertising payload
   and GATT device name kept the old value until reboot. Added
   zephcore_ble_update_name() and called it from the handler.

3. No advertising-health watchdog
   If bt_le_adv_start() ever failed transiently (HCI timeout,
   controller pacing), the device would silently stop advertising
   and stay undiscoverable until reboot. Added an adv_running flag
   and a 5s watchdog in the companion housekeeping handler that
   nudges adv back on if it stops outside a connection. Tracks
   Arduino nrf52's equivalent 10s watchdog.
This commit is contained in:
liquidraver
2026-05-20 11:09:45 +02:00
parent 78f0c1c840
commit d7e420bf2f
5 changed files with 82 additions and 2 deletions
+48
View File
@@ -134,6 +134,19 @@ static bool fast_adv_active;
* Cleared by recycled() itself (both run on the cooperative system work queue). */
static bool adv_stop_for_interval_change;
/* Ground truth for "controller is currently broadcasting adv PDUs":
* set TRUE : bt_le_adv_start() returned success
* set FALSE : bt_le_adv_stop() called explicitly (set_enabled(false),
* adv_slow_work interval change, update_name restart)
* set FALSE : a phone connected — Zephyr stops adv internally to consume
* the BT_MAX_CONN=1 slot (no slot left to advertise from).
* Re-set TRUE later when recycled() → start_adv() runs.
* Exposed via zephcore_ble_is_advertising() for the companion advertising
* watchdog (main_companion.cpp housekeeping) that catches transient
* bt_le_adv_start failures. Arduino nrf52 has an equivalent 10s watchdog
* (SerialBLEInterface.cpp:343). */
static bool adv_running;
/* Runtime BLE passkey */
static uint32_t ble_passkey = CONFIG_ZEPHCORE_BLE_PASSKEY;
@@ -283,6 +296,12 @@ static void connected(struct bt_conn *conn, uint8_t err)
LOG_INF("connected: %s", addr);
current_conn = bt_conn_ref(conn);
/* Zephyr stops advertising internally when the conn slot is consumed
* (BT_MAX_CONN=1 — there's no slot left to advertise from). Sync our
* adv_running flag so zephcore_ble_is_advertising() reflects ground
* truth, not just "we last called bt_le_adv_start()". */
adv_running = false;
/* Cancel fast→slow transition — already connected, no need to switch */
k_work_cancel_delayable(&adv_slow_work);
@@ -755,6 +774,7 @@ static void adv_slow_work_fn(struct k_work *work)
fast_adv_active = false;
adv_stop_for_interval_change = true; /* suppress recycled() restart */
bt_le_adv_stop();
adv_running = false;
start_adv();
/* adv_stop_for_interval_change cleared by recycled() on the work queue */
}
@@ -773,9 +793,11 @@ static void start_adv(void)
int err = bt_le_adv_start(&adv_param, ad, ad_len, sd, sd_len);
if (err && err != -EALREADY) {
LOG_ERR("adv start failed: %d", err);
adv_running = false;
} else {
LOG_INF("BLE advertising: %s",
fast_adv_active ? "20ms fast (60s)" : "211ms slow");
adv_running = true;
}
}
@@ -894,6 +916,7 @@ void zephcore_ble_set_enabled(bool enable)
}
/* Stop advertising */
bt_le_adv_stop();
adv_running = false;
LOG_INF("BLE disabled");
} else {
/* Re-enable advertising — start fast window */
@@ -919,6 +942,11 @@ bool zephcore_ble_is_congested(void)
return ble_tx_congested;
}
bool zephcore_ble_is_advertising(void)
{
return adv_running;
}
void zephcore_ble_set_passkey(uint32_t passkey)
{
if (passkey >= 100000 && passkey <= 999999) {
@@ -966,6 +994,26 @@ void zephcore_ble_disconnect(void)
}
}
void zephcore_ble_update_name(const char *new_name)
{
build_device_name_and_adv(new_name);
/* If currently advertising (not connected), restart so the new name
* is published immediately. Restart at fast interval so anyone
* scanning sees the new name quickly. */
if (!current_conn) {
LOG_INF("name updated, restarting adv");
adv_stop_for_interval_change = true; /* suppress recycled() restart */
bt_le_adv_stop();
adv_running = false;
fast_adv_active = true;
k_work_reschedule(&adv_slow_work, K_MSEC(BT_ADV_FAST_DURATION_MS));
start_adv();
}
/* If connected: GATT device name (via bt_set_name in build_device_name_and_adv)
* is live now; advertising payload updates on next adv cycle after disconnect. */
}
void zephcore_ble_conn_params_ready(void)
{
if (!conn_params_pending || !current_conn) {
+17
View File
@@ -52,6 +52,13 @@ bool zephcore_ble_is_connected(void);
/** True if TX queue is full and overflow retry is active. */
bool zephcore_ble_is_congested(void);
/** True if the controller is currently broadcasting advertising PDUs.
* Returns FALSE during an active connection (Zephyr stops adv when the
* BT_MAX_CONN=1 slot is consumed) and FALSE after any explicit stop.
* Companion main loop polls this each housekeeping tick (~5s) and calls
* zephcore_ble_set_enabled(true) if adv ever stops outside a connection. */
bool zephcore_ble_is_advertising(void);
void zephcore_ble_set_passkey(uint32_t passkey);
uint32_t zephcore_ble_get_passkey(void);
@@ -80,6 +87,16 @@ void zephcore_ble_disconnect(void);
*/
void zephcore_ble_conn_params_ready(void);
/**
* Rebuild advertising payload + GATT device name from a new prefs name.
* If currently advertising (no active connection), stops and restarts
* adv so the new name is published immediately. If connected, the new
* payload takes effect on the next adv cycle after disconnect.
*
* Call from CMD_SET_ADVERT_NAME handler after persisting prefs.
*/
void zephcore_ble_update_name(const char *new_name);
#ifdef __cplusplus
}
#endif
+6 -2
View File
@@ -123,8 +123,12 @@ static void usb_rx_work_fn(struct k_work *work)
LOG_DBG("usb_rx: frame complete len=%u hdr=0x%02x", payload_len, payload[0]);
/* Check for CMD_APP_START to switch interface */
if (payload_len >= 1 && payload[0] == 0x03 /* CMD_APP_START */) {
/* Check for CMD_APP_START to switch interface.
* CMD_APP_START is 0x01 — see CompanionMesh.cpp:26.
* (Previously hardcoded 0x03 with the same comment, which
* is actually CMD_SEND_CHANNEL_TXT_MSG and meant the USB
* handshake silently dropped the app's first frame.) */
if (payload_len >= 1 && payload[0] == 0x01 /* CMD_APP_START */) {
if (zephcore_ble_get_active_iface() == ZEPHCORE_IFACE_BLE &&
zephcore_ble_is_connected()) {
LOG_INF("usb_rx: CMD_APP_START, disconnecting BLE");
+2
View File
@@ -1899,6 +1899,8 @@ bool CompanionMesh::handleProtocolFrame(const uint8_t *data, size_t len)
memcpy(prefs.node_name, &data[1], nlen);
prefs.node_name[nlen] = '\0';
_store->savePrefs(prefs);
/* Push the new name to BLE so scanners see it without a reboot. */
zephcore_ble_update_name(prefs.node_name);
}
sendPacketOk();
return true;
+9
View File
@@ -303,6 +303,15 @@ static void mesh_event_loop(void)
companion_mesh_ptr->maintenanceLoop();
}
/* BLE advertising watchdog — if bt_le_adv_start failed
* transiently (HCI timeout, controller pacing) the device
* would silently stop advertising and be undiscoverable
* until next reboot. Cheap to nudge it back here. */
if (!zephcore_ble_is_connected() && !zephcore_ble_is_advertising()) {
LOG_WRN("BLE adv watchdog: not advertising, re-enabling");
zephcore_ble_set_enabled(true);
}
mesh_housekeeping_ui_refresh();
}
#endif