On 16 March, 2017 - Dirk Hohndel wrote: > > I'm not sure if everyone who is likely to be interested in this is on the > libdivecomputer mailing list... > Linus, Anton, Lubomir, Robert... if you have comments on the below, please > respond over on > that mailing list (not hear, let's help keep this discussion in one place).
Yes, its on my reading list. For anyone interested, our current Custom serial implementation can be looked on at: https://github.com/glance-/libdivecomputer/commit/94e7a77d01b4af26934a943877b0f832dd88dad8 It looks a bit funny in its places, but its designed to be a merge-friendly bolt on to libdivecomputer. Its what we use to hook our qtserialbluetooth and serial_ftdi code into and below libdivecomputer. Now when I think about it, I think i never sent this code to Jef, because the design was rejected in a previous discussion, and he didn't like our previous custom serial code. //Anton > > Thanks > > /D > > > Begin forwarded message: > > > > From: Jef Driesen <[email protected]> > > Subject: I/O layer refactoring > > Date: March 16, 2017 at 9:45:27 AM PDT > > To: libdivecomputer mailinglist <[email protected]> > > > > Hi, > > > > I'm planning some major non-backwards compatible changes for the next > > release. The first one is a refactoring of the I/O layer to support > > bluetooth communication (and more). The remainder of this email contains a > > description of the problem and a proposal for how I would like to address > > this. > > > > > > Libdivecomputer's low-level I/O layer was pretty much designed with only > > serial communication in mind. This is very obvious if you look at the > > signature of the dc_device_open function: > > > > dc_status_t > > dc_device_open (dc_device_t **device, dc_context_t *context, > > dc_descriptor_t *descriptor, const char *name); > > > > That last parameter is the name of the serial port to open. > > > > But nowadays libdivecomputer does not only support serial communication, > > but also IrDA, USB and USB HID. So far we have been able to work around > > this api limitation. Because for these communication protocols, we can > > autodetect the device based on the IrDA device name, or the USB VID/PID > > numbers. Simply pass NULL as name, and libdivecomputer will take care of > > doing the device enumeration internally. > > > > Unfortunately, that workaround isn't entirely foolproof. For example if you > > have multiple devices connected, libdivecomputer will always pick the first > > device it recognizes as a dive computer. Thus you won't be able to connect > > to the other one(s). And if our heuristic to recognize a dive computer is > > wrong, it won't be able to pick a device at all. In practice, this won't > > cause any trouble because this is pretty rare corner case. Indeed, very few > > users will have two dive computers connected at the same time, and the > > heuristics are reasonably solid (e.g. the IrDA device name and the USB > > VID/PID numbers of the dive computer never change). > > > > But if we're going to add support for bluetooth communication, I wouldn't > > be surprised if the above assumption breaks. Since bluetooth is a very > > popular technology, it's no longer very unlikely to have multiple bluetooth > > devices connected at the same time (e.g. mouse/keyboard, phone, speakers, > > etc). On top of that, the bluetooth device name is often configurable by > > the user, and thus autodetection based on simple heuristics won't work > > anymore. The only way to fix that, is to move the device discovery to the > > application, and let the end-user select the correct device. > > > > That will require to expose the low-level I/O layer in the public api. But > > that alone isn't enough. We also need some way to pass the result of the > > discovery back to the dc_device_open function. Thus the "name" parameter > > needs to replaced with something more generic. The easiest solution would > > be to just pass a void pointer: > > > > dc_status_t > > dc_device_open (dc_device_t **device, dc_context_t *context, > > dc_descriptor_t *descriptor, void *iostream); > > > > And then the actual data type can depend on the communication mechanism: a > > string for serial, a 32bit address with lsap number or service name for > > IrDA, a 48bit address and port number for bluetooth, and so on. > > > > But if we're going to modify the api, we can also take it one step further. > > Why not move the opening and closing of the underlying I/O channel to the > > application, and pass the open connection as the parameter? If we make sure > > that each such I/O channel implements a common interface, then the dive > > computer backends are no longer tied to a specific I/O implementation. This > > has several advantages: > > > > * For bluetooth enabled devices, the application can offer the choice of > > using native bluetooth communication, or the legacy serial communication > > (e.g. the bluetooth serial port emulation mode of the operating system we > > are relying on today). > > > > * We can easily implement new I/O layers. For example a user-space driver > > for usb-serial chipsets (ftdi, pl2303, cp210x, cdc-acm) for use on mobile > > platforms (android, ios), where the kernel drivers are usually not > > available. Or a custom I/O layer, where the actual communication is > > implemented by the application. For the simulator we could a tcp/ip (or > > pipe) based implementation. > > > > There are a few disadvantages as well. First of all, this will of course > > require some extra code on the application side. The bare minimum would be > > something like this: > > > > dc_iostream_t *iostream = NULL; > > dc_device_t *device = NULL > > > > /* Open the communication channel. */ > > switch (type) > > case SERIAL: > > dc_serial_enumerate(...); > > dc_serial_open(&iostream, context, name); > > break; > > case IRDA: > > dc_irda_open(&iostream, context); > > dc_irda_discover(iostream, ...); > > dc_irda_connect_lsap(iostream, address, lsap); > > break; > > case BLUETOOTH: > > dc_bluetooth_open(&iostream, context); > > dc_bluetooth_discover(iostream, ...); > > dc_bluetooth_connect(iostream, address, port); > > break; > > case CUSTOM: > > dc_custom_open(&iostream, context, ...); > > break; > > } > > > > /* Download dives as usual. */ > > dc_device_open(&device, context, descriptor, iostream); > > dc_device_foreach(device, ...); > > dc_device_close(device); > > > > /* Close the communication channel. */ > > dc_iostream_close(iostream); > > > > As you can see in the above pseudo code, it will certainly add some extra > > complexity, because suddenly the application will need some knowledge about > > internal details like the IrDA lsap number and the bluetooth port number. > > > > > > The interface of this new common iostream api would be modeled after the > > serial communication api (see the attached header file). This may seem a > > bit awkward, considered that most of the serial api is meaningless for the > > other implementations. But I don't see any alternative (*). Internally, in > > the dive computer backends, I just want to be able to call the function > > unconditionally, without having to check the type of the underlying I/O > > stream. Other implementations can just leave those functions unimplemented > > (causing the call to fail with DC_STATUS_UNSUPPORTED), or implement it as a > > no-op (always return DC_STATUS_SUCCESS). For IrDA and USB the first option > > will be the obvious choice. But for bluetooth the second option will be > > required in order to support dual serial/bluetooth devices. > > > > (*) I considered moving the serial communication specific functions > > (baudrate, dtr, rts, etc) to an intermediate interface. That would remove > > those functions from implementations where they don't make sense (IrDA and > > USB). But if we want dual serial/bluetooth support, then bluetooth will > > still need to implement that serial interface. So if we need it there > > anyway, then I think it's not worth the extra complexity. > > > > > > The only I/O implementation that doesn't really fit into this model is USB > > communication. USB support three different types of transfers (control, > > bulk and interrupt), while the iostream interface supports only a single > > set of read/write functions. Currently the cobalt is the only backend using > > USB, and it uses a combination of control and bulk transfers. We don't even > > have an abstraction layer and use libusb directly there. I'm not sure what > > would be the best way to deal with that, but maybe we can just leave this > > as-is for now? > > > > > > This will also have an impact on the list of supported devices. At the > > moment, libdivecomputer will exclude devices for which the underlying I/O > > layer isn't available. For example on Mac OS X, the Uwatec Smart dive > > computers are excluded because IrDA isn't supported. But with custom I/O > > layers, that's no longer possible and libdivecomputer will always have to > > report all models. Thus it will be up to the application to restrict the > > list if necessary. The only thing we can do is provide some new api that > > lets the application query whether a built-in I/O layer is available or not. > > > > > > Comments and feedback on the above proposal are welcome! > > > > Jef_______________________________________________ > > devel mailing list > > [email protected] > > http://libdivecomputer.org/cgi-bin/mailman/listinfo/devel > > _______________________________________________ > subsurface mailing list > [email protected] > http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface -- Anton Lundin +46702-161604 _______________________________________________ subsurface mailing list [email protected] http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
