Bluetooth: Audio: Improved cleanup on disconnect

Thalley · jhedberg · commit b2cf78af4cdd · 2022-03-09T15:04:57.000+02:00
The LE Audio clients will now do more and proper cleanup
during disconnects. Furthermore, the will also
take a proper bt_conn_ref when the conn pointer
is assigned locally.

Signed-off-by: Emil Gydesen &lt;emil.gydesen@nordicsemi.no&gt;
diff --git a/subsys/bluetooth/audio/aics_client.c b/subsys/bluetooth/audio/aics_client.c
@@ -621,7 +621,7 @@ static uint8_t aics_discover_func(struct bt_conn *conn, const struct bt_gatt_att
 	return BT_GATT_ITER_CONTINUE;
 }
 
-static void aics_client_reset(struct bt_aics *inst, struct bt_conn *conn)
+static void aics_client_reset(struct bt_aics *inst)
 {
 	inst->cli.desc_writable = 0;
 	inst->cli.change_counter = 0;
@@ -635,12 +635,36 @@ static void aics_client_reset(struct bt_aics *inst, struct bt_conn *conn)
 	inst->cli.control_handle = 0;
 	inst->cli.desc_handle = 0;
 
-	/* It's okay if these fail */
-	(void)bt_gatt_unsubscribe(conn, &inst->cli.state_sub_params);
-	(void)bt_gatt_unsubscribe(conn, &inst->cli.status_sub_params);
-	(void)bt_gatt_unsubscribe(conn, &inst->cli.desc_sub_params);
+	if (inst->cli.conn != NULL) {
+		struct bt_conn *conn = inst->cli.conn;
+
+		/* It's okay if these fail. In case of disconnect, we can't
+		 * unsubscribe and they will just fail.
+		 * In case that we reset due to another call of the discover
+		 * function, we will unsubscribe (regardless of bonding state)
+		 * to accommodate the new discovery values.
+		 */
+		(void)bt_gatt_unsubscribe(conn, &inst->cli.state_sub_params);
+		(void)bt_gatt_unsubscribe(conn, &inst->cli.status_sub_params);
+		(void)bt_gatt_unsubscribe(conn, &inst->cli.desc_sub_params);
+
+		bt_conn_unref(conn);
+		inst->cli.conn = NULL;
+	}
+}
+
+static void disconnected(struct bt_conn *conn, uint8_t reason)
+{
+	for (size_t i = 0; i < ARRAY_SIZE(aics_insts); i++) {
+		if (aics_insts[i].cli.conn == conn) {
+			aics_client_reset(&aics_insts[i]);
+		}
+	}
 }
 
+BT_CONN_CB_DEFINE(conn_callbacks) = {
+	.disconnected = disconnected,
+};
 
 int bt_aics_discover(struct bt_conn *conn, struct bt_aics *inst,
 		     const struct bt_aics_discover_param *param)
@@ -669,11 +693,11 @@ int bt_aics_discover(struct bt_conn *conn, struct bt_aics *inst,
 		return -EBUSY;
 	}
 
-	aics_client_reset(inst, conn);
+	aics_client_reset(inst);
 
 	(void)memset(&inst->cli.discover_params, 0, sizeof(inst->cli.discover_params));
 
-	inst->cli.conn = conn;
+	inst->cli.conn = bt_conn_ref(conn);
 	inst->cli.discover_params.start_handle = param->start_handle;
 	inst->cli.discover_params.end_handle = param->end_handle;
 	inst->cli.discover_params.type = BT_GATT_DISCOVER_CHARACTERISTIC;
diff --git a/subsys/bluetooth/audio/csis_client.c b/subsys/bluetooth/audio/csis_client.c
@@ -623,7 +623,7 @@ static uint8_t primary_discover_func(struct bt_conn *conn,
 		cur_inst->cli.idx = client->inst_count;
 		cur_inst->cli.start_handle = attr->handle + 1;
 		cur_inst->cli.end_handle = prim_service->end_handle;
-		cur_inst->cli.conn = conn;
+		cur_inst->cli.conn = bt_conn_ref(conn);
 		client->inst_count++;
 	}
 
