| Message ID | 20200817135606.Bluez.v2.1.I21d21871d85db48b07ba847742c7cb933024307c@changeid (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Luiz Von Dentz |
| Headers | show |
| Series | [Bluez,v2] adapter- Device needs to be in the temporary state after pairing failure | expand |
Hi Yu Liu, On Mon, Aug 17, 2020 at 4:04 PM Yu Liu <yudiliu@google.com> wrote: > > This caused the device hanging around as a discovered device forever > even if it is turned off or not in present. > > Reviewed-by: sonnysasaka@chromium.org > > Signed-off-by: Yu Liu <yudiliu@google.com> > --- > > Changes in v2: > - Fix title length and format issue > > Changes in v1: > - Initial change > > src/device.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/src/device.c b/src/device.c > index bb8e07e8f..93e71850c 100644 > --- a/src/device.c > +++ b/src/device.c > @@ -6008,6 +6008,12 @@ void device_bonding_complete(struct btd_device *device, uint8_t bdaddr_type, > > if (status) { > device_cancel_authentication(device, TRUE); > + > + // Put the device back to the temporary state so that it will be > + // treated as a newly discovered device. Use C style comments /* */ above. > + if (!device_is_paired(device, bdaddr_type)) > + btd_device_set_temporary(device, true); Hmm, are we perhaps removing the temporary flag too early? Or this is a result of calling Connect which clears the temporary flag? Then perhaps we should at least if the upper layer has marked the device as trusted as it should indicate the device object should be kept even if not paired. > device_bonding_failed(device, status); > return; > } > -- > 2.28.0.220.ged08abb693-goog >
that could be a reason and a potential fix, we remove the temp flag in dev_connect and pair_device very early on, but i suspect changing that would potentially have bigger impact and needs more due diligence and testing. On Mon, Aug 17, 2020 at 4:24 PM Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote: > > Hi Yu Liu, > > On Mon, Aug 17, 2020 at 4:04 PM Yu Liu <yudiliu@google.com> wrote: > > > > This caused the device hanging around as a discovered device forever > > even if it is turned off or not in present. > > > > Reviewed-by: sonnysasaka@chromium.org > > > > Signed-off-by: Yu Liu <yudiliu@google.com> > > --- > > > > Changes in v2: > > - Fix title length and format issue > > > > Changes in v1: > > - Initial change > > > > src/device.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/src/device.c b/src/device.c > > index bb8e07e8f..93e71850c 100644 > > --- a/src/device.c > > +++ b/src/device.c > > @@ -6008,6 +6008,12 @@ void device_bonding_complete(struct btd_device *device, uint8_t bdaddr_type, > > > > if (status) { > > device_cancel_authentication(device, TRUE); > > + > > + // Put the device back to the temporary state so that it will be > > + // treated as a newly discovered device. > > Use C style comments /* */ above. > > > + if (!device_is_paired(device, bdaddr_type)) > > + btd_device_set_temporary(device, true); > > Hmm, are we perhaps removing the temporary flag too early? Or this is > a result of calling Connect which clears the temporary flag? Then > perhaps we should at least if the upper layer has marked the device as > trusted as it should indicate the device object should be kept even if > not paired. > > > device_bonding_failed(device, status); > > return; > > } > > -- > > 2.28.0.220.ged08abb693-goog > > > > > -- > Luiz Augusto von Dentz
Hi Luiz, I just sent another patch to address your comments. The issue this cl is trying to address is that when we pair a device by calling pair_device, it removes the temporary flag very early on, then it the pairing failed due to reasons such as failed to connect the temp flag won't be removed and the device will be hanging around forever. Please review again. Thanks. On Mon, Aug 17, 2020 at 4:24 PM Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote: > > Hi Yu Liu, > > On Mon, Aug 17, 2020 at 4:04 PM Yu Liu <yudiliu@google.com> wrote: > > > > This caused the device hanging around as a discovered device forever > > even if it is turned off or not in present. > > > > Reviewed-by: sonnysasaka@chromium.org > > > > Signed-off-by: Yu Liu <yudiliu@google.com> > > --- > > > > Changes in v2: > > - Fix title length and format issue > > > > Changes in v1: > > - Initial change > > > > src/device.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/src/device.c b/src/device.c > > index bb8e07e8f..93e71850c 100644 > > --- a/src/device.c > > +++ b/src/device.c > > @@ -6008,6 +6008,12 @@ void device_bonding_complete(struct btd_device *device, uint8_t bdaddr_type, > > > > if (status) { > > device_cancel_authentication(device, TRUE); > > + > > + // Put the device back to the temporary state so that it will be > > + // treated as a newly discovered device. > > Use C style comments /* */ above. > > > + if (!device_is_paired(device, bdaddr_type)) > > + btd_device_set_temporary(device, true); > > Hmm, are we perhaps removing the temporary flag too early? Or this is > a result of calling Connect which clears the temporary flag? Then > perhaps we should at least if the upper layer has marked the device as > trusted as it should indicate the device object should be kept even if > not paired. > > > device_bonding_failed(device, status); > > return; > > } > > -- > > 2.28.0.220.ged08abb693-goog > > > > > -- > Luiz Augusto von Dentz
diff --git a/src/device.c b/src/device.c index bb8e07e8f..93e71850c 100644 --- a/src/device.c +++ b/src/device.c @@ -6008,6 +6008,12 @@ void device_bonding_complete(struct btd_device *device, uint8_t bdaddr_type, if (status) { device_cancel_authentication(device, TRUE); + + // Put the device back to the temporary state so that it will be + // treated as a newly discovered device. + if (!device_is_paired(device, bdaddr_type)) + btd_device_set_temporary(device, true); + device_bonding_failed(device, status); return; }