Date: Tue, 21 Apr 2009 11:53:22 +0100 (BST) From: Iain Hibbert <plunky@rya-online.net> To: Maksim Yevmenkin <maksim.yevmenkin@gmail.com> Cc: freebsd-bluetooth@freebsd.org Subject: Re: libhci update Message-ID: <1240311202.361300.1366.nullmailer@galant.ukfsn.org> In-Reply-To: <bb4a86c70904201053y1a04d76el336432d3e1a23576@mail.gmail.com> References: <E1Lv5La-00058x-HH@smtpbarns01> <bb4a86c70904201053y1a04d76el336432d3e1a23576@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, 20 Apr 2009, Maksim Yevmenkin wrote: > >> thanks for the feedback. i'm attaching revisited patch. please take a > >> look and let me know if this is something you happy with. > > > > (pls ignore typos & bad formatting, am on mobile device :) > > > > Bt_devrecv() should probably take void * for buffer? Also manpage suggests it > > will only receive hci events. Do you think its worth doing some validation of received > > data? (eg return EIO for truncated reads?) > > i will change uint8_t * to void *, no problem. i also updated man page > and removed 'event' word :) Ok also in bt_devrecv() - return type should be ssize_t - size cannot be < 0 (use "size == 0")? and what of validation of received data here? It could be worth checking that the buffer is not truncated so that the caller does not have to worry about it, but that does require checking the packet type.. > > Bt_devreq() needs to set/restore a filter too > > well, maybe. bt_devreq() operates on already opened socket. the > assumption i'm making here is that application will set appropriate > filter before calling bt_devreq(). otherwise, application would have > to always set 'event' field to acceptable value (or zero). i could go > either way here. just need to document implemented behavior better. Mm, its a good point - there are arguments either way (bloat vs guaranteed success) but I think since the difference between devreq() and devrecv() is that devreq() handles all the fiddly details for you, I think its worth doing that aswell.. > > Do you think its worth to cook dev_class into a normalised host > > numeric value rather than 3 bytes ? > > right, hmm, i guess we could turn dev_class into uint32_t in le16 byte > order (since everything else in bluetooth hci is in le16 order), but > maybe its too much? spec breaks out dev_class as 3 bytes array and talks > about bytes and bits within each byte. i'm kinda leaning towards leaving > it as is, because otherwise application would have to translate byte/bit > into uint32_t mask. it could be somewhat convenient, i guess. again, no > strong feeling about it. could go either way. My thought was that some of the 'fields' do seem to cross over into multiple bytes (eg the service class flags in the default format type) and perhaps its easier to interpret as a single value where bits can be tested in host order by such as (class & (1 << N))? I don't know how much that helps but the assigned numbers document does seem to show it as a bit array. (I'm not having a strong opinion either :) > > Probably need to specifically mention that the inquiry response to be > > released using free() in manpage? > > it says so, i.e. > > The > .Fa ii > parameter specifies where to place inquiry results. > On success, the function will return total number of inquiry results, > will allocate buffer to store all the inquiry results and > will return pointer to the allocated buffer in the > .Fa ii > parameter. > It is up to the caller of the function to dispose of the buffer. No, I mean to specifically mention ".Xr free 3" as the method used to dispose of the buffer. > > Something i thought about on the train yesterday but can't visualise > > on the small screen. For device inquiry did you consider using a > > callback method (as per devenum) in stead of returning the structure > > array? At least i recall my nokia would show the list building (but > > perhaps that was when it got the names) and this windows mobile device > > does show the raw list in progress (though stupidly just displays a > > list of 'unknown device' until it gets the names :) > > yes, i thought about it. the only difference is that it would be not so > close to linux/bluez api. i guess, we could provide another function? I thought about that some yesterday (on another train :) and I think there could be some other problems in any case, eg some devices (at least, maybe all?) can not handle RemoteNameReq etc while in inquiry mode so the amount of things that can be done is minimal - btconfig(8) does print a dot each time a result is returned but I think thats probably about the limit of what is useful. So, perhaps just leave it for now.. if somebody requires something like that its not difficult to cut and paste library code to work something out. iain
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1240311202.361300.1366.nullmailer>