@@ -1224,6 +1224,55 @@ static int csis_client_read_set_lock(struct bt_csis *inst)
 	return bt_gatt_read(inst->cli.conn, &read_params);
 }
 
+static void csis_client_reset(struct bt_csis_client_inst *inst)
+{
+	for (size_t i = 0; i < ARRAY_SIZE(inst->csis_insts); i++) {
+		struct bt_csis_client_svc_inst *cli = &inst->csis_insts[i].cli;
+
+		cli->idx = 0;
+		cli->rank = 0;
+		cli->set_lock = 0;
+		cli->start_handle = 0;
+		cli->end_handle = 0;
+		cli->set_sirk_handle = 0;
+		cli->set_size_handle = 0;
+		cli->set_lock_handle = 0;
+		cli->rank_handle = 0;
+
+		if (cli->conn != NULL) {
+			struct bt_conn *conn = cli->conn;
+
+			/* It's okay if these fail. In case of disconnect,
+			 * we can't unsubscribe and they will just fail.
+			 * In case that we reset due to another call of the
+			 * discover function, we will unsubscribe (regardless of
+			 * bonding state) to accommodate the new discovery
+			 * values.
+			 */
+			(void)bt_gatt_unsubscribe(conn, &cli->sirk_sub_params);
+			(void)bt_gatt_unsubscribe(conn, &cli->size_sub_params);
+			(void)bt_gatt_unsubscribe(conn, &cli->lock_sub_params);
+
+			bt_conn_unref(conn);
+			cli->conn = NULL;
+		}
+	}
+}
+
+static void disconnected(struct bt_conn *conn, uint8_t reason)
+{
+	struct bt_csis_client_inst *inst = &client_insts[bt_conn_index(conn)];
+
+	/* All csis_insts share the same conn pointer */
+	if (inst->csis_insts[0].cli.conn == conn) {
+		csis_client_reset(inst);
+	}
+}
+
+BT_CONN_CB_DEFINE(conn_callbacks) = {
+	.disconnected = disconnected,
+};
+
 /*************************** PUBLIC FUNCTIONS ***************************/
 void bt_csis_client_register_cb(struct bt_csis_client_cb *cb)
 {
diff --git a/subsys/bluetooth/audio/media_proxy.c b/subsys/bluetooth/audio/media_proxy.c
@@ -631,6 +631,18 @@ int media_proxy_ctrl_register(struct media_proxy_ctrl_cbs *ctrl_cbs)
 };
 
 #ifdef CONFIG_BT_MCC
+static void disconnected(struct bt_conn *conn, uint8_t reason)
+{
+	if (mprx.remote_player.conn == conn) {
+		bt_conn_unref(mprx.remote_player.conn);
+		mprx.remote_player.conn = NULL;
+	}
+}
+
+BT_CONN_CB_DEFINE(conn_callbacks) = {
+	.disconnected = disconnected,
+};
+
 int media_proxy_ctrl_discover_player(struct bt_conn *conn)
 {
 	int err;
@@ -689,7 +701,10 @@ int media_proxy_ctrl_discover_player(struct bt_conn *conn)
 		return err;
 	}
 
-	mprx.remote_player.conn = conn;
+	if (mprx.remote_player.conn != NULL) {
+		bt_conn_unref(mprx.remote_player.conn);
+	}
+	mprx.remote_player.conn = bt_conn_ref(conn);
 	mprx.remote_player.registered = true;  /* TODO: Do MCC init and "registration" at startup */
 
 	return 0;
diff --git a/subsys/bluetooth/audio/mics_client.c b/subsys/bluetooth/audio/mics_client.c
@@ -361,13 +361,35 @@ static void mics_client_reset(struct bt_mics *mics)
 	mics->cli.mute_handle = 0;
 	mics->cli.aics_inst_cnt = 0;
 
