Temporary device removal during discovery

Temporary device removal during discovery

[Andrey Batyiev]

Hello everyone,

I've found the following issue:
1. in bluetoothctl run "power on", "scan on"
2. discovery is now permanent
3. make one device discoverable for a moment (e.g. turn bluetooth on
on your phone)
4. bluez would detect new device
5. turn bluetooth off on your phone
6. now wait

Expected result:
7. your phone should disappear from discovered set after some time

Actual result:
7. phone would stay there until discovery is off (i.e. "scan off" in
bluetoothctl)


It seems like there is a code in src/adapter.c responsible for purge
stale entries
(remove_temp_devices), however it only triggers when discovery is off
(and after 3 mins).


My use case is to continuously monitor the bluetooth environment. Is
it bluez responsibility to
remove stale entries during discovery or should my own app repeatedly
stop discovery?

Thanks,
   Andrey

Re: Temporary device removal during discovery

[Bastien Nocera]

Hey Andrey,

On Wed, 2020-07-08 at 13:24 +0300, Andrey Batyiev wrote:
> Hello everyone,
> 
> I've found the following issue:
> 1. in bluetoothctl run "power on", "scan on"
> 2. discovery is now permanent
> 3. make one device discoverable for a moment (e.g. turn bluetooth on
> on your phone)
> 4. bluez would detect new device
> 5. turn bluetooth off on your phone
> 6. now wait
> 
> Expected result:
> 7. your phone should disappear from discovered set after some time
> 
> Actual result:
> 7. phone would stay there until discovery is off (i.e. "scan off" in
> bluetoothctl)
> 
> 
> It seems like there is a code in src/adapter.c responsible for purge
> stale entries
> (remove_temp_devices), however it only triggers when discovery is off
> (and after 3 mins).
> 
> 
> My use case is to continuously monitor the bluetooth environment. Is
> it bluez responsibility to
> remove stale entries during discovery or should my own app repeatedly
> stop discovery?

It's been a problem for a while. I sent one of those mails as well:
https://www.spinics.net/lists/linux-bluetooth/msg75947.html
https://www.spinics.net/lists/linux-bluetooth/msg74397.html

Can you please file a bug at https://github.com/bluez/bluez/issues ?

I'll CC: myself on it too.

Cheers

Re: Temporary device removal during discovery

[Andrey Batyiev]

Hi Bastien, everyone,

I've made a patch to the bluez, so the temporary devices are routinely
purged based on their last_seen attribute.

What do you think about such solution?

Thanks,
    Andrey



diff --git a/src/adapter.c b/src/adapter.c
index 529002b02..101b03633 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -88,6 +88,7 @@

#define CONN_SCAN_TIMEOUT (3)
#define IDLE_DISCOV_TIMEOUT (5)
+#define TEMP_DEV_REAPER_INTERVAL (30)
#define TEMP_DEV_TIMEOUT (3 * 60)
#define BONDING_TIMEOUT (2 * 60)

@@ -1491,18 +1492,18 @@ static gboolean remove_temp_devices(gpointer user_data)

       DBG("%s", adapter->path);

-       adapter->temp_devices_timeout = 0;
-
       for (l = adapter->devices; l != NULL; l = next) {
               struct btd_device *dev = l->data;

               next = g_slist_next(l);

-               if (device_is_temporary(dev) && !btd_device_is_connected(dev))
+               if (device_is_temporary(dev) &&
+                   !btd_device_is_connected(dev) &&
+                   device_last_seen_delta(dev) > TEMP_DEV_TIMEOUT)
                       btd_adapter_remove_device(adapter, dev);
       }

-       return FALSE;
+       return TRUE;
}

static void discovery_cleanup(struct btd_adapter *adapter)
@@ -1516,11 +1517,6 @@ static void discovery_cleanup(struct
btd_adapter *adapter)
               adapter->discovery_idle_timeout = 0;
       }

-       if (adapter->temp_devices_timeout > 0) {
-               g_source_remove(adapter->temp_devices_timeout);
-               adapter->temp_devices_timeout = 0;
-       }
-
       g_slist_free_full(adapter->discovery_found,
                                               invalidate_rssi_and_tx_power);
       adapter->discovery_found = NULL;
