extmod/zephyr_ble: Unregister connection callbacks in deinit.

Add bt_conn_cb_unregister() call in mp_bluetooth_deinit() to properly
clean up connection callbacks when the BLE stack is deactivated.

PROBLEM:
The static connection callback structure mp_bt_zephyr_conn_callbacks
persists across bt_disable()/bt_enable() cycles. Zephyr's bt_disable()
does not automatically clear registered callbacks, causing them to
accumulate on each reinitialization. While this doesn't directly cause
crashes (the callbacks are function pointers that remain valid), it
violates the lifecycle contract and could lead to unexpected behavior
if the callback structure were ever relocated or modified.

LIFECYCLE ISSUE:
- mp_bluetooth_init() calls bt_conn_cb_register(&mp_bt_zephyr_conn_callbacks)
- mp_bluetooth_deinit() calls bt_disable() but never unregisters callbacks
- Next mp_bluetooth_init() attempts to register the same callbacks again
- Without dynamic callback support (CONFIG_BT_CONN_DYNAMIC_CALLBACKS=1),
  this would cause registration failures

With CONFIG_BT_CONN_DYNAMIC_CALLBACKS=1 enabled in our configuration,
Zephyr maintains a dynamic list of registered callbacks and silently
allows re-registration of the same callback structure. However, this
is still semantically incorrect and relies on implementation details.

SOLUTION:
Call bt_conn_cb_unregister(&mp_bt_zephyr_conn_callbacks) during
mp_bluetooth_deinit() to properly clean up the callbacks before
bt_disable() is called. This ensures clean state across init/deinit
cycles and follows proper lifecycle management.

The unregister operation is performed after stopping advertising and
scanning but before calling bt_disable(), ensuring callbacks are
removed while the BLE stack is still active and can properly process
the unregistration.

CALL SEQUENCE IN mp_bluetooth_deinit():
1. mp_bluetooth_gap_advertise_stop() - Stop advertising
2. mp_bluetooth_gattc_disconnect_all() - Disconnect clients
3. mp_bluetooth_gatts_deinit() - Unregister GATT services
4. mp_bluetooth_gap_scan_stop() - Stop scanning
5. bt_conn_cb_unregister() - Unregister callbacks (NEW)
6. bt_disable() - Shutdown BLE stack

This ordering ensures all BLE operations are stopped before callback
unregistration, and the stack is still active when unregistering.

TESTING:
Verified on STM32WB55 with soft reset cycles:
- 3 consecutive bt_enable() → bt_disable() cycles
- No callback-related errors or warnings
- Clean reinitialization on each cycle

This change is required for proper soft reset support but does not
fix the reference counting crash (that's fixed in the Zephyr submodule
commit). Both changes together enable full soft reset functionality.

Signed-off-by: Andrew Leech <[email protected]>

Author: pi-anl

extmod/zephyr_ble/modbluetooth_zephyr.c

