Re: [PATCH v1] Client: Fix the list_attributes command returning nothing for a dual-mode remote

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Luiz,


On 9/27/2024 10:31 PM, Luiz Augusto von Dentz wrote:
> Hi Cheng,
> 
> On Fri, Sep 27, 2024 at 9:16 AM Cheng Jiang <quic_chejiang@xxxxxxxxxxx> wrote:
>>
>> When a dual-mode device is paired first on BR/EDR and
>> then on BLE through RPA, the RPA changes to a public
>> address after receiving the IRK. This results in two proxies
>> in default_ctrl->devices with the same public address.
>> In cmd_list_attributes, if the BR/EDR proxy is found first,
>> it prints no attributes.
> 
> This seems to be a bug then, if we resolve the address and there is
> already a device object for it then that shall be used instead of
> keeping 2 different objects paths, fixing bluetoothctl to allow
> multiple proxies with the same device won't do anything for other
> clients so this is just a workaround.
> 
> There seems to be some code for detecting and merging the objects:
> 
> /* It is possible that we have two device objects for the same device in
>  * case it has first been discovered over BR/EDR and has a private
>  * address when discovered over LE for the first time. In such a case we
>  * need to inherit critical values from the duplicate so that we don't
>  * ovewrite them when writing to storage. The next time bluetoothd
>  * starts the device will show up as a single instance.
>  */
> void device_merge_duplicate(struct btd_device *dev, struct btd_device *dup)
> 
> But it doesn't seem to carry over the services, etc, as it seems we
> can't really just use one object at this point then both need to
> interact with each other, perhaps by storing the duplicate into
> btd_device so the right object can be used depending on the bearer,
> etc.
> 
Yes, this is just a workaround for the bluetoothctl client. The device_merge_duplicate
can't handle it. Actually, there are two different dbus interfaces created for the
two device objects. I didn't find a good way to merge these two dbus interface (I'm 
not familiar with dbus). 

Another thought is use the AddressType property to distinguish the two device objects.
however, in current implementation, BR/EDR and BLE public address are both assumed as
"public". Can we add a new string type (like "le_public") in `property_get_address_type`
for BLE public address.

Do you have any idea to get the bearer info in client? 


>> Modify cmd_list_attributes to search all proxies in
>> default_ctrl->devices and display the related attributes.
>> ---
>>  client/main.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++----
>>  1 file changed, 51 insertions(+), 4 deletions(-)
>>
>> diff --git a/client/main.c b/client/main.c
>> index 50aa3e7a6..17c1fb278 100644
>> --- a/client/main.c
>> +++ b/client/main.c
>> @@ -768,6 +768,29 @@ static GDBusProxy *find_proxy_by_address(GList *source, const char *address)
>>         return NULL;
>>  }
>>
>> +static GList *find_all_proxy_by_address(GList *source, const char *address)
>> +{
>> +       GList *list;
>> +       GList *all_proxy = NULL;
>> +
>> +       for (list = g_list_first(source); list; list = g_list_next(list)) {
>> +               GDBusProxy *proxy = list->data;
>> +               DBusMessageIter iter;
>> +               const char *str;
>> +
>> +               if (g_dbus_proxy_get_property(proxy, "Address", &iter) == FALSE)
>> +                       continue;
>> +
>> +               dbus_message_iter_get_basic(&iter, &str);
>> +
>> +               if (!strcasecmp(str, address))
>> +                       all_proxy = g_list_append(all_proxy, proxy);
>> +       }
>> +
>> +       return all_proxy;
>> +}
>> +
>> +
>>  static gboolean check_default_ctrl(void)
>>  {
>>         if (!default_ctrl) {
>> @@ -2051,7 +2074,9 @@ static void cmd_disconn(int argc, char *argv[])
>>
>>  static void cmd_list_attributes(int argc, char *argv[])
>>  {
>> -       GDBusProxy *proxy;
>> +       GList *all_proxy = NULL;
>> +       GList *list;
>> +       GDBusProxy *proxy = NULL;
>>         const char *path;
>>
>>         if (argc > 1 && !strcmp(argv[1], "local")) {
>> @@ -2059,11 +2084,33 @@ static void cmd_list_attributes(int argc, char *argv[])
>>                 goto done;
>>         }
>>
>> -       proxy = find_device(argc, argv);
>> -       if (!proxy)
>> +       if (argc < 2 || !strlen(argv[1])) {
>> +               if (default_dev) {
>> +                       proxy = default_dev;
>> +                       path = g_dbus_proxy_get_path(proxy);
>> +                       goto done;
>> +               }
>> +               bt_shell_printf("Missing device address argument\n");
>>                 return bt_shell_noninteractive_quit(EXIT_FAILURE);
>> +       } else {
>> +               if (check_default_ctrl() == FALSE)
>> +                       return bt_shell_noninteractive_quit(EXIT_FAILURE);
>>
>> -       path = g_dbus_proxy_get_path(proxy);
>> +               all_proxy = find_all_proxy_by_address(default_ctrl->devices,
>> +                                                               argv[1]);
>> +               if (!all_proxy) {
>> +                       bt_shell_printf("Device %s not available\n", argv[1]);
>> +                       return bt_shell_noninteractive_quit(EXIT_FAILURE);
>> +               }
>> +               for (list = g_list_first(all_proxy); list;
>> +                                               list = g_list_next(list)) {
>> +                       proxy = list->data;
>> +                       path = g_dbus_proxy_get_path(proxy);
>> +                       gatt_list_attributes(path);
>> +               }
>> +               g_list_free(all_proxy);
>> +               return bt_shell_noninteractive_quit(EXIT_SUCCESS);
>> +       }
>>
>>  done:
>>         gatt_list_attributes(path);
>> --
>> 2.25.1
>>
>>
> 
> 





[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux