Top posting to comment on the subject. It should have "[BlueZ PATCH]" as the prefix so that the user-space CI runs on it. On Thu, 2023-03-30 at 23:12 +0200, Hans de Goede wrote: > discovery_filter_to_mgmt_cp() does not add > discovery_filter.discoverable > to the created mgmt_cp_start_service_discovery struct. > > Instead update_discovery_filter() seprately checks > client->discovery_filter->discoverable for all clients. "separately". > This means that for discovery-filters which only have the > discoverable > flag set, to put the adapter in discoverable mode while discovering, > the created mgmt_cp_start_service_discovery struct is empty. > > This empty mgmt_cp_start_service_discovery struct then gets send "sent" > to the kernel as part of a MGMT_OP_START_SERVICE_DISCOVERY msg > by start_discovery_timeout(). > > This use of an empty filter with MGMT_OP_START_SERVICE_DISCOVERY > causes some bluetooth devices to not get seen with some (most?) > Broadcom bluetooth adapters. This problem has been observed with > the following Broadcom models: BCM4343A0, BCM43430A1, BCM43341B0 . > > On these models the following 2 devices were not being discovered > when starting a scan with a filter with just discoverable set > in the filter (as gnome-bluetooth does): > > Device 09:02:01:03:0F:87 (public) > Name: Bluetooth 3.0 Keyboard > Alias: Bluetooth 3.0 Keyboard > Class: 0x00000540 > Icon: input-keyboard > Paired: yes > Bonded: yes > Trusted: yes > Blocked: no > Connected: yes > WakeAllowed: yes > LegacyPairing: yes > UUID: Service Discovery Serve.. (00001000-0000-1000-8000- > 00805f9b34fb) > UUID: Human Interface Device... (00001124-0000-1000-8000- > 00805f9b34fb) > UUID: PnP Information (00001200-0000-1000-8000- > 00805f9b34fb) > Modalias: bluetooth:v05ACp022Cd011B > > Device 00:60:D1:00:00:34 (public) > Name: Bluetooth Mouse > Alias: Bluetooth Mouse > Class: 0x00002580 > Icon: input-mouse > Paired: yes > Bonded: yes > Trusted: yes > Blocked: no > Connected: yes > WakeAllowed: yes > LegacyPairing: no > UUID: Human Interface Device... (00001124-0000-1000-8000- > 00805f9b34fb) > UUID: PnP Information (00001200-0000-1000-8000- > 00805f9b34fb) > Modalias: usb:v0103p0204d001E > > Since setting the discoverable flag on a filter only is a way to > automatically put the adapter in discoverable mode itself while > it is discovering; and since this does not any device filtering > at all; modify merge_discovery_filters() to treat discovery with > such filters as regular unfiltered discovery. > > This results in start_discovery_timeout() starting regular > discovery through a MGMT_OP_START_DISCOVERY message and this > fixes these 2 example devices not getting discovered by the > mentioned Broadcom BT adapter models. > > Link: > https://gitlab.gnome.org/GNOME/gnome-bluetooth/-/merge_requests/163 > --- > Note the same argument can be made for the pattern and duplicate part > of > the filters which also get handled outside of the kernel filter. > But I prefer to keep the first patch small and targetted at solving > things > not working with the gnome-bluetooth filter settings. > > Also I'm not familiar enough with the code to say with certainty that > filters with just a pattern or the duplicate flag set (or a > combination) > should also be treated as unfiltered discovery. > --- > src/adapter.c | 17 ++++++++++++++++- > 1 file changed, 16 insertions(+), 1 deletion(-) > > diff --git a/src/adapter.c b/src/adapter.c > index 7947160a6..cc7f891d9 100644 > --- a/src/adapter.c > +++ b/src/adapter.c > @@ -2192,6 +2192,7 @@ static int merge_discovery_filters(struct > btd_adapter *adapter, int *rssi, > bool empty_uuid = false; > bool has_regular_discovery = false; > bool has_filtered_discovery = false; > + uint8_t adapter_scan_type = get_scan_type(adapter); > > for (l = adapter->discovery_list; l != NULL; l = > g_slist_next(l)) { > struct discovery_client *client = l->data; > @@ -2202,6 +2203,20 @@ static int merge_discovery_filters(struct > btd_adapter *adapter, int *rssi, > continue; > } > > + /* > + * Detect empty filter with only discoverable > + * (which does not require a kernel filter) set. > + */ > + if (item->uuids == NULL && > + item->pathloss == DISTANCE_VAL_INVALID && > + item->rssi == DISTANCE_VAL_INVALID && > + item->type == adapter_scan_type && > + item->duplicate == false && > + item->pattern == NULL) { I would have split this chunky "if" into a separate function, but otherwise the logic looks good. Reviewed-by: Bastien Nocera <hadess@xxxxxxxxxx> > + has_regular_discovery = true; > + continue; > + } > + > has_filtered_discovery = true; > > *transport |= item->type; > @@ -2251,7 +2266,7 @@ static int merge_discovery_filters(struct > btd_adapter *adapter, int *rssi, > * It there is both regular and filtered scan > running, then > * clear whole fitler to report all devices. > */ > - *transport = get_scan_type(adapter); > + *transport = adapter_scan_type; > *rssi = HCI_RSSI_INVALID; > g_slist_free(*uuids); > *uuids = NULL;
