Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 14 May 2009 09:26:18 -0700
From:      Maksim Yevmenkin <maksim.yevmenkin@gmail.com>
To:        Iain Hibbert <plunky@rya-online.net>
Cc:        freebsd-bluetooth@freebsd.org
Subject:   Re: libhci update
Message-ID:  <bb4a86c70905140926n488cb2b5x5f5530e01d70bd66@mail.gmail.com>
In-Reply-To: <1242294653.314348.1748.nullmailer@galant.ukfsn.org>
References:  <E1Lv5La-00058x-HH@smtpbarns01> <bb4a86c70904201053y1a04d76el336432d3e1a23576@mail.gmail.com> <1240311202.361300.1366.nullmailer@galant.ukfsn.org> <bb4a86c70904210959w6de5e808h9f85ee2bb1995dbf@mail.gmail.com> <1240352254.082638.416.nullmailer@galant.ukfsn.org> <bb4a86c70904211651m6127745ao9d4f26c91e428994@mail.gmail.com> <1240386569.369073.696.nullmailer@galant.ukfsn.org> <bb4a86c70904220909j5d047ce6x6260bd2e87b5b7bd@mail.gmail.com> <1242294653.314348.1748.nullmailer@galant.ukfsn.org>

next in thread | previous in thread | raw e-mail | index | archive | help
Iain,

>> thanks for the review. i committed it to -head.
>
> Hi Max, I'm in progress of translating for NetBSD and in bt_devinquiry() I
> see
>
>        if (ii == NULL) {
>                errno = EINVAL;
>                return (-1);
>        }
>
> but I think this test might be bogus? manpage implies that the caller
> provides a buffer but we allocate one..

no, the test is correct. yes, we allocate buffer internally, but we
need to return pointer to the buffer back to the caller. so, ii is
basically ii is struct bt_devinquiry **. perhaps language in the man
page is not clear enough?

>        /* Calculate inquire length in 1.28 second units */
>        to = (time_t) ((double) length / 1.28);
>
>        if (to <= 0)
>                cp->inquiry_length = 4;         /* 5.12 seconds */
>        else if (to > 254)
>                cp->inquiry_length = 255;       /* 326.40 seconds */
>        else
>                cp->inquiry_length = to + 1;
>
> 2.1 spec says 1.28 -> 61.44 seconds range is acceptable (0x01->0x30)
>
> Then, to avoid the floating point arithmetic, can use (to * 100 / 128) but
> would need to be range checked first. I'm inclined to use something like
>
>        if (to == 0)
>                to = 4;
>        else if (to == 1)
>                to = 2;
>        else if (to > 61)
>                to = 61;
>
>        cp->inquiry_length = (uint8_t)(to * 100 / 128);

i have no problem with it.

> also, I'm not sure that the timeout is handled right; the bt_devrecv()
> uses the complete timeout each time but the time endpoint might need to be
> calculated so that time-to-end can be used?

well, yes. bt_devrecv() was envisioned as "one-shot" call. the only
case, imo, where it can restart internally with the same timeout is
when select is interrupted by a signal.

> Also I'm wondering what to do in the case bt_devrecv() does actually time
> out - what can it mean and should we [attempt to] cancel the inquiry?

ahh, but in case of bt_devinquiry() it probably does not matter that
much because inquiry itself is limited by the "length" parameter. so
we have to receive something back from the device. i was thinking to
pass -1 as timeout to bt_devrecv() in this particular case, but
decided not to do it (try harder to exit the bt_devinquiry()).

> (actually the whole timeout thing is probably irrelevant for a properly
> functioning device :)

exactly :)

thanks,
max

p.s. thanks for the sdp update. i will try and take a look when i get
free minute.



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?bb4a86c70905140926n488cb2b5x5f5530e01d70bd66>