| Message ID | 20240925090948.1540589-1-quic_chejiang@quicinc.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v3] device: Remove device after all bearers are disconnected | expand |
Hi Cheng, On Wed, Sep 25, 2024 at 9:31 PM Cheng Jiang <quic_chejiang@quicinc.com> wrote: > > Hi Luiz, > > On 9/25/2024 10:56 PM, Luiz Augusto von Dentz wrote: > > Hi Cheng, > > > > On Wed, Sep 25, 2024 at 5:10 AM Cheng Jiang <quic_chejiang@quicinc.com> wrote: > >> > >> For a combo mode remote, both BR/EDR and BLE may be connected. > >> RemoveDevice should be handled after all bearers are dropped, > >> otherwise, remove device is not performed in adapter_remove_connection. > >> --- > >> src/device.c | 12 +++++++++++- > >> 1 file changed, 11 insertions(+), 1 deletion(-) > >> > >> diff --git a/src/device.c b/src/device.c > >> index f8f61e643..c25bf6b60 100644 > >> --- a/src/device.c > >> +++ b/src/device.c > >> @@ -3492,8 +3492,18 @@ void device_remove_connection(struct btd_device *device, uint8_t bdaddr_type, > >> DBusMessage *msg = device->disconnects->data; > >> > >> if (dbus_message_is_method_call(msg, ADAPTER_INTERFACE, > >> - "RemoveDevice")) > >> + "RemoveDevice")) { > >> + > >> + /* Don't handle the RemoveDevice msg if device is > >> + * connected. For a dual mode remote, both BR/EDR > >> + * and BLE may be connected, RemoveDevice should > >> + * be handled after all bearers are disconnects. > >> + */ > >> + if (device->bredr_state.connected || > >> + device->le_state.connected) > >> + break; > > > > While this seems to make sense further down there is the same state: > > > > line 3531: if (device->bredr_state.connected || device->le_state.connected) > > > > So what this is doing is just to avoid removing the msg from > > device->disconnects but perhaps I'm missing something. > Yes, it is used to avoid remove the msg. If BR/EDR and BLE are connected, when user perform > remove operation, the BR/EDR connection is disconnect first. Then it set the "remove_device" > to True, and remove the msg from device->disconnects, but BLE is still connected, so this function > (device_remove_connection) will return without set the *remove to True. > Then the BLE is disconnected, but at this time, msg is already removed from device->disconnects, so > "remove_device" is not set to True. And accordingly *remove is not true. The device is not removed for > adapter in adapter_remove_connection. Need to wait for temporary device timeout to remove the device. Aha, that makes a lot more sense now, could you please update this in the patch description just to make it more clear, I will push it once you do that. > > > > >> remove_device = true; > >> + } > >> > >> g_dbus_send_reply(dbus_conn, msg, DBUS_TYPE_INVALID); > >> device->disconnects = g_slist_remove(device->disconnects, msg); > >> -- > >> 2.25.1 > >> > >> > > > > >
diff --git a/src/device.c b/src/device.c index f8f61e643..c25bf6b60 100644 --- a/src/device.c +++ b/src/device.c @@ -3492,8 +3492,18 @@ void device_remove_connection(struct btd_device *device, uint8_t bdaddr_type, DBusMessage *msg = device->disconnects->data; if (dbus_message_is_method_call(msg, ADAPTER_INTERFACE, - "RemoveDevice")) + "RemoveDevice")) { + + /* Don't handle the RemoveDevice msg if device is + * connected. For a dual mode remote, both BR/EDR + * and BLE may be connected, RemoveDevice should + * be handled after all bearers are disconnects. + */ + if (device->bredr_state.connected || + device->le_state.connected) + break; remove_device = true; + } g_dbus_send_reply(dbus_conn, msg, DBUS_TYPE_INVALID); device->disconnects = g_slist_remove(device->disconnects, msg);