Subsurface Mobile: Adding mares USB (cp210x) support; how to proceed

[Dirk Hohndel]

Christof,

I'm excited to see the progress!
Unfortunately my 'main' computer is currently being repaired which makes it harder for me to create Android beta builds, but it should be possible to do the testing with builds from the CI system.

> On Mar 6, 2020, at 6:19 AM, Christof Arnosti <charno at charno.ch> wrote:
> 
> So I just added the missing PID / VID and prepared a pull request at https://github.com/Subsurface-divelog/subsurface/pull/2647 <https://github.com/Subsurface-divelog/subsurface/pull/2647>, in the hope that some more people see the changes and maybe install the CI-generated android package so that we can more easily get feedback from people owning other computers.
> 

I will definitely look at that later this afternoon (Pacific Time). I will comment / send questions about the pull request on GitHub.
>>>> Problems:
>>>> - usb read timeout is not working due to an android Bug
>>>> (https://stackoverflow.com/questions/9108548/android-usb-host-bulktransfer-is-losing-data <https://stackoverflow.com/questions/9108548/android-usb-host-bulktransfer-is-losing-data>).
>>>> Currently I start all read-requests without timeout. For Android API
>>>> level 26 (Oreo 8.0), the used awaitRequest()-Method has an added
>>>> timeout-parameter which could be used.
>>>> - Not all dc_custom functionality is implemented. Notably so
>>>> get_available (which seems to be used only optionally) and set_break
>>>> (used by cochran_commander.c and reefnet_sensuspro.c).
>>> No worries, just leave them for now. We can always implement them
>>> later.
>> The problem is less doing the implementation but more missing support in usb-serial-for-android. So to implement the features we would have to change (and possibly upstream) usb-serial-for-android. The maintainer seems to be quite active.


Many of the projects in this space have been very supportive of changes submitted for our use case, so I'd definitely try to go that route.

>>>> TODO:
>>>> - Add known PID/VID pairs at
>>>> https://github.com/charno/subsurface/blob/android-serial-clean/android-mobile/src/org/subsurfacedivelog/mobile/AndroidSerial.java#L94 <https://github.com/charno/subsurface/blob/android-serial-clean/android-mobile/src/org/subsurfacedivelog/mobile/AndroidSerial.java#L94>.
>>>> For this the default probe table has to be modified, which seems quite
>>>> straight-forward. Do you somewhere have a list of PID / VID / Used
>>>> Chipset-thruples?
>>> I think our current scheme where we "open" a virtual device, "ftdi" or
>>> in your code "usb-serial-for-android" is a bad way of doing it. I think
>>> its better if we where to present the list of detected usb devices to
>>> the user, and let the user pick which usb device to connect against.
>>> 
>>> The crazy hunting for vid/pid's in serial_ftdi_open_device is just
>>> begging for trouble in my mind.
>>> 
>>> 
>>> I think open should take a usb bus/device and just use it. It also
>>> solves any issues where a crazy user plugins in multiple devices at once
>>> and then downloads data off one or more of them.
>>> 
>>> That said, the PID/VID lists are in:
>>> https://github.com/Subsurface-divelog/subsurface/blob/master/core/serial_ftdi.c#L153 <https://github.com/Subsurface-divelog/subsurface/blob/master/core/serial_ftdi.c#L153>
>>> https://github.com/Subsurface-divelog/subsurface/blob/master/android-mobile/res/xml/device_filter.xml <https://github.com/Subsurface-divelog/subsurface/blob/master/android-mobile/res/xml/device_filter.xml>
>>> 
>>> Some are documented on https://www.libdivecomputer.org/drivers.html <https://www.libdivecomputer.org/drivers.html> to.
>>> Jef might know if there are more vid/pid's to care about, or if that
>>> list is up to date.
>> This sounds like a good idea. I don't know Qt / qml at all, so that's possibly work for another person.
>> 
>> What I think would be a nice way would be to provide a  list similar to this:
>> - <UsbDeviceName> usb-serial-for-android (autodetect driver)
>> - <UsbDeviceName> usb-serial-for-android (Cp21xx)
>> - <UsbDeviceName> usb-serial-for-android (Ftdi)
>> - <UsbDeviceName> usb-serial-for-android (Prolific)
>> - <UsbDeviceName> usb-serial-for-android (Ch34x)
>> - <UsbDeviceName> usb-serial-for-android (CdcAcm)
>> - <UsbDeviceName> libftdi (deprecated)
>> 

That's a lot and might make things harder on small screens.
Also, under which circumstances do you think that the deprecated libftdi solution would be preferable? Wouldn't we just want to remove that?
One thing that I implemented a while ago was the intent handling that opened Subsurface-mobile when a serial device was plugged in; again, which situation would require the user to specifically select the chipset?
I'm just trying to understand how this would be used (and how we would document things).
>> With this options, basically all divecomputers with supported usb-to-serial interfaces could be opened. If it's a known VID/PID pair, the (default) autodetect connection could be used, and otherwise a specific driver could be chosen.
>> 

Given that the universe of dive computers that libdivecomputer supports is really rather small, my guess is that we should be able to assemble a fairly complete list of VID/PIDs, but I'll need to read the code to make sure I understand how that manual selection would work, I guess :-)

>> The open call could then be easily adopted with the information, either by parsing the string (ugly, and possibly creates problems when multiple devices of the same type are connected), or by directly passing an (android) UsbDevice-Object and the driver-classname to the serial_usb_android_open function.
>> 

Since we should have the UsbDevice object through the intent, that seems like it should work, no? Or am I missing something (it's been a while that I tried (and failed) to work on that.

>>>> - remove serial_ftdi? -> there are at least the cochran_commander.c and
>>>> reefnet_sensuspro.c computers which won't work with
>>> Just leave it for now. We can always kill it off later when/if we no
>>> longer find it useful. We currently use it on more platforms than just
>>> android, and its issues are android specific so it might still be
>>> usefull.
>> Ah, I didn't know that it's used on different platforms.

Yes, it's available on our desktop platforms as well as it sometimes helps to make it easier to connect to divecomputers

>> Thanks again for all the feedback. If testing with other divecomputers (preferably also with other serial chipsets) goes well, is there anything missing before this can be merged? Should I open a pull request already?
>> 

Thanks for opening the pull request. I'll comment there and we can figure out next steps once a few people had a chance to test.

/D
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.subsurface-divelog.org/pipermail/subsurface/attachments/20200306/cc073dae/attachment.html>