Libdivecomputer Android API discussion

[Jef Driesen]

On 2014-06-25 14:22, Venkatesh Shukla wrote:
> I have been working on bringing libdivecomputer on android.
> I have started with support for ftdi-chipset based divecomputers. For
> talking with ftdi based devices on android, I have implemented
> serial_ftdi.c. . I have tried to implement all the the necessary 
> functions.
> Though some of them are left out due to their unavailability in 
> libftdi.
> 
> These left out functions are
> 
>    1. serial_set_break  - This is used only in reefnet_sensus_pro
>    hanshaking. I do not know if it uses ftdi chipset. If it does, it 
> would
>    become important to implement this function. In the present 
> implementation,
>    it remains unusable.

Reefnet uses the Prolific pl2303 chipset, so it's not really an issue. 
The Sensus Pro is a pretty outdated model too, so there will be very few 
people who still have one.

>    2. serial_send_break - I could not find any usage of this function 
> in
>    libdivecomputer.

Correct. It's not used anywhere.

>    3. serial_get_transmitted - Even though the received bytes in the 
> buffer
>    can be accessed, I couldn't find any way to get the transmitted 
> bytes. This
>    function is used internally for logging. Returns -1 as it is not
>    implemented.

It doesn't return the number of transmitted bytes, but the number of 
bytes that are currently present in the serial port buffer. Nowadays, we 
always call tcdrain when writing data, so this value should always be 
zero. This used to be different. Anyway, it's not an important function 
and it might even get removed in the future. So an implementation that 
always return 0 would be fine.

> Changes made for building libdivecomputer for android are
> 
>    1. Check for presence of libftdi1
>    2. serial_ftdi.c is compiled and used instead of serial_posix.c
>    3. libftdi1 is included in Libs.private in libdivecomputer.pc
> 
> All these changes can be viewed here
> <https://github.com/venkateshshukla/libdivecomputer>. I have been 
> testing
> it with universal script and subsurface-android. Results have been
> encouraging.