-	/* It's okay if this fails */
 	if (mics->cli.conn != NULL) {
-		(void)bt_gatt_unsubscribe(mics->cli.conn,
-					  &mics->cli.mute_sub_params);
+		struct bt_conn *conn = mics->cli.conn;
+
+		/* It's okay if this fails. In case of disconnect, we can't
+		 * unsubscribe and it will just fail.
+		 * In case that we reset due to another call of the discover
+		 * function, we will unsubscribe (regardless of bonding state)
+		 * to accommodate the new discovery values.
+		 */
+		(void)bt_gatt_unsubscribe(conn, &mics->cli.mute_sub_params);
+
+		bt_conn_unref(conn);
+		mics->cli.conn = NULL;
+	}
+}
+
+static void disconnected(struct bt_conn *conn, uint8_t reason)
+{
+	struct bt_mics *mics = &mics_insts[bt_conn_index(conn)];
+
+	if (mics->cli.conn == conn) {
+		mics_client_reset(mics);
 	}
 }
 
+BT_CONN_CB_DEFINE(conn_callbacks) = {
+	.disconnected = disconnected,
+};
+
 int bt_mics_discover(struct bt_conn *conn, struct bt_mics **mics)
 {
 	static bool initialized;
@@ -411,7 +433,7 @@ int bt_mics_discover(struct bt_conn *conn, struct bt_mics **mics)
 		}
 	}
 
-	mics_inst->cli.conn = conn;
+	mics_inst->cli.conn = bt_conn_ref(conn);
 	mics_inst->client_instance = true;
 	mics_inst->cli.discover_params.func = primary_discover_func;
 	mics_inst->cli.discover_params.uuid = mics_uuid;
diff --git a/subsys/bluetooth/audio/stream.c b/subsys/bluetooth/audio/stream.c
@@ -40,7 +40,11 @@ void bt_audio_stream_attach(struct bt_conn *conn,
 {
 	BT_DBG("conn %p stream %p ep %p codec %p", conn, stream, ep, codec);
 
-	stream->conn = conn;
+	if (conn != NULL) {
+		__ASSERT(stream->conn == NULL || stream->conn == conn,
+			 "stream->conn already attached");
+		stream->conn = bt_conn_ref(conn);
+	}
 	stream->codec = codec;
 	stream->ep = ep;
 	ep->stream = stream;
@@ -217,7 +221,10 @@ void bt_audio_stream_detach(struct bt_audio_stream *stream)
 
 	BT_DBG("stream %p", stream);
 
-	stream->conn = NULL;
+	if (stream->conn != NULL) {
+		bt_conn_unref(stream->conn);
+		stream->conn = NULL;
+	}
 	stream->codec = NULL;
 	stream->ep->stream = NULL;
 	stream->ep = NULL;
diff --git a/subsys/bluetooth/audio/vcs_client.c b/subsys/bluetooth/audio/vcs_client.c
@@ -677,10 +677,8 @@ static void vocs_discover_cb(struct bt_vocs *inst, int err)
 }
 #endif /* CONFIG_BT_VCS_CLIENT_VOCS */
 
-static void vcs_client_reset(struct bt_conn *conn)
+static void vcs_client_reset(struct bt_vcs *vcs_inst)
 {
-	struct bt_vcs *vcs_inst = &vcs_insts[bt_conn_index(conn)];
-
 	memset(&vcs_inst->cli.state, 0, sizeof(vcs_inst->cli.state));
 	vcs_inst->cli.flags = 0;
 	vcs_inst->cli.start_handle = 0;
@@ -693,11 +691,36 @@ static void vcs_client_reset(struct bt_conn *conn)
 
 	memset(&vcs_inst->cli.discover_params, 0, sizeof(vcs_inst->cli.discover_params));
 
-	/* It's okay if these fail */
-	(void)bt_gatt_unsubscribe(conn, &vcs_inst->cli.state_sub_params);
-	(void)bt_gatt_unsubscribe(conn, &vcs_inst->cli.flag_sub_params);
+	if (vcs_inst->cli.conn != NULL) {
+		struct bt_conn *conn = vcs_inst->cli.conn;
+
+		/* It's okay if these fail. In case of disconnect, we can't
+		 * unsubscribe and they will just fail.
+		 * In case that we reset due to another call of the discover
+		 * function, we will unsubscribe (regardless of bonding state)
+		 * to accommodate the new discovery values.
+		 */
+		(void)bt_gatt_unsubscribe(conn, &vcs_inst->cli.state_sub_params);
+		(void)bt_gatt_unsubscribe(conn, &vcs_inst->cli.flag_sub_params);
+
+		bt_conn_unref(conn);
+		vcs_inst->cli.conn = NULL;
+	}
+}
+
+static void disconnected(struct bt_conn *conn, uint8_t reason)
+{
+	struct bt_vcs *vcs_inst = &vcs_insts[bt_conn_index(conn)];
+
+	if (vcs_inst->cli.conn == conn) {
+		vcs_client_reset(vcs_inst);
+	}
 }
 
+BT_CONN_CB_DEFINE(conn_callbacks) = {
+	.disconnected = disconnected,
+};
+
 static void bt_vcs_client_init(void)
 {
 	int i, j;
@@ -765,12 +788,12 @@ int bt_vcs_discover(struct bt_conn *conn, struct bt_vcs **vcs)
 		initialized = true;
 	}
 
-	vcs_client_reset(conn);
+	vcs_client_reset(vcs_inst);
 
 	memcpy(&vcs_inst->cli.uuid, BT_UUID_VCS, sizeof(vcs_inst->cli.uuid));
 
 	vcs_inst->client_instance = true;
-	vcs_inst->cli.conn = conn;
+	vcs_inst->cli.conn = bt_conn_ref(conn);
 	vcs_inst->cli.discover_params.func = primary_discover_func;
 	vcs_inst->cli.discover_params.uuid = &vcs_inst->cli.uuid.uuid;
 	vcs_inst->cli.discover_params.type = BT_GATT_DISCOVER_PRIMARY;
diff --git a/subsys/bluetooth/audio/vocs_client.c b/subsys/bluetooth/audio/vocs_client.c
@@ -656,7 +656,7 @@ int bt_vocs_client_conn_get(const struct bt_vocs *vocs, struct bt_conn **conn)
 	return 0;
 }
 