@@ -1536,9 +1532,6 @@ static void discovery_cleanup(struct btd_adapter *adapter)
               if (device_is_temporary(dev) && !device_is_connectable(dev))
                       btd_adapter_remove_device(adapter, dev);
       }
-
-       adapter->temp_devices_timeout = g_timeout_add_seconds(TEMP_DEV_TIMEOUT,
-                                               remove_temp_devices, adapter);
}

static void discovery_free(void *user_data)
@@ -2405,7 +2398,7 @@ static bool parse_pathloss(DBusMessageIter *value,
       return true;
}

-static bool parse_transport(DBusMessageIter *value,
+static bool parse_transport(DBusMessageIter *value,
                                       struct discovery_filter *filter)
{
       char *transport_str;
@@ -5362,6 +5355,11 @@ static void adapter_free(gpointer user_data)
               adapter->passive_scan_timeout = 0;
       }

+       if (adapter->temp_devices_timeout > 0) {
+               g_source_remove(adapter->temp_devices_timeout);
+               adapter->temp_devices_timeout = 0;
+       }
+
       if (adapter->load_ltks_timeout > 0)
               g_source_remove(adapter->load_ltks_timeout);

@@ -6343,6 +6341,9 @@ static struct btd_adapter *btd_adapter_new(uint16_t index)

       adapter->auths = g_queue_new();

+       adapter->temp_devices_timeout =
g_timeout_add_seconds(TEMP_DEV_REAPER_INTERVAL,
+                                               remove_temp_devices, adapter);
+
       return btd_adapter_ref(adapter);
}

diff --git a/src/device.c b/src/device.c
index 0deee2707..cebbabab2 100644
--- a/src/device.c
+++ b/src/device.c
@@ -4240,6 +4240,15 @@ void device_update_last_seen(struct btd_device
*device, uint8_t bdaddr_type)
               device->le_seen = time(NULL);
}

