Hi Marcel,
On Tue, Oct 15, 2013, Marcel Holtmann wrote:
> >>>> + if (cp->val) {
> >>>> + struct hci_cp_write_current_iac_lap cp;
> >>>> +
> >>>> + if (timeout > 0) {
> >>>> + /* Limited discoverable mode */
> >>>
> >>> Wouldn't we want to follow the recommendation from the Core spec
> >>> here, volume 3, part C, section 4.1.2.1:
> >>>
> >>> "A Bluetooth device should not be in limited discoverable mode for
> >>> more than TGAP(104)"
> >>>
> >>> TGAP(104) in turn is defined as 1 minute (vol 3, part C, Appendix A).
> >>
> >> I was not going to be this strict. So yes, userspace can violate
> >> the GAP part here, but it might be a good idea to ensure that when
> >> a too large value is entered we stick with general discoverable
> >> mode. However is this really worth it.
> >
> > For Advertising the max value is actually not just a recommendation
> > but a requirement (check my other mail), so at least there we should
> > probably enforce it. And if we have to enforce it for LE we might as
> > well do it for BR/EDR too. It's anyway a rather simple change to the
> > if-statement.
> >
> >> Another option is to just reject too large timeout values as
> >> invalid parameters and force userspace to implement large timeouts
> >> with general discoverable mode in userspace.
> >
> > I don't think it's a good idea to force this kind of special
> > handling to user space (not just the requirement to know where the
> > limits go but also the need to implement a separate timer).
>
> This sounds also a bit like black magic. Userspace needs to know that
> a small value enters limited discoverable mode and a large one sticks
> with general discoverable mode. We might should have added a flag to
> the command in the first place. However I think our general intention
> was that with a timeout we do limited discoverable mode and without a
> timeout we do general discoverable mode.
>
> I was complaining on why we bothered with a timeout for this parameter
> when building src/share/mgmt.[ch] and cleaning up some of the core
> adapter handling. Mainly because for PairableTimeout D-Bus property we
> have to run our own timer and for DiscoverableTimeout property we rely
> on the kernel. And in the end just running your own timeout was a lot
> simpler, then integrating with the kernel.
>
> So I would be actually fine just saying with timeout that is limited
> discoverable mode. If you want general discoverable mode with a
> timeout, then you have to run this by yourself.
Fair enough, and I really don't want to complicate this more by adding a
new 0x02 value either. The patch has now been pushed to bluetooth-next.
Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
