Hi Cheng,
On Wed, Sep 25, 2024 at 9:31 PM Cheng Jiang <quic_chejiang@xxxxxxxxxxx> 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@xxxxxxxxxxx> 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
> >>
> >>
> >
> >
>
--
Luiz Augusto von Dentz
