On 2017-06-22 02:49, Linus Torvalds wrote:
On Tue, Jun 20, 2017 at 11:59 PM, Jef Driesen <[email protected]>
wrote:
You are right that the USB HID packets are vendor specific, and we
can't
incorporate that in the usbhid code. But that's not really what the
I/O
refactoring is about. It only handles how those packets are being
send/received, not their content.
Ok. That will work. In fact, it's what I'm doing for the BLE support
to make USB HID and BLE be real choices.
But it almost certainly has to be integrated with the "custom serial"
kind of model that allows the *outside* to set the packet sending
details, rather than being something internal to libdivecomputer.
And that's because libdivecomputer almost certainly will never be able
to do BLE sanely. Sending packets over GATT a very complex and nasty
protocol, not anything like the fairly straightforward "serial line
over bluetooth" that rfcomm is.
We're using QtConnectivity in subsurface for BLE (well, in my
experimental patches), and while I'm making some progress, we've
actually found two major issues in QtConnectivity already exactly
because BLE is very complex. And that was after I found some issues in
Bluez earlier just to pair the Suunto EON Steel in the first place.
So I do have a set of three patches that does this, and turns the
"custom_serial" thing into a "custom_io" thing (that allows both a
serial _and_ a packet interface), and lets devices use that.
I then made a simpe helper to say "if no custom IO has been defined,
create a custom IO definition for this USB HID device", and converted
both the Suunto EON Steel and the Scubapro G2 profile over to that
helper.
That actually allows me to feed in a custom packet handling set of
routines, and I'm now experimenting with BLE, but am hitting
QtConnectivity issues.
You can see my libdvicecomputer work at
git://github.com/torvalds/libdc-for-dirk.git Subsurface-branch
where the top three commits do that packet interface (it needs a
subsurface patch to then work with subsurface, which is why those
three patches haven't been pushed out to the real repo yet - I'll need
to synchronize with Dirk).
NOTE! It should be *trivial* to do the same kind of thing for a serial
line as I did for USB HID, ie a dive computer backend that sends
"packets" over a serial line could do a very similar "open serial line
with these settings" and then create a packet-sending custom_io
handler for that. So long-term, I'd actually like to remove the
"serial" part of the custom_io struct entirely. But I left it alone
for now, just to minimize the churn, and because I really just want to
work on the BLE side.
This all sounds very similar to the I/O refactoring I proposed. Minus
the packetization stuff of course (because that wasn't necessary yet at
that time). But that's trivial to add. We only need to expose the packet
size (just like you did), so you can use the read/write functions with
the right size.
I don't think it's that easy to completely get rid of the serial
communication stuff. There are several backends that need to change the
serial line settings and DTR/RTS lines during the communication. That's
why those functions are present in the abstract I/O interface, even if
they only apply to serial communication. My reasoning was those
functions can be implemented as stubs for I/O implementations where they
don't make sense.
Note: if we're lucky and the Bluetooth Low Energy (BLE) uses the same
type
of packetization as their USB HID implementation, then we just need to
swap
the I/O layer, and we're done.
From what I've seen, they are very similar at least for both the EON
Steel and the Scubapro G2.
But there is one big difference: the packet sizes are different for
BLE than from USB HID.
USB HID uses 64-byte packets, while BLE uses 32-byte packets but has
various other BLE headers.
In fact, I think there's only really 20 bytes of payload per packet in
GATT, and that the packetization is entirely different - it looks like
BLE over GATT acts more like a "stream" than the very obvious HID
packetization that at least the Suunto EON Steel is very obviously
very aware of.
So at a minimum, the divecomputer-specific download code needs to know
the packetization boundaries, in order to handle that correctly. Which
is why my custom_io definition has five fields: one field for the
packet size, and four fields for open/close/read/write functions
respectively.
But that *may* be the only real difference. Due to my lack of success
in getting BLE working yet, I can't guarantee there aren't other
issues.
If the differences are bigger than just a difference in the packet size,
then we can probably still deal with that with a minimal amount of
transport specific knowledge in the backend.
Take for example again the three uwatec variants: smart (IrDA), meridian
(usb-serial with some additional framing) and g2 (USB HID and BLE GATT
with their own packetization). Since the high level parts of the
protocol are the same, we could implement this as a single backend with
some function pointer for the transfer function:
typedef dc_status_t (*uwatec_smart_transfer_t) (uwatec_smart_device_t
*device, const unsigned char command[], unsigned int csize, unsigned
char answer[], unsigned int asize);
typedef struct uwatec_smart_device_t {
...
uwatec_smart_transfer_t transfer;
} uwatec_smart_device_t;
dc_status_t
uwatec_smart_device_open (dc_device_t **out, dc_context_t *context,
dc_iostream_t *iostream)
{
uwatec_smart_device_t *device;
...
switch (dc_iostream_get_type(iostream)) {
case DC_TRANSPORT_IRDA:
device->transport = irda_transfer;
break;
case DC_TRANSPORT_SERIAL:
device->transport = serial_transfer;
break;
case DC_TRANSPORT_USBHID:
device->transport = usbhid_transfer;
break;
case DC_TRANSPORT_BLE:
device->transport = ble_transfer;
break;
}
...
}
This way, the dive computer backend needs to be aware how the data
packets are structured, but it doesn't need to know how they are send
out. That's still up to the I/O implementation.
Thus if you implement a custom transport, you just need to indicate the
correct transport type, and you'll get the right type of packets. So for
testing purpose you can easily implement a transport that pretends to do
BLE, and then send out the data over a serial line (or a tcp/ip socket,
etc). (That's in fact how I test the smart backend against the simulator
without having to deal with IrDA.)
Jef
_______________________________________________
subsurface mailing list
[email protected]
http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
