| Message ID | 20240403110808.805771-1-frederic.danis@collabora.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [BlueZ] plugins: Add new plugin to fix airpods pairing | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| tedd_an/CheckPatch | success | CheckPatch PASS |
| tedd_an/GitLint | success | Gitlint PASS |
| tedd_an/BuildEll | success | Build ELL PASS |
| tedd_an/BluezMake | success | Bluez Make PASS |
| tedd_an/MakeCheck | success | Bluez Make Check PASS |
| tedd_an/MakeDistcheck | success | Make Distcheck PASS |
| tedd_an/CheckValgrind | success | Check Valgrind PASS |
| tedd_an/CheckSmatch | success | CheckSparse PASS |
| tedd_an/bluezmakeextell | success | Make External ELL PASS |
| tedd_an/IncrementalBuild | success | Incremental Build PASS |
| tedd_an/ScanBuild | success | Scan Build PASS |
This is automated email and please do not reply to this email! Dear submitter, Thank you for submitting the patches to the linux bluetooth mailing list. This is a CI test results with your patch series: PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=841054 ---Test result--- Test Summary: CheckPatch PASS 0.55 seconds GitLint PASS 0.34 seconds BuildEll PASS 24.87 seconds BluezMake PASS 1668.80 seconds MakeCheck PASS 13.49 seconds MakeDistcheck PASS 178.87 seconds CheckValgrind PASS 248.59 seconds CheckSmatch PASS 351.49 seconds bluezmakeextell PASS 120.29 seconds IncrementalBuild PASS 1467.46 seconds ScanBuild PASS 1018.85 seconds --- Regards, Linux Bluetooth
Hi, On Wed, 3 Apr 2024 at 13:08, Frédéric Danis <frederic.danis@collabora.com> wrote: > > Apple Airpods are discoverable and pairable in BREDR mode, but also > advertise in unconnectable mode in LE with the same Public address, at the > same time. As the pairing process uses the latest seen address, sometimes > it uses the LE Public address to pair, which fails. > > This commit adds a new adapter driver plugin which force the BREDR last > seen time on LE Public address device found event related to an Apple > device, allowing pairing process to always use the BREDR. > While I'm fine with having plugin for this specific case, I wonder if select_conn_bearer() shouldn't take bearer's connectable state into account when selecting bearer for connection > This commit is based on proposal > https://lore.kernel.org/all/20240103101328.1812899-1-clancy_shang@163.com/ > --- > Makefile.plugins | 3 ++ > plugins/airpods.c | 132 ++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 135 insertions(+) > create mode 100644 plugins/airpods.c > > diff --git a/Makefile.plugins b/Makefile.plugins > index 4aa2c9c92..2ebd8aaf6 100644 > --- a/Makefile.plugins > +++ b/Makefile.plugins > @@ -11,6 +11,9 @@ builtin_sources += plugins/autopair.c > builtin_modules += policy > builtin_sources += plugins/policy.c > > +builtin_modules += airpods > +builtin_sources += plugins/airpods.c > + > if ADMIN > builtin_modules += admin > builtin_sources += plugins/admin.c > diff --git a/plugins/airpods.c b/plugins/airpods.c > new file mode 100644 > index 000000000..5043f0cca > --- /dev/null > +++ b/plugins/airpods.c > @@ -0,0 +1,132 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * > + * BlueZ - Bluetooth protocol stack for Linux > + * > + * Copyright (C) 2024 Frédéric Danis <frederic.danis@collabora.com> > + * > + * > + */ > + > +#ifdef HAVE_CONFIG_H > +#include <config.h> > +#endif > + > +#include <stdbool.h> > + > +#include <glib.h> > + > +#include "bluetooth/bluetooth.h" > + > +#include "lib/mgmt.h" > +#include "src/plugin.h" > +#include "src/adapter.h" > +#include "src/device.h" > +#include "src/eir.h" > +#include "src/log.h" > +#include "src/shared/mgmt.h" > +#include "src/shared/util.h" > + > +#define APPLE_INC_VENDOR_ID 0x004c > + > +static struct mgmt *mgmt; > + > +static bool eir_msd_is_apple_inc(GSList *msd_list) > +{ > + GSList *msd_l, *msd_next; > + > + for (msd_l = msd_list; msd_l != NULL; msd_l = msd_next) { > + const struct eir_msd *msd = msd_l->data; > + > + msd_next = g_slist_next(msd_l); > + > + if (msd->company == APPLE_INC_VENDOR_ID) > + return true; > + } > + > + return false; > +} > + > +static void airpods_device_found_callback(uint16_t index, uint16_t length, > + const void *param, void *user_data) > +{ > + struct btd_device *dev; > + const struct mgmt_ev_device_found *ev = param; > + struct btd_adapter *adapter = user_data; > + uint16_t eir_len; > + uint32_t flags = le32_to_cpu(ev->flags); > + struct eir_data eir_data; > + > + dev = btd_adapter_find_device(adapter, &ev->addr.bdaddr, > + ev->addr.type); > + if (!dev) > + return; > + > + if (length < sizeof(*ev)) { > + warn("Too short device found event (%u bytes)", length); > + return; > + } > + > + eir_len = btohs(ev->eir_len); > + if (length != sizeof(*ev) + eir_len) { > + warn("Device found event size mismatch (%u != %zu)", > + length, sizeof(*ev) + eir_len); > + return; > + } > + > + if (eir_len == 0) > + return; > + > + memset(&eir_data, 0, sizeof(eir_data)); > + eir_parse(&eir_data, ev->eir, eir_len); > + > + if (eir_msd_is_apple_inc(eir_data.msd_list) && > + (flags & MGMT_DEV_FOUND_NOT_CONNECTABLE) && > + (ev->addr.type == BDADDR_LE_PUBLIC)) { > + DBG("Force BREDR last seen"); > + device_set_bredr_support(dev); > + device_update_last_seen(dev, BDADDR_BREDR, true); > + } > +} > + > +static int airpods_probe(struct btd_adapter *adapter) > +{ > + if (!mgmt) > + mgmt = mgmt_new_default(); > + > + if (!mgmt) { > + fprintf(stderr, "Failed to open management socket\n"); > + return 0; > + } > + > + mgmt_register(mgmt, MGMT_EV_DEVICE_FOUND, > + btd_adapter_get_index(adapter), > + airpods_device_found_callback, > + adapter, NULL); > + > + return 0; > +} > + > +static void airpods_remove(struct btd_adapter *adapter) > +{ > + mgmt_unregister_index(mgmt, btd_adapter_get_index(adapter)); > +} > + > +static struct btd_adapter_driver airpods_driver = { > + .name = "airpods", > + .probe = airpods_probe, > + .remove = airpods_remove, > +}; > + > +static int airpods_init(void) > +{ > + return btd_register_adapter_driver(&airpods_driver); > +} > + > +static void airpods_exit(void) > +{ > + btd_unregister_adapter_driver(&airpods_driver); > +} > + > +BLUETOOTH_PLUGIN_DEFINE(airpods, VERSION, > + BLUETOOTH_PLUGIN_PRIORITY_LOW, airpods_init, airpods_exit) > -- > 2.34.1 > >
Hi, On 03/04/2024 15:50, Szymon Janc wrote: > Hi, > > On Wed, 3 Apr 2024 at 13:08, Frédéric Danis > <frederic.danis@collabora.com> wrote: >> Apple Airpods are discoverable and pairable in BREDR mode, but also >> advertise in unconnectable mode in LE with the same Public address, at the >> same time. As the pairing process uses the latest seen address, sometimes >> it uses the LE Public address to pair, which fails. >> >> This commit adds a new adapter driver plugin which force the BREDR last >> seen time on LE Public address device found event related to an Apple >> device, allowing pairing process to always use the BREDR. >> > While I'm fine with having plugin for this specific case, I wonder if > select_conn_bearer() shouldn't take > bearer's connectable state into account when selecting bearer for connection Yes, this could be a lot simpler, but couldn't it break some other pairing case? >> This commit is based on proposal >> https://lore.kernel.org/all/20240103101328.1812899-1-clancy_shang@163.com/ >> --- >> Makefile.plugins | 3 ++ >> plugins/airpods.c | 132 ++++++++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 135 insertions(+) >> create mode 100644 plugins/airpods.c >> >> diff --git a/Makefile.plugins b/Makefile.plugins >> index 4aa2c9c92..2ebd8aaf6 100644 >> --- a/Makefile.plugins >> +++ b/Makefile.plugins >> @@ -11,6 +11,9 @@ builtin_sources += plugins/autopair.c >> builtin_modules += policy >> builtin_sources += plugins/policy.c >> >> +builtin_modules += airpods >> +builtin_sources += plugins/airpods.c >> + >> if ADMIN >> builtin_modules += admin >> builtin_sources += plugins/admin.c >> diff --git a/plugins/airpods.c b/plugins/airpods.c >> new file mode 100644 >> index 000000000..5043f0cca >> --- /dev/null >> +++ b/plugins/airpods.c >> @@ -0,0 +1,132 @@ >> +// SPDX-License-Identifier: GPL-2.0-or-later >> +/* >> + * >> + * BlueZ - Bluetooth protocol stack for Linux >> + * >> + * Copyright (C) 2024 Frédéric Danis <frederic.danis@collabora.com> >> + * >> + * >> + */ >> + >> +#ifdef HAVE_CONFIG_H >> +#include <config.h> >> +#endif >> + >> +#include <stdbool.h> >> + >> +#include <glib.h> >> + >> +#include "bluetooth/bluetooth.h" >> + >> +#include "lib/mgmt.h" >> +#include "src/plugin.h" >> +#include "src/adapter.h" >> +#include "src/device.h" >> +#include "src/eir.h" >> +#include "src/log.h" >> +#include "src/shared/mgmt.h" >> +#include "src/shared/util.h" >> + >> +#define APPLE_INC_VENDOR_ID 0x004c >> + >> +static struct mgmt *mgmt; >> + >> +static bool eir_msd_is_apple_inc(GSList *msd_list) >> +{ >> + GSList *msd_l, *msd_next; >> + >> + for (msd_l = msd_list; msd_l != NULL; msd_l = msd_next) { >> + const struct eir_msd *msd = msd_l->data; >> + >> + msd_next = g_slist_next(msd_l); >> + >> + if (msd->company == APPLE_INC_VENDOR_ID) >> + return true; >> + } >> + >> + return false; >> +} >> + >> +static void airpods_device_found_callback(uint16_t index, uint16_t length, >> + const void *param, void *user_data) >> +{ >> + struct btd_device *dev; >> + const struct mgmt_ev_device_found *ev = param; >> + struct btd_adapter *adapter = user_data; >> + uint16_t eir_len; >> + uint32_t flags = le32_to_cpu(ev->flags); >> + struct eir_data eir_data; >> + >> + dev = btd_adapter_find_device(adapter, &ev->addr.bdaddr, >> + ev->addr.type); >> + if (!dev) >> + return; >> + >> + if (length < sizeof(*ev)) { >> + warn("Too short device found event (%u bytes)", length); >> + return; >> + } >> + >> + eir_len = btohs(ev->eir_len); >> + if (length != sizeof(*ev) + eir_len) { >> + warn("Device found event size mismatch (%u != %zu)", >> + length, sizeof(*ev) + eir_len); >> + return; >> + } >> + >> + if (eir_len == 0) >> + return; >> + >> + memset(&eir_data, 0, sizeof(eir_data)); >> + eir_parse(&eir_data, ev->eir, eir_len); >> + >> + if (eir_msd_is_apple_inc(eir_data.msd_list) && >> + (flags & MGMT_DEV_FOUND_NOT_CONNECTABLE) && >> + (ev->addr.type == BDADDR_LE_PUBLIC)) { >> + DBG("Force BREDR last seen"); >> + device_set_bredr_support(dev); >> + device_update_last_seen(dev, BDADDR_BREDR, true); >> + } >> +} >> + >> +static int airpods_probe(struct btd_adapter *adapter) >> +{ >> + if (!mgmt) >> + mgmt = mgmt_new_default(); >> + >> + if (!mgmt) { >> + fprintf(stderr, "Failed to open management socket\n"); >> + return 0; >> + } >> + >> + mgmt_register(mgmt, MGMT_EV_DEVICE_FOUND, >> + btd_adapter_get_index(adapter), >> + airpods_device_found_callback, >> + adapter, NULL); >> + >> + return 0; >> +} >> + >> +static void airpods_remove(struct btd_adapter *adapter) >> +{ >> + mgmt_unregister_index(mgmt, btd_adapter_get_index(adapter)); >> +} >> + >> +static struct btd_adapter_driver airpods_driver = { >> + .name = "airpods", >> + .probe = airpods_probe, >> + .remove = airpods_remove, >> +}; >> + >> +static int airpods_init(void) >> +{ >> + return btd_register_adapter_driver(&airpods_driver); >> +} >> + >> +static void airpods_exit(void) >> +{ >> + btd_unregister_adapter_driver(&airpods_driver); >> +} >> + >> +BLUETOOTH_PLUGIN_DEFINE(airpods, VERSION, >> + BLUETOOTH_PLUGIN_PRIORITY_LOW, airpods_init, airpods_exit) >> -- >> 2.34.1 >> >> >
diff --git a/Makefile.plugins b/Makefile.plugins index 4aa2c9c92..2ebd8aaf6 100644 --- a/Makefile.plugins +++ b/Makefile.plugins @@ -11,6 +11,9 @@ builtin_sources += plugins/autopair.c builtin_modules += policy builtin_sources += plugins/policy.c +builtin_modules += airpods +builtin_sources += plugins/airpods.c + if ADMIN builtin_modules += admin builtin_sources += plugins/admin.c diff --git a/plugins/airpods.c b/plugins/airpods.c new file mode 100644 index 000000000..5043f0cca --- /dev/null +++ b/plugins/airpods.c @@ -0,0 +1,132 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * + * BlueZ - Bluetooth protocol stack for Linux + * + * Copyright (C) 2024 Frédéric Danis <frederic.danis@collabora.com> + * + * + */ + +#ifdef HAVE_CONFIG_H +#include <config.h> +#endif + +#include <stdbool.h> + +#include <glib.h> + +#include "bluetooth/bluetooth.h" + +#include "lib/mgmt.h" +#include "src/plugin.h" +#include "src/adapter.h" +#include "src/device.h" +#include "src/eir.h" +#include "src/log.h" +#include "src/shared/mgmt.h" +#include "src/shared/util.h" + +#define APPLE_INC_VENDOR_ID 0x004c + +static struct mgmt *mgmt; + +static bool eir_msd_is_apple_inc(GSList *msd_list) +{ + GSList *msd_l, *msd_next; + + for (msd_l = msd_list; msd_l != NULL; msd_l = msd_next) { + const struct eir_msd *msd = msd_l->data; + + msd_next = g_slist_next(msd_l); + + if (msd->company == APPLE_INC_VENDOR_ID) + return true; + } + + return false; +} + +static void airpods_device_found_callback(uint16_t index, uint16_t length, + const void *param, void *user_data) +{ + struct btd_device *dev; + const struct mgmt_ev_device_found *ev = param; + struct btd_adapter *adapter = user_data; + uint16_t eir_len; + uint32_t flags = le32_to_cpu(ev->flags); + struct eir_data eir_data; + + dev = btd_adapter_find_device(adapter, &ev->addr.bdaddr, + ev->addr.type); + if (!dev) + return; + + if (length < sizeof(*ev)) { + warn("Too short device found event (%u bytes)", length); + return; + } + + eir_len = btohs(ev->eir_len); + if (length != sizeof(*ev) + eir_len) { + warn("Device found event size mismatch (%u != %zu)", + length, sizeof(*ev) + eir_len); + return; + } + + if (eir_len == 0) + return; + + memset(&eir_data, 0, sizeof(eir_data)); + eir_parse(&eir_data, ev->eir, eir_len); + + if (eir_msd_is_apple_inc(eir_data.msd_list) && + (flags & MGMT_DEV_FOUND_NOT_CONNECTABLE) && + (ev->addr.type == BDADDR_LE_PUBLIC)) { + DBG("Force BREDR last seen"); + device_set_bredr_support(dev); + device_update_last_seen(dev, BDADDR_BREDR, true); + } +} + +static int airpods_probe(struct btd_adapter *adapter) +{ + if (!mgmt) + mgmt = mgmt_new_default(); + + if (!mgmt) { + fprintf(stderr, "Failed to open management socket\n"); + return 0; + } + + mgmt_register(mgmt, MGMT_EV_DEVICE_FOUND, + btd_adapter_get_index(adapter), + airpods_device_found_callback, + adapter, NULL); + + return 0; +} + +static void airpods_remove(struct btd_adapter *adapter) +{ + mgmt_unregister_index(mgmt, btd_adapter_get_index(adapter)); +} + +static struct btd_adapter_driver airpods_driver = { + .name = "airpods", + .probe = airpods_probe, + .remove = airpods_remove, +}; + +static int airpods_init(void) +{ + return btd_register_adapter_driver(&airpods_driver); +} + +static void airpods_exit(void) +{ + btd_unregister_adapter_driver(&airpods_driver); +} + +BLUETOOTH_PLUGIN_DEFINE(airpods, VERSION, + BLUETOOTH_PLUGIN_PRIORITY_LOW, airpods_init, airpods_exit)