Hi Archie, On Wed, Jul 15, 2020 at 7:15 AM Archie Pusaka <apusaka@xxxxxxxxxx> wrote: > > Hi Luiz, > > If we mark it as temporary, then the device will immediately get > deleted upon disconnection. > https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/src/adapter.c#n6875 > This is the same situation as directly calling device_remove. > > May I know the reason why we want to put the device as temporary? > > If we currently don't have a way to keep a "previously connected but > no longer bonded" device, then removing the device perhaps is the next > best option. It still makes the user scan for the virtually > disconnected device though. We keep a cache of previously known devices, but we only display them once they are actually found otherwise it may grow too big which is inconvenient, I wonder where this concept of "previously connected but no longer bonded" comes from though, we had the temporary to map devices that would not be persisted on the storage which I guess this is what it is about, that said we could perhaps have a timeout before setting it as temporary but we might want to integrate with the logic of detecting devices disappearing. > Thanks, > Archie > > On Wed, 15 Jul 2020 at 01:10, Luiz Augusto von Dentz > <luiz.dentz@xxxxxxxxx> wrote: > > > > Hi Archie, > > > > On Tue, Jul 7, 2020 at 9:30 PM Archie Pusaka <apusaka@xxxxxxxxxx> wrote: > > > > > > Hi Luiz, > > > > > > As far as the spec is concerned, we can also remove the device by > > > calling device_remove. However, I suppose it would be confusing for > > > end users if they can no longer find their HID device on the device > > > list just because the device previously sent a virtual cable > > > disconnection. > > > The HID 1.0 spec part 6.4.2 also gives an example of a possible > > > scenario when a virtually cabled device is removed: "Unplugged devices > > > shall be marked as known and put into a “most recently used list” of > > > known devices to facilitate future re-connecting". > > > > Then perhaps we shall have it marked as temporary as well, that said > > we do want to introduce disappearing logic so temporary devices are > > not left dangling for too long. > > > > > Thanks, > > > Archie > > > > > > > > > On Wed, 8 Jul 2020 at 02:03, Luiz Augusto von Dentz > > > <luiz.dentz@xxxxxxxxx> wrote: > > > > > > > > Hi Archie, > > > > > > > > On Sat, Jun 27, 2020 at 8:54 AM Archie Pusaka <apusaka@xxxxxxxxxx> wrote: > > > > > > > > > > From: Archie Pusaka <apusaka@xxxxxxxxxxxx> > > > > > > > > > > This patch splits the "bonding removal" function in device.c, > > > > > because we need to remove bonding information when receiving > > > > > "virtual cable unplug" in HID profile. > > > > > > > > > > Reviewed-by: Alain Michaud <alainm@xxxxxxxxxxxx> > > > > > --- > > > > > > > > > > Changes in v2: None > > > > > > > > > > src/device.c | 25 +++++++++++++++---------- > > > > > src/device.h | 1 + > > > > > 2 files changed, 16 insertions(+), 10 deletions(-) > > > > > > > > > > diff --git a/src/device.c b/src/device.c > > > > > index 7b0eb256e..9fb0e018c 100644 > > > > > --- a/src/device.c > > > > > +++ b/src/device.c > > > > > @@ -4162,6 +4162,17 @@ static void delete_folder_tree(const char *dirname) > > > > > rmdir(dirname); > > > > > } > > > > > > > > > > +void device_remove_bonding(struct btd_device *device, uint8_t bdaddr_type) > > > > > +{ > > > > > + if (bdaddr_type == BDADDR_BREDR) > > > > > + device->bredr_state.bonded = false; > > > > > + else > > > > > + device->le_state.bonded = false; > > > > > + > > > > > + btd_adapter_remove_bonding(device->adapter, &device->bdaddr, > > > > > + bdaddr_type); > > > > > +} > > > > > + > > > > > static void device_remove_stored(struct btd_device *device) > > > > > { > > > > > char device_addr[18]; > > > > > @@ -4170,17 +4181,11 @@ static void device_remove_stored(struct btd_device *device) > > > > > char *data; > > > > > gsize length = 0; > > > > > > > > > > - if (device->bredr_state.bonded) { > > > > > - device->bredr_state.bonded = false; > > > > > - btd_adapter_remove_bonding(device->adapter, &device->bdaddr, > > > > > - BDADDR_BREDR); > > > > > - } > > > > > + if (device->bredr_state.bonded) > > > > > + device_remove_bonding(device, BDADDR_BREDR); > > > > > > > > > > - if (device->le_state.bonded) { > > > > > - device->le_state.bonded = false; > > > > > - btd_adapter_remove_bonding(device->adapter, &device->bdaddr, > > > > > - device->bdaddr_type); > > > > > - } > > > > > + if (device->le_state.bonded) > > > > > + device_remove_bonding(device, device->bdaddr_type); > > > > > > > > > > device->bredr_state.paired = false; > > > > > device->le_state.paired = false; > > > > > diff --git a/src/device.h b/src/device.h > > > > > index 06b100499..907c7c5c4 100644 > > > > > --- a/src/device.h > > > > > +++ b/src/device.h > > > > > @@ -49,6 +49,7 @@ uint16_t btd_device_get_vendor(struct btd_device *device); > > > > > uint16_t btd_device_get_vendor_src(struct btd_device *device); > > > > > uint16_t btd_device_get_product(struct btd_device *device); > > > > > uint16_t btd_device_get_version(struct btd_device *device); > > > > > +void device_remove_bonding(struct btd_device *device, uint8_t bdaddr_type); > > > > > void device_remove(struct btd_device *device, gboolean remove_stored); > > > > > > > > Is there any particular reason why device_remove is not enough here? I > > > > don't see any reason to leave the device object around after removing > > > > its bonding. > > > > > > > > > int device_address_cmp(gconstpointer a, gconstpointer b); > > > > > int device_bdaddr_cmp(gconstpointer a, gconstpointer b); > > > > > -- > > > > > 2.27.0.212.ge8ba1cc988-goog > > > > > > > > > > > > > > > > > -- > > > > Luiz Augusto von Dentz > > > > > > > > -- > > Luiz Augusto von Dentz -- Luiz Augusto von Dentz
