[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: libhci update



On Wed, 15 Apr 2009, Maksim Yevmenkin wrote:

> please find attached patch that implements more compatibility shims.
> any comments are greatly appreciated.

+int
+bt_devsend(int s, uint16_t ogf, uint16_t ocf, int plen, void *param)

One thing that I did when writing the NetBSD stack which IMHO made source
somewhat cleaner was provide HCI_CMD_xxxx definintions that included the
OGF and OCF directly, eg

#define HCI_OGF_LINK_CONTROL			0x01
#define HCI_OCF_INQUIRY				0x0001
#define HCI_CMD_INQUIRY				0x0401

It could be considered a remote possibility of a namespace conflict (ie
same command in different groups) but I doubt that it will ever happen
(and would be easily handled). That would make this into

+int
+bt_devsend(int s, uint16_t cmd, int plen, void *param)

thoughts?

Also, plen should be a size_t and it should come after the pointer, see
write, read, memcpy, snprintf etc for prior art :)

+int
+bt_devrecv(int s, uint8_t *buf, int size, time_t to)

and ditto for size_t here

+int
+bt_devinquiry(char const *devname, int length, int num_rsp,
+		uint8_t const *lap, struct bt_devinquiry **ii)

I wonder if allowing for a different LAP is at all useful?  I would say,
for the very remote possibility that somebody wants to do that, they could
just cut and past the code..

Also with inquiry, would it make sense to just pass a time_t and calculate
the 'inquiry length' internally?

+struct bt_devinquiry {
+	bdaddr_t	bdaddr;
+	uint8_t		pscan_rep_mode;
+	uint8_t		pscan_period_mode;
+	uint8_t		pscan_mode;
+	uint8_t		dev_class[3];
+	uint16_t	clock_offset;
+};

Does this structure need to be a direct copy of the inquiry result?
page_scan_period_mode and page_scan_mode are deprecated since a long time
so its probably not worth providing the values (most devices return 0
that I've seen).

We also need to [be able] to handle the inquiry-result-with-rssi which
gives an extra int8_t RSSI value, and the 2.1 extended inquiry result
data. For both of those, I think its ok if they are zero if not provided.
(ie actual support can be added later)

struct bt_devinquiry {
	bdaddr_t	bdaddr;
	uint8_t		pscan_rep_mode;
	uint8_t		class[HCI_CLASS_SIZE];
	uint16_t	clock_offset;
	int8_t		rssi;
	uint8_t		data[HCI_EXTENDED_INQUIRY_DATA_SIZE];
};

?

+int
+bt_devfilter(int s, struct bt_devfilter const *new, struct bt_devfilter *old)

And finally, the HCI filter is slightly different in NetBSD (I provided
independent PKT and EVT filters each of 256 bits) and I'm going to think
about that..

regards,
iain
_______________________________________________
freebsd-bluetooth_(_at_)_freebsd_(_dot_)_org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-bluetooth
To unsubscribe, send any mail to "freebsd-bluetooth-unsubscribe_(_at_)_freebsd_(_dot_)_org"