+time_t device_last_seen_delta(struct btd_device *device)
+{
+       const time_t now = time(NULL);
+       const time_t bredr_time = now - device->bredr_seen;
+       const time_t le_time = now - device->le_seen;
+
+       return bredr_time < le_time ? bredr_time : le_time;
+}
+
/* 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
diff --git a/src/device.h b/src/device.h
index cb8d884e8..75fd3ec60 100644
--- a/src/device.h
+++ b/src/device.h
@@ -44,6 +44,7 @@ void device_update_addr(struct btd_device *device,
const bdaddr_t *bdaddr,
void device_set_bredr_support(struct btd_device *device);
void device_set_le_support(struct btd_device *device, uint8_t bdaddr_type);
void device_update_last_seen(struct btd_device *device, uint8_t bdaddr_type);
+time_t device_last_seen_delta(struct btd_device *device);
void device_merge_duplicate(struct btd_device *dev, struct btd_device *dup);
uint32_t btd_device_get_class(struct btd_device *device);
uint16_t btd_device_get_vendor(struct btd_device *device);

Re: Temporary device removal during discovery

[Luiz Augusto von Dentz]

Hi Andrey,

On Wed, Jul 8, 2020 at 8:56 AM Andrey Batyiev <batyiev@gmail.com> wrote:
>
> Hi Bastien, everyone,
>
> I've made a patch to the bluez, so the temporary devices are routinely
> purged based on their last_seen attribute.
>
> What do you think about such solution?
>
> Thanks,
>     Andrey
>
>
>
> diff --git a/src/adapter.c b/src/adapter.c
> index 529002b02..101b03633 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -88,6 +88,7 @@
>
> #define CONN_SCAN_TIMEOUT (3)
> #define IDLE_DISCOV_TIMEOUT (5)
> +#define TEMP_DEV_REAPER_INTERVAL (30)
> #define TEMP_DEV_TIMEOUT (3 * 60)
> #define BONDING_TIMEOUT (2 * 60)
>
> @@ -1491,18 +1492,18 @@ static gboolean remove_temp_devices(gpointer user_data)
>
>        DBG("%s", adapter->path);
>
> -       adapter->temp_devices_timeout = 0;
> -
>        for (l = adapter->devices; l != NULL; l = next) {
>                struct btd_device *dev = l->data;
>
>                next = g_slist_next(l);
>
> -               if (device_is_temporary(dev) && !btd_device_is_connected(dev))
> +               if (device_is_temporary(dev) &&
> +                   !btd_device_is_connected(dev) &&
> +                   device_last_seen_delta(dev) > TEMP_DEV_TIMEOUT)

The delta logic might be a nice addition as a separate patch, it is
more for detecting devices disappearing then actually cleanup during
power off.

>                        btd_adapter_remove_device(adapter, dev);
>        }
>
> -       return FALSE;
> +       return TRUE;
> }
>
> static void discovery_cleanup(struct btd_adapter *adapter)
> @@ -1516,11 +1517,6 @@ static void discovery_cleanup(struct
> btd_adapter *adapter)
>                adapter->discovery_idle_timeout = 0;
>        }
>
> -       if (adapter->temp_devices_timeout > 0) {
> -               g_source_remove(adapter->temp_devices_timeout);
> -               adapter->temp_devices_timeout = 0;
> -       }
> -
>        g_slist_free_full(adapter->discovery_found,
>                                                invalidate_rssi_and_tx_power);
>        adapter->discovery_found = NULL;
> @@ -1536,9 +1532,6 @@ static void discovery_cleanup(struct btd_adapter *adapter)
>                if (device_is_temporary(dev) && !device_is_connectable(dev))
>                        btd_adapter_remove_device(adapter, dev);
>        }
> -
> -       adapter->temp_devices_timeout = g_timeout_add_seconds(TEMP_DEV_TIMEOUT,
> -                                               remove_temp_devices, adapter);
> }
>
> static void discovery_free(void *user_data)
> @@ -2405,7 +2398,7 @@ static bool parse_pathloss(DBusMessageIter *value,
>        return true;
> }
>
> -static bool parse_transport(DBusMessageIter *value,
> +static bool parse_transport(DBusMessageIter *value,
>                                        struct discovery_filter *filter)
> {
>        char *transport_str;
> @@ -5362,6 +5355,11 @@ static void adapter_free(gpointer user_data)
>                adapter->passive_scan_timeout = 0;
>        }
>
> +       if (adapter->temp_devices_timeout > 0) {
> +               g_source_remove(adapter->temp_devices_timeout);
> +               adapter->temp_devices_timeout = 0;
> +       }
> +
>        if (adapter->load_ltks_timeout > 0)
>                g_source_remove(adapter->load_ltks_timeout);
>
> @@ -6343,6 +6341,9 @@ static struct btd_adapter *btd_adapter_new(uint16_t index)
>
>        adapter->auths = g_queue_new();
>
> +       adapter->temp_devices_timeout =
> g_timeout_add_seconds(TEMP_DEV_REAPER_INTERVAL,
> +                                               remove_temp_devices, adapter);
> +
>        return btd_adapter_ref(adapter);
> }
>
> diff --git a/src/device.c b/src/device.c
> index 0deee2707..cebbabab2 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -4240,6 +4240,15 @@ void device_update_last_seen(struct btd_device
> *device, uint8_t bdaddr_type)
>                device->le_seen = time(NULL);
> }
>
> +time_t device_last_seen_delta(struct btd_device *device)
> +{
> +       const time_t now = time(NULL);
> +       const time_t bredr_time = now - device->bredr_seen;
> +       const time_t le_time = now - device->le_seen;
> +
> +       return bredr_time < le_time ? bredr_time : le_time;
> +}
> +
> /* 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
> diff --git a/src/device.h b/src/device.h
> index cb8d884e8..75fd3ec60 100644
> --- a/src/device.h
> +++ b/src/device.h
> @@ -44,6 +44,7 @@ void device_update_addr(struct btd_device *device,
> const bdaddr_t *bdaddr,
> void device_set_bredr_support(struct btd_device *device);
> void device_set_le_support(struct btd_device *device, uint8_t bdaddr_type);
> void device_update_last_seen(struct btd_device *device, uint8_t bdaddr_type);
> +time_t device_last_seen_delta(struct btd_device *device);
> void device_merge_duplicate(struct btd_device *dev, struct btd_device *dup);
> uint32_t btd_device_get_class(struct btd_device *device);
> uint16_t btd_device_get_vendor(struct btd_device *device);



-- 
Luiz Augusto von Dentz

Re: Temporary device removal during discovery

[Luiz Augusto von Dentz]

Hi Andrey, Bastien,

On Wed, Jul 8, 2020 at 2:14 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Andrey,
>
> On Wed, Jul 8, 2020 at 8:56 AM Andrey Batyiev <batyiev@gmail.com> wrote:
> >
> > Hi Bastien, everyone,
> >
> > I've made a patch to the bluez, so the temporary devices are routinely
> > purged based on their last_seen attribute.
> >
> > What do you think about such solution?

I pushed a patch so than if the adapter is power off or unplugged it
will immediately remove all temporary devices.

> > Thanks,
> >     Andrey
> >
> >
> >
> > diff --git a/src/adapter.c b/src/adapter.c
> > index 529002b02..101b03633 100644
> > --- a/src/adapter.c
> > +++ b/src/adapter.c
> > @@ -88,6 +88,7 @@
> >
> > #define CONN_SCAN_TIMEOUT (3)
> > #define IDLE_DISCOV_TIMEOUT (5)
> > +#define TEMP_DEV_REAPER_INTERVAL (30)
> > #define TEMP_DEV_TIMEOUT (3 * 60)
> > #define BONDING_TIMEOUT (2 * 60)
> >
> > @@ -1491,18 +1492,18 @@ static gboolean remove_temp_devices(gpointer user_data)
> >
> >        DBG("%s", adapter->path);
> >
> > -       adapter->temp_devices_timeout = 0;
> > -
> >        for (l = adapter->devices; l != NULL; l = next) {
> >                struct btd_device *dev = l->data;
> >
> >                next = g_slist_next(l);
> >
> > -               if (device_is_temporary(dev) && !btd_device_is_connected(dev))
> > +               if (device_is_temporary(dev) &&
> > +                   !btd_device_is_connected(dev) &&
> > +                   device_last_seen_delta(dev) > TEMP_DEV_TIMEOUT)
>
> The delta logic might be a nice addition as a separate patch, it is
> more for detecting devices disappearing then actually cleanup during
> power off.
>
> >                        btd_adapter_remove_device(adapter, dev);
> >        }
> >
> > -       return FALSE;
> > +       return TRUE;
> > }
> >
> > static void discovery_cleanup(struct btd_adapter *adapter)
> > @@ -1516,11 +1517,6 @@ static void discovery_cleanup(struct
> > btd_adapter *adapter)
> >                adapter->discovery_idle_timeout = 0;
> >        }
> >
> > -       if (adapter->temp_devices_timeout > 0) {
> > -               g_source_remove(adapter->temp_devices_timeout);
> > -               adapter->temp_devices_timeout = 0;
> > -       }
> > -
> >        g_slist_free_full(adapter->discovery_found,
> >                                                invalidate_rssi_and_tx_power);
> >        adapter->discovery_found = NULL;
> > @@ -1536,9 +1532,6 @@ static void discovery_cleanup(struct btd_adapter *adapter)
> >                if (device_is_temporary(dev) && !device_is_connectable(dev))
> >                        btd_adapter_remove_device(adapter, dev);
> >        }
> > -
> > -       adapter->temp_devices_timeout = g_timeout_add_seconds(TEMP_DEV_TIMEOUT,
> > -                                               remove_temp_devices, adapter);
> > }
> >
> > static void discovery_free(void *user_data)
> > @@ -2405,7 +2398,7 @@ static bool parse_pathloss(DBusMessageIter *value,
> >        return true;
> > }
> >
> > -static bool parse_transport(DBusMessageIter *value,
> > +static bool parse_transport(DBusMessageIter *value,
> >                                        struct discovery_filter *filter)
> > {
> >        char *transport_str;
> > @@ -5362,6 +5355,11 @@ static void adapter_free(gpointer user_data)
> >                adapter->passive_scan_timeout = 0;
> >        }
> >
> > +       if (adapter->temp_devices_timeout > 0) {
> > +               g_source_remove(adapter->temp_devices_timeout);
> > +               adapter->temp_devices_timeout = 0;
> > +       }
> > +
> >        if (adapter->load_ltks_timeout > 0)
> >                g_source_remove(adapter->load_ltks_timeout);
> >
> > @@ -6343,6 +6341,9 @@ static struct btd_adapter *btd_adapter_new(uint16_t index)
> >
> >        adapter->auths = g_queue_new();
> >
> > +       adapter->temp_devices_timeout =
> > g_timeout_add_seconds(TEMP_DEV_REAPER_INTERVAL,
> > +                                               remove_temp_devices, adapter);
> > +
> >        return btd_adapter_ref(adapter);
> > }
> >
> > diff --git a/src/device.c b/src/device.c
> > index 0deee2707..cebbabab2 100644
> > --- a/src/device.c
> > +++ b/src/device.c
> > @@ -4240,6 +4240,15 @@ void device_update_last_seen(struct btd_device
> > *device, uint8_t bdaddr_type)
> >                device->le_seen = time(NULL);
> > }
> >
> > +time_t device_last_seen_delta(struct btd_device *device)
> > +{
> > +       const time_t now = time(NULL);
> > +       const time_t bredr_time = now - device->bredr_seen;
> > +       const time_t le_time = now - device->le_seen;
> > +
> > +       return bredr_time < le_time ? bredr_time : le_time;
> > +}
> > +
> > /* 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
> > diff --git a/src/device.h b/src/device.h
> > index cb8d884e8..75fd3ec60 100644
> > --- a/src/device.h
> > +++ b/src/device.h
> > @@ -44,6 +44,7 @@ void device_update_addr(struct btd_device *device,
> > const bdaddr_t *bdaddr,
> > void device_set_bredr_support(struct btd_device *device);
> > void device_set_le_support(struct btd_device *device, uint8_t bdaddr_type);
> > void device_update_last_seen(struct btd_device *device, uint8_t bdaddr_type);
> > +time_t device_last_seen_delta(struct btd_device *device);
> > void device_merge_duplicate(struct btd_device *dev, struct btd_device *dup);
> > uint32_t btd_device_get_class(struct btd_device *device);
> > uint16_t btd_device_get_vendor(struct btd_device *device);
>
>
>
> --
> Luiz Augusto von Dentz



-- 
Luiz Augusto von Dentz

Re: Temporary device removal during discovery

[Andrey Batyiev]

Hi Luiz,

On Thu, Jul 9, 2020 at 12:14 AM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> The delta logic might be a nice addition as a separate patch, it is
> more for detecting devices disappearing then actually cleanup during
> power off.
No-no, it's not about adapter powering off.

I meant that (external) devices never disappear from the bluez device
list during the discovery,
even if the (external) devices are turned off (i.e. they should be
purged by bluez).

So:
- bluez is central
- bluez is discovering
- peripheral appear for a moment, than disappear (i.e. peripheral
would be turned off)
- bluez would not remove device from the list (at least until
discovery is stopped)

Use case:
- bluez is monitoring environment (discovering literally forever)
- peripherals are brought in and out of bluez visibility range
- bluez list of visible devices grows infinitely and causes problems
(hundreds of devices)


Thanks,
   Andrey

Re: Temporary device removal during discovery

[Luiz Augusto von Dentz]

Hi Andrey,

On Wed, Jul 8, 2020 at 3:57 PM Andrey Batyiev <batyiev@gmail.com> wrote:
>
> Hi Luiz,
>
> On Thu, Jul 9, 2020 at 12:14 AM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> > The delta logic might be a nice addition as a separate patch, it is
> > more for detecting devices disappearing then actually cleanup during
> > power off.
> No-no, it's not about adapter powering off.
>
> I meant that (external) devices never disappear from the bluez device
> list during the discovery,
> even if the (external) devices are turned off (i.e. they should be
> purged by bluez).
>
> So:
> - bluez is central
> - bluez is discovering
> - peripheral appear for a moment, than disappear (i.e. peripheral
> would be turned off)
> - bluez would not remove device from the list (at least until
> discovery is stopped)
>
> Use case:
> - bluez is monitoring environment (discovering literally forever)
> - peripherals are brought in and out of bluez visibility range
> - bluez list of visible devices grows infinitely and causes problems
> (hundreds of devices)

That is exactly what I mean with detecting devices disappearing, so
I'm fine to introduce such logic and use the temporary timeout so it
can be removed while an existing discovery is in place.

-- 
Luiz Augusto von Dentz

Re: Temporary device removal during discovery

[Bastien Nocera]

On Thu, 2020-07-09 at 01:57 +0300, Andrey Batyiev wrote:
> Hi Luiz,
> 
> On Thu, Jul 9, 2020 at 12:14 AM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> > The delta logic might be a nice addition as a separate patch, it is
> > more for detecting devices disappearing then actually cleanup
> > during
> > power off.
> No-no, it's not about adapter powering off.
> 
> I meant that (external) devices never disappear from the bluez device
> list during the discovery,
> even if the (external) devices are turned off (i.e. they should be
> purged by bluez).
> 
> So:
> - bluez is central
> - bluez is discovering
> - peripheral appear for a moment, than disappear (i.e. peripheral
> would be turned off)
> - bluez would not remove device from the list (at least until
> discovery is stopped)
> 
> Use case:
> - bluez is monitoring environment (discovering literally forever)
> - peripherals are brought in and out of bluez visibility range
> - bluez list of visible devices grows infinitely and causes problems
> (hundreds of devices)

I'd also expect devices that are recently discovered to disappear if
they haven't replied to a discovery in 30 seconds. It would stop
GNOME's Bluetooth Settings's Bluetooth list forever expanding.

Or we need to give the ability for front-ends to do that by exporting
the "last seen" dates.

Cheers

Re: Temporary device removal during discovery

[Luiz Augusto von Dentz]

Hi Bastien,

On Thu, Jul 9, 2020 at 1:26 AM Bastien Nocera <hadess@hadess.net> wrote:
>
> On Thu, 2020-07-09 at 01:57 +0300, Andrey Batyiev wrote:
> > Hi Luiz,
> >
> > On Thu, Jul 9, 2020 at 12:14 AM Luiz Augusto von Dentz
> > <luiz.dentz@gmail.com> wrote:
> > > The delta logic might be a nice addition as a separate patch, it is
> > > more for detecting devices disappearing then actually cleanup
> > > during
> > > power off.
> > No-no, it's not about adapter powering off.
> >
> > I meant that (external) devices never disappear from the bluez device
> > list during the discovery,
> > even if the (external) devices are turned off (i.e. they should be
> > purged by bluez).
> >
> > So:
> > - bluez is central
> > - bluez is discovering
> > - peripheral appear for a moment, than disappear (i.e. peripheral
> > would be turned off)
> > - bluez would not remove device from the list (at least until
> > discovery is stopped)
> >
> > Use case:
> > - bluez is monitoring environment (discovering literally forever)
> > - peripherals are brought in and out of bluez visibility range
> > - bluez list of visible devices grows infinitely and causes problems
> > (hundreds of devices)
>
> I'd also expect devices that are recently discovered to disappear if
> they haven't replied to a discovery in 30 seconds. It would stop
> GNOME's Bluetooth Settings's Bluetooth list forever expanding.
>
> Or we need to give the ability for front-ends to do that by exporting
> the "last seen" dates.

I am fine with that as well, 30 seconds doesn't sound too bad even for
cleanup temporary devices as the current 3 minutes seems awful long
sometimes, we could perhaps have a filter for configuring that though
so if the application wants to have its own timeout, the only problem
is if there are multiple application doing that on parallel we would
need a strategy on how to handle that.

-- 
Luiz Augusto von Dentz

Re: Temporary device removal during discovery

[Bastien Nocera]

On Fri, 2020-07-10 at 11:06 -0700, Luiz Augusto von Dentz wrote:
> Hi Bastien,
> 
> On Thu, Jul 9, 2020 at 1:26 AM Bastien Nocera <hadess@hadess.net>
> wrote:
> > On Thu, 2020-07-09 at 01:57 +0300, Andrey Batyiev wrote:
> > > Hi Luiz,
> > > 
> > > On Thu, Jul 9, 2020 at 12:14 AM Luiz Augusto von Dentz
> > > <luiz.dentz@gmail.com> wrote:
> > > > The delta logic might be a nice addition as a separate patch,
> > > > it is
> > > > more for detecting devices disappearing then actually cleanup
> > > > during
> > > > power off.
> > > No-no, it's not about adapter powering off.
> > > 
> > > I meant that (external) devices never disappear from the bluez
> > > device
> > > list during the discovery,
> > > even if the (external) devices are turned off (i.e. they should
> > > be
> > > purged by bluez).
> > > 
> > > So:
> > > - bluez is central
> > > - bluez is discovering
> > > - peripheral appear for a moment, than disappear (i.e. peripheral
> > > would be turned off)
> > > - bluez would not remove device from the list (at least until
> > > discovery is stopped)
> > > 
> > > Use case:
> > > - bluez is monitoring environment (discovering literally forever)
> > > - peripherals are brought in and out of bluez visibility range
> > > - bluez list of visible devices grows infinitely and causes
> > > problems
> > > (hundreds of devices)
> > 
> > I'd also expect devices that are recently discovered to disappear
> > if
> > they haven't replied to a discovery in 30 seconds. It would stop
> > GNOME's Bluetooth Settings's Bluetooth list forever expanding.
> > 
> > Or we need to give the ability for front-ends to do that by
> > exporting
> > the "last seen" dates.
> 
> I am fine with that as well, 30 seconds doesn't sound too bad even
> for
> cleanup temporary devices as the current 3 minutes seems awful long
> sometimes, we could perhaps have a filter for configuring that though
> so if the application wants to have its own timeout, the only problem
> is if there are multiple application doing that on parallel we would
> need a strategy on how to handle that.

In which case it might be easier to export that last seen date,
otherwise that's just a lot of moving parts inside bluetoothd when we
could filter it trivially in the front-ends.

Re: Temporary device removal during discovery

[Luiz Augusto von Dentz]

Hi Bastien,

On Fri, Jul 10, 2020 at 12:00 PM Bastien Nocera <hadess@hadess.net> wrote:
>
> On Fri, 2020-07-10 at 11:06 -0700, Luiz Augusto von Dentz wrote:
> > Hi Bastien,
> >
> > On Thu, Jul 9, 2020 at 1:26 AM Bastien Nocera <hadess@hadess.net>
> > wrote:
> > > On Thu, 2020-07-09 at 01:57 +0300, Andrey Batyiev wrote:
> > > > Hi Luiz,
> > > >
> > > > On Thu, Jul 9, 2020 at 12:14 AM Luiz Augusto von Dentz
> > > > <luiz.dentz@gmail.com> wrote:
> > > > > The delta logic might be a nice addition as a separate patch,
> > > > > it is
> > > > > more for detecting devices disappearing then actually cleanup
> > > > > during
> > > > > power off.
> > > > No-no, it's not about adapter powering off.
> > > >
> > > > I meant that (external) devices never disappear from the bluez
> > > > device
> > > > list during the discovery,
> > > > even if the (external) devices are turned off (i.e. they should
> > > > be
> > > > purged by bluez).
> > > >
> > > > So:
> > > > - bluez is central
> > > > - bluez is discovering
> > > > - peripheral appear for a moment, than disappear (i.e. peripheral
> > > > would be turned off)
> > > > - bluez would not remove device from the list (at least until
> > > > discovery is stopped)
> > > >
> > > > Use case:
> > > > - bluez is monitoring environment (discovering literally forever)
> > > > - peripherals are brought in and out of bluez visibility range
> > > > - bluez list of visible devices grows infinitely and causes
> > > > problems
> > > > (hundreds of devices)
> > >
> > > I'd also expect devices that are recently discovered to disappear
> > > if
> > > they haven't replied to a discovery in 30 seconds. It would stop
> > > GNOME's Bluetooth Settings's Bluetooth list forever expanding.
> > >
> > > Or we need to give the ability for front-ends to do that by
> > > exporting
> > > the "last seen" dates.
> >
> > I am fine with that as well, 30 seconds doesn't sound too bad even
> > for
> > cleanup temporary devices as the current 3 minutes seems awful long
> > sometimes, we could perhaps have a filter for configuring that though
> > so if the application wants to have its own timeout, the only problem
> > is if there are multiple application doing that on parallel we would
> > need a strategy on how to handle that.
>
> In which case it might be easier to export that last seen date,
> otherwise that's just a lot of moving parts inside bluetoothd when we
> could filter it trivially in the front-ends.
>
Right, we still need to clean up the temporary devices though, but
perhaps adding a LastSeen property is not a bad idea after all so
applications can do this sort of filtering themselves.

-- 
Luiz Augusto von Dentz