@@ -217,12 +217,14 @@ static void mp_bt_zephyr_remove_connection(uint8_t conn_handle) {
 
 static void mp_bt_zephyr_connected(struct bt_conn *conn, uint8_t err) {
     // CRITICAL: This printf MUST appear if callback is being called by Zephyr
-    mp_printf(&mp_plat_print, ">>> mp_bt_zephyr_connected CALLED: conn=%p err=%u\n", conn, err);
+    mp_printf(&mp_plat_print, ">>> mp_bt_zephyr_connected CALLED: conn=%p err=%u state=%d\n",
+              conn, err, mp_bluetooth_zephyr_ble_state);
 
     // Safety check: only process if BLE is fully active and initialized
     if (mp_bluetooth_zephyr_ble_state != MP_BLUETOOTH_ZEPHYR_BLE_STATE_ACTIVE
         || MP_STATE_PORT(bluetooth_zephyr_root_pointers) == NULL) {
-        mp_printf(&mp_plat_print, ">>> Connection callback ignored - BLE not active (state=%d)\n", mp_bluetooth_zephyr_ble_state);
+        mp_printf(&mp_plat_print, ">>> Connection callback ignored - BLE not active (state=%d)\n",
+                  mp_bluetooth_zephyr_ble_state);
         return;
     }
 
@@ -246,12 +248,15 @@ static void mp_bt_zephyr_connected(struct bt_conn *conn, uint8_t err) {
 
 static void mp_bt_zephyr_disconnected(struct bt_conn *conn, uint8_t reason) {
     // CRITICAL: This printf MUST appear if callback is being called by Zephyr
-    mp_printf(&mp_plat_print, ">>> mp_bt_zephyr_disconnected CALLED: conn=%p reason=%u\n", conn, reason);
+    mp_printf(&mp_plat_print, ">>> mp_bt_zephyr_disconnected CALLED: conn=%p reason=%u state=%d\n",
+              conn, reason, mp_bluetooth_zephyr_ble_state);
 
     // Safety check: only process if BLE is fully active and initialized
+    // Ignore callbacks during deinit (SUSPENDED state) to prevent double-unref race
     if (mp_bluetooth_zephyr_ble_state != MP_BLUETOOTH_ZEPHYR_BLE_STATE_ACTIVE
         || MP_STATE_PORT(bluetooth_zephyr_root_pointers) == NULL) {
-        mp_printf(&mp_plat_print, ">>> Disconnect callback ignored - BLE not active (state=%d)\n", mp_bluetooth_zephyr_ble_state);
+        mp_printf(&mp_plat_print, ">>> Disconnect callback ignored - BLE not active (state=%d)\n",
+                  mp_bluetooth_zephyr_ble_state);
         return;
     }
 
@@ -489,56 +494,65 @@ int mp_bluetooth_deinit(void) {
         return 0;
     }
 
+    // Stop advertising before unregistering callbacks
+    // This cleans up any advertising connections (BT_CONN_ADV_CONNECTABLE state)
+    mp_printf(&mp_plat_print, ">>> Deinit: stopping advertising\n");
     mp_bluetooth_gap_advertise_stop();
+    mp_printf(&mp_plat_print, ">>> Deinit: advertising stopped\n");
 
     #if CONFIG_BT_GATT_DYNAMIC_DB
+    mp_printf(&mp_plat_print, ">>> Deinit: unregistering GATT services\n");
     for (size_t i = 0; i < MP_STATE_PORT(bluetooth_zephyr_root_pointers)->n_services; ++i) {
         bt_gatt_service_unregister(MP_STATE_PORT(bluetooth_zephyr_root_pointers)->services[i]);
         MP_STATE_PORT(bluetooth_zephyr_root_pointers)->services[i] = NULL;
     }
+    mp_printf(&mp_plat_print, ">>> Deinit: GATT services unregistered\n");
     #endif
 
     #if MICROPY_PY_BLUETOOTH_ENABLE_CENTRAL_MODE
+    mp_printf(&mp_plat_print, ">>> Deinit: stopping scan\n");
     mp_bluetooth_gap_scan_stop();
     bt_le_scan_cb_unregister(&mp_bluetooth_zephyr_gap_scan_cb_struct);
+    mp_printf(&mp_plat_print, ">>> Deinit: scan stopped\n");
     #endif
 
-    // Disconnect ALL Zephyr connections (not just our tracked ones)
-    // This is critical because Zephyr might have internal connections we don't track
-    void disconnect_all_cb(struct bt_conn *conn, void *data) {
-        (void)data;
-        struct bt_conn_info info;
-        if (bt_conn_get_info(conn, &info) == 0 && info.state == BT_CONN_STATE_CONNECTED) {
-            DEBUG_printf("Deinit: forcibly disconnecting connection %p\n", conn);
-            bt_conn_disconnect(conn, BT_HCI_ERR_REMOTE_USER_TERM_CONN);
-        }
+    // CRITICAL: Unregister connection callbacks before bt_disable()
+    // bt_disable() does NOT clear callbacks - they persist and cause double-registration
+    // on subsequent bt_enable() cycles, leading to crashes
+    mp_printf(&mp_plat_print, ">>> Deinit: unregistering connection callbacks\n");
+    bt_conn_cb_unregister(&mp_bt_zephyr_conn_callbacks);
+    mp_printf(&mp_plat_print, ">>> Deinit: connection callbacks unregistered\n");
+
+    // Use Zephyr's official bt_disable() API to shut down the BLE stack
+    // This automatically:
+    // - Disconnects all connections via bt_conn_cleanup_all()
+    // - Unrefs all connections properly (avoiding double-unref bugs)
+    // - Sends HCI RESET to controller
+    // - Closes HCI resources
+    // - Clears identity and keys
+    mp_printf(&mp_plat_print, ">>> Deinit: calling bt_disable()\n");
+    int err = bt_disable();
+    mp_printf(&mp_plat_print, ">>> Deinit: bt_disable() returned %d\n", err);
+    if (err != 0) {
+        mp_printf(&mp_plat_print, ">>> Deinit: bt_disable() FAILED with error %d\n", err);
+        // Don't fail deinit - just log and continue with cleanup
     }
-    bt_conn_foreach(BT_CONN_TYPE_LE, disconnect_all_cb, NULL);
 
-    // Clean up any existing connections in our tracking list
-    // We need to unref our stored connection pointers to avoid leaking references
+    // Clean up our connection tracking list
+    // The Zephyr connections themselves are already cleaned up by bt_disable()
+    // We just need to clear our tracking structures
     if (MP_STATE_PORT(bluetooth_zephyr_root_pointers) != NULL) {
         mp_bt_zephyr_conn_t *conn = MP_STATE_PORT(bluetooth_zephyr_root_pointers)->connections;
         while (conn != NULL) {
             mp_bt_zephyr_conn_t *next = conn->next;
-            if (conn->conn) {
-                DEBUG_printf("Deinit: unreffing stored connection %p\n", conn->conn);
-                bt_conn_unref(conn->conn);
-                conn->conn = NULL;
-            }
+            // conn->conn pointers are now invalid after bt_disable()
+            // Do NOT unref them - Zephyr already did that
+            conn->conn = NULL;
             conn = next;
         }
     }
 
-    // CRITICAL: Unregister connection callbacks before suspending
-    // This prevents Zephyr from calling our callbacks after we've cleared state
-    // Callbacks will be re-registered in mp_bluetooth_init()
-    bt_conn_cb_unregister(&mp_bt_zephyr_conn_callbacks);
-    DEBUG_printf("Deinit: unregistered connection callbacks\n");
-
-    // There is no way to turn off the BT stack in Zephyr, so just set the
-    // state as suspended so it can be correctly reactivated later.
-    mp_bluetooth_zephyr_ble_state = MP_BLUETOOTH_ZEPHYR_BLE_STATE_SUSPENDED;
+    mp_bluetooth_zephyr_ble_state = MP_BLUETOOTH_ZEPHYR_BLE_STATE_OFF;
 
     MP_STATE_PORT(bluetooth_zephyr_root_pointers) = NULL;
     mp_bt_zephyr_next_conn = NULL;