Hi Luiz,
On 9/27/2024 10:03 PM, Luiz Augusto von Dentz wrote:
> Hi Cheng,
>
> On Thu, Sep 26, 2024 at 10:39 PM 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 | 21 ++++++++++++++++++++-
>> 1 file changed, 20 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/device.c b/src/device.c
>> index f8f61e643..4efd755a0 100644
>> --- a/src/device.c
>> +++ b/src/device.c
>> @@ -3492,8 +3492,27 @@ 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.
>> + * Otherwise, if msg is removed, but not all
>> + * connection are dropped, this function returns
>> + * before *remove is updated, then after all
>> + * connections are dropped, but device->disconnects
>> + * is NULL, remove_device is not updated. Consequently
>> + * *remove is not set to true. The device is not removed
>> + * for adapter in adapter_remove_connection. Need
>> + * to wait for temporary device timeout to remove
>> + * the device.
>> + */
>
> I mean as git description, anyway Im having second thoughts about this
> change, see below.
>
>> + 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);
>
> How we move the block checking for disconnect message after check all
> bearers have been disconnected:
>
> diff --git a/src/device.c b/src/device.c
> index f8f61e64376c..76d2c859c747 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -3488,18 +3488,6 @@ void device_remove_connection(struct btd_device
> *device, uint8_t bdaddr_type,
> device->connect = NULL;
> }
>
> - while (device->disconnects) {
> - DBusMessage *msg = device->disconnects->data;
> -
> - if (dbus_message_is_method_call(msg, ADAPTER_INTERFACE,
> - "RemoveDevice"))
> - remove_device = true;
> -
> - g_dbus_send_reply(dbus_conn, msg, DBUS_TYPE_INVALID);
> - device->disconnects = g_slist_remove(device->disconnects, msg);
> - dbus_message_unref(msg);
> - }
> -
> /* Check paired status of both bearers since it's possible to be
> /* Check paired status of both bearers since it's possible to be
> /* Check paired status of both bearers since it's possible to be
> * paired but not connected via link key to LTK conversion.
> */
> @@ -3539,6 +3527,18 @@ void device_remove_connection(struct btd_device
> *device, uint8_t bdaddr_type,
> g_dbus_emit_property_changed(dbus_conn, device->path,
> DEVICE_INTERFACE, "Connected");
>
> + while (device->disconnects) {
> + DBusMessage *msg = device->disconnects->data;
> +
> + if (dbus_message_is_method_call(msg, ADAPTER_INTERFACE,
> + "RemoveDevice"))
> + remove_device = true;
> +
> + g_dbus_send_reply(dbus_conn, msg, DBUS_TYPE_INVALID);
> + device->disconnects = g_slist_remove(device->disconnects, msg);
> + dbus_message_unref(msg);
> + }
> +
> if (remove_device)
> *remove = remove_device;
>
>
Agree with you. Update the patch (v5). Thank you!
>> --
>> 2.25.1
>>
>>
>
>