I have tested your code on Linux, with an OSTC3 (or was it an OSTC2, 
don't remember), and it worked fine. I didn't had time to review it 
properly.

> Now, toward providing a workable API for android, I have following 
> points
> in mind. These were mentioned by Jef in an earlier mail.
> 
>    1. Support for only ftdi devices are implemented for now. But 
> support
>    for other chipsets need to be included later on. Hence, 
> serial_ftdi.c alone
>    is not enough. We need a serial_android.c which will redirect calls 
> to
>    respective chipset specific scripts.
>    2. I have come to the conclusion that for android, it will be best 
> if we
>    pass the file descriptor of the USB device after acquiring the 
> necessary
>    permissions. This fd can be used to create a libusb_device and 
> subsequently
>    all USB related features can be accessed. For making a libusb_device 
> from
>    file descriptor obtained from android, I had tweaked libusb. This 
> tweaked
>    version is present here ( https://github.com/venkateshshukla/libusb 
> ).
>    This libusb_device could be used to obtain VID and PID and then 
> control can
>    then be branched accordingly to ftdi, pl2303, cp210x and other 
> chipset
>    specific scripts on android.
>    3. We will require a generalized mechanism for passing parameters. 
> This
>    is especially needed now as we have several branches - Normal TTY 
> usage (
>    posix/win32 , path to the device), android (file descriptor) and 
> Bluetooth
>    address.
> 
> Please provide your thoughts on the above points. Also any other thing 
> that
> I should keep in mind for android API?

Originally, the solution I had in mind for the different transport types 
(serial, irda, usb and bluetooth), was to introduce a specific data 
structure for each transport type:

typedef dc_params_serial_t {
     const char *devname;
} dc_params_serial_t;

typedef dc_params_usb_t {
     unsigned int vid;
     unsigned int pid;
} dc_params_usb_t;

typedef dc_params_bluetooth_t {
     unsigned char address[6];
} dc_params_bluetooth_t;

dc_status_t
dc_device_open (..., dc_descriptor_t *descriptor, const void *params);

The application would then have to pass the appropriate data structure 
to the dc_device_open function, depending on the transport type 
indicated by the device descriptor:

dc_params_serial_t serial;
dc_params_usb_t usb;
dc_params_bluetooth_t bluetooth;

switch (dc_descriptor_get_transport(descriptor)) {
case DC_TRANSPORT_SERIAL:
     serial.devname = ...;
     dc_device_open(..., descriptor, &serial);
     break;
case DC_TRANSPORT_USB:
     usb.vid = ...;
     usb.pid = ...;
     dc_device_open(..., descriptor, &usb);
     break;
case DC_TRANSPORT_BLUETOOTH:
     bluetooth.address = ...;
     dc_device_open(..., descriptor, &bluetooth);
     break;
}

Although this will certainly work, there are some drawbacks:

Ideally libdivecomputer should provide some api to enumerate serial, usb 
and bluetooth devices. When the user chooses a device that uses serial 
communication, you want to be able to show a list with all serial ports. 
The same goes for usb and bluetooth communication. But for some 
transport types like irda sockets (and maybe also bluetooth sockets) you 
need to open a socket before you can enumerate the available devices. 
But as long as the backend code opens the socket internally, this 
functionality can't be exposed to the application. Currently the USB 
(Atomics Cobalt) and IrDA (Uwatec Smart) based backends do work around 
this problem by performing the discovery internally, and pick the device 
that looks like the dive computer (e.g based on its name, USB VID/PID, 
etc). But this heuristics isn't perfect and can certainly fail. For 
example when connecting two identical dive computers, or when some other 
usb-serial gadget with the same USB VID/PID (not unlikely with simple 
usb-serial cables using the stock ftdi/pl2303 VID/PID) is connected. The 
heuristics will have to decide which device to pick (e.g. the first 
one), but that might be the wrong choice. Yes, I'm aware this is a 
corner case, but not impossible.

It's also not clear to me how this solution should deal with multiple 
implementations of a single transport. For Android, there'll be multiple 
implementations for serial communication, one for each chipset (ftdi, 
pl2303, cdc-acm, etc). But how do you decide which chipset to pick? The 
device backend doesn't always have this information. For example for 
Suunto devices,there are several cables on the market using different 
usb-serial chipsets. So there is no one to one mapping between devices 
and usb-serial chipsets.


The next thing that comes to my mind is a solution where the application 
opens the appropriate "transport "itself, and passes an instance of an 
abstract transport object to the dc_device_open function:

dc_serial_t *serial = NULL;
dc_usb_t *usb = NULL;
dc_bluetooth_socket_t *bluetooth = NULL;

switch (dc_descriptor_get_transport(descriptor)) {
case DC_TRANSPORT_SERIAL:
#ifdef ANDROID
     dc_serial_{ftdi,pl2303,...}_open(&serial, fd);
#else
     dc_serial_open(&serial, name);
#endif
     dc_device_open(..., descriptor, serial);
     break;
case DC_TRANSPORT_USB:
     dc_usb_open(&usb, vid, pid);
     dc_device_open(..., descriptor, usb);
     break;
case DC_TRANSPORT_BLUETOOTH:
     dc_bluetooth_socket_open(&bluetooth, ...);
     dc_bluetooth_socket_discovery(bluetooth, ...);
     dc_bluetooth_socket_connect(bluetooth, address);
     dc_device_open(..., descriptor, bluetooth);
     break;
}

This would allow multiple implementations of a transport, as long as 
each of them implements the same interface, but that's easy. It also 
solves the discovery problem, because the low-level transport interface 
is now available to the application.

But unfortunately it also introduces a new problem. Take a look at the 
USB transport. Now the application suddenly needs to know which USB 
VID/PID to use. Previously the device backend took care of that 
internally (with the caveat mentioned earlier). For example the Atomic 
Cobalt uses the VID/PID 0x0471/0x0888. Right now the application doesn't 
need to know this, because the backend opens the usb device that matches 
these number. The same thing happens for Uwatec Smart, where the backend 
discovers the IrDA devices in range, and connects to first one that 
matches the known device names. How do we deal with this here?

All ideas are welcome!

Jef