Fwd: I/O layer refactoring

[Dirk Hohndel]

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).

Thanks

/D

> Begin forwarded message:
> 
> From: Jef Driesen <jef at libdivecomputer.org>
> Subject: I/O layer refactoring
> Date: March 16, 2017 at 9:45:27 AM PDT
> To: libdivecomputer mailinglist <devel at libdivecomputer.org>
> 
> 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
> devel at libdivecomputer.org
> http://libdivecomputer.org/cgi-bin/mailman/listinfo/devel

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.subsurface-divelog.org/pipermail/subsurface/attachments/20170316/182d4075/attachment-0002.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: iostream.h
Type: text/x-c
Size: 9028 bytes
Desc: not available
URL: <http://lists.subsurface-divelog.org/pipermail/subsurface/attachments/20170316/182d4075/attachment-0001.bin>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.subsurface-divelog.org/pipermail/subsurface/attachments/20170316/182d4075/attachment-0003.html>