-static void vocs_client_reset(struct bt_vocs *inst, struct bt_conn *conn)
+static void vocs_client_reset(struct bt_vocs *inst)
 {
 	memset(&inst->cli.state, 0, sizeof(inst->cli.state));
 	inst->cli.location_writable = 0;
@@ -669,10 +669,22 @@ static void vocs_client_reset(struct bt_vocs *inst, struct bt_conn *conn)
 	inst->cli.control_handle = 0;
 	inst->cli.desc_handle = 0;
 
-	/* It's okay if these fail */
-	(void)bt_gatt_unsubscribe(conn, &inst->cli.state_sub_params);
-	(void)bt_gatt_unsubscribe(conn, &inst->cli.location_sub_params);
-	(void)bt_gatt_unsubscribe(conn, &inst->cli.desc_sub_params);
+	if (inst->cli.conn != NULL) {
+		struct bt_conn *conn = inst->cli.conn;
+
+		/* It's okay if these fail. In case of disconnect, we can't
+		 * unsubscribe and they will just fail.
+		 * In case that we reset due to another call of the discover
+		 * function, we will unsubscribe (regardless of bonding state)
+		 * to accommodate the new discovery values.
+		 */
+		(void)bt_gatt_unsubscribe(conn, &inst->cli.state_sub_params);
+		(void)bt_gatt_unsubscribe(conn, &inst->cli.location_sub_params);
+		(void)bt_gatt_unsubscribe(conn, &inst->cli.desc_sub_params);
+
+		bt_conn_unref(conn);
+		inst->cli.conn = NULL;
+	}
 }
 
 int bt_vocs_discover(struct bt_conn *conn, struct bt_vocs *inst,
@@ -702,9 +714,9 @@ int bt_vocs_discover(struct bt_conn *conn, struct bt_vocs *inst,
 		return -EBUSY;
 	}
 
-	vocs_client_reset(inst, conn);
+	vocs_client_reset(inst);
 
-	inst->cli.conn = conn;
+	inst->cli.conn = bt_conn_ref(conn);
 	inst->cli.discover_params.start_handle = param->start_handle;
 	inst->cli.discover_params.end_handle = param->end_handle;
 	inst->cli.discover_params.type = BT_GATT_DISCOVER_CHARACTERISTIC;
