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

Fri Mar 6 06:19:51 PST 2020

Hi together,

So I just added the missing PID / VID and prepared a pull request at
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.

Best regards
Christof

On 06.03.20 15:17, Christof Arnosti wrote:
>
> Hi Anton,
>
> Thanks for your feedback!
>
> On 06.03.20 08:43, Anton Lundin wrote:
>> On 05 March, 2020 - Christof Arnosti wrote:
>>
>>> Hi together,
>>>
>>> So I continued on my quest to support USB-to-Serial adapters in
>>> subsurface-mobile for android.
>>>
>>> I focused on implementing the usb-serial-for-android driver. It works
>>> with my computer (TM), but there are still some problems in the code and
>>> I can't test with other computers.
>>>
>>> The Code can be found at
>>> https://github.com/charno/subsurface/commits/android-serial-clean.
>> Cool! Nice work.
>>
>>> Problems:
>>> - usb read timeout is not working due to an android Bug
>>> (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.
>>> 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.
>>> 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/android-mobile/res/xml/device_filter.xml
>>
>> Some are documented on 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)
>
> 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.
>
> 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.
>
> This could also probably help in the libftdi-implementation by using
> ftdi_usb_open_dev instead of ftdi_usb_open, after the problem of
> opening a libusb-device from a UsbConnection-Object is solved.
>
>>> - libusb-problem, affecting libusb divecomputers and serial_ftdi
>>> connections. I had a short look into it but did not go further down this
>>> path.
>> Just leave them. It's whole another swear of problems.
>>
>>> - 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.
>>> usb-serial-for-android implementation, since they need serial break support.
>>>
>>> I would be happy if you could have a look at the code and maybe test the
>>> implementation with some more computers. I did use a Mares Puck Pro.
>> I had a quick look at it.
>>
>> There are some obvious tabs vs. spaces issues, but nothing that's any
>> work to clean up.
> Fixed (I hope I got everything now).
>> I would probably done away with AndroidSerial.java and merged all that
>> code into the c++ code in serial_usb_android.cpp . Its more a style
>> thingie, and no blocker, just preferences. One of the downsides with the
>> current split between java and c++ is the duplication of constants and
>> so on, which isn't all that pretty.
> I mainly decided to use a Java-Based abstraction layer to make
> exception handling easier.
>> I don't think serial_usb_android.cpp  belongs in core, but I can't
>> really make up my mind where it would fit better, so that's just fine for
>> now.
> I oriented on the serial_ftdi.c path.
>> Why do we need the System.loadLibrary("subsurface-mobile") ? Shouldn't
>> qt figure that out some how?
> Yes it does. Removed. I introduced this since I had problems calling a
> (removed in the meantime) JNI Native-Method from java, but it was
> another problem.
>> I also would love a way for custom serial implementation to log to
>> libdivecomputers log system, so one can get consistent logging, even
>> when using a custom serial, but that's not a serial_usb_android issue,
>> rather a libdivecomputer issue.
>>
>>
>> //Anton
>
> 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?
>
> Best regards
> Christof
>
>>> On 02.03.20 16:39, Christof Arnosti wrote:
>>>> On 01.03.20 22:09, Anton Lundin wrote:
>>>>> On 01 March, 2020 - Dirk Hohndel wrote:
>>>>>
>>>>>>> On Mar 1, 2020, at 10:32 AM, Christof Arnosti <charno at charno.ch> wrote:
>>>>>>>
>>>>>>> Soo... I own a Mares Puck Pro, and I want to use Subsurface Mobile to
>>>>>>> transfer dives from the computer. 
>>>>>>>
>>>>>>> I found that currently for android a libftdi / libusb-based driver
>>>>>>> (serial_ftdi.{c,h}) is available for FTDI based serial converters.
>>>>>> And by 'available' we mean:
>>>>>>
>>>>>> IT DOESN'T WORK ON BASICALLY ANY ANDROID DEVICE
>>>>>>
>>>>>> This works on some old Android devices. And it can be hacked to work
>>>>>> on rooted Android devices. But for 90+% of our users this doesn't work
>>>>>> at all. As you point out below, we have made attempts to fix this and 
>>>>>> admittedly I have spent way too much time on that already... there's even
>>>>>> a PR on GitHub where I try to make this work but I got stuck...
>>>>>>
>>>>>>> The Mares USB cable uses a cp210x based converter.
>>>>>>>
>>>>>>> In the FAQ at subsurface-divelog.org I found the following paragraph:
>>>>>>>
>>>>>>> "Subsurface-mobile on Android and dive computers with FTDI download cables
>>>>>>>
>>>>>>> Subsurface-mobile on Android (but not on iOS beacuse of hardware
>>>>>>> limitations) is designed to be able to download from some dive computers
>>>>>>> using FTDI based download cables and a USB OTG adapter. Unfortunately,
>>>>>>> there is an issue with the way in which Subsurface-mobile opens USB
>>>>>>> devices on Android. We do this from “native code”, not from a Java
>>>>>>> application. And while many Android phones allow this method of
>>>>>>> accessing OTG devices, an (apparently increasing) number of devices
>>>>>>> don’t. On these Android devices the attempt to download from
>>>>>>> divecomputers via the USB OTG adapter always fails. We understand in
>>>>>>> theory how to work around this problem, but haven’t found the right
>>>>>>> developer who could volunteer their time to help us fix the issue."
>>>>>> I need to update the FAQ. Initially it was indeed "some" where it failed.
>>>>>> Now it is basically "all".
>>>>>>
>>>>>>> Well... Maybe I want to be this person, and for sure I want to be the
>>>>>>> person to add cp210x support. I would be glad if somebody could explain
>>>>>>> what the proposed workaround mentioned in the paragraph would be?
>>>>>> Paging Anton - but I know that he has explained it on this mailing list a
>>>>>> few times in the past, so googling for posts from him with Android in the
>>>>>> title might get you to the right one.
>>>>>>
>>>>>>> While recherching I found the git repo
>>>>>>> https://github.com/mik3y/usb-serial-for-android containing a collection
>>>>>>> of USB serial converter drivers for android. The drivers are licensed as
>>>>>>> LGPL2.1 (GPL compatible) and are written in Java using the Android USB
>>>>>>> Host API.
>>>>>>>
>>>>>>> From the code that I have read I have two possible ways to go forward:
>>>>>>>
>>>>>>> First (my preferred way):
>>>>>>> Directly use the java code from usb-serial-for-android, write a small
>>>>>>> abstraction class in java to provide a convenient interface, and a small
>>>>>>> class in c to call the java abstraction class (similar to what
>>>>>>> serial_ftdi does with libftdi).
>>>>>>>
>>>>>>> Pros:
>>>>>>> - This could act as a generic way to access USB serial-based
>>>>>>> divecomputers, including FTDI ones.
>>>>>>> - Using the official Android USB Host API means that the chance for
>>>>>>> support on different phones is quite high.
>>>>>>>
>>>>>>> Cons:
>>>>>>> - More Platform-Specific, non-C code.
>>>>>>>
>>>>>>> Second:
>>>>>>> Translate the CP210x driver from usb-serial-for-android to C code using
>>>>>>> libusb. 
>>>>>>>
>>>>>>> Pros:
>>>>>>> - C-Code
>>>>>>>
>>>>>>> Cons:
>>>>>>> - More work
>>>>>>> - Chipset-Specific
>>>>>>>
>>>>>>> What do you think about this two possibilities? I would prefer to
>>>>>>> directly use usb-serial-for-android, since for the desktop application
>>>>>>> there is already existing support for the different USB-Serial chipsets,
>>>>>>> so portability of the code for non-android builds is not that important
>>>>>>> in my eyes. With this subsurface would also only need one
>>>>>>> android-specific driver implementation for different chipsets.
>>>>>>>
>>>>>>> I would love to hear your toughts and maybe get some additional pointers
>>>>>>> I could use while developing. I would mainly work on the task in the
>>>>>>> next week, since I have holidays and the event I wanted to go to was
>>>>>>> canceled due to the corona virus.
>>>>>> Since Anton knows more about this than anyone else on this list (or at
>>>>>> least I think so), I really hope that tagging him directly on the To: line will
>>>>>> help to catch his attention. It seems much more useful to have him respond...
>>>>>>
>>>>>> Thanks for looking into this. It would be amazing to see this fixed. I had
>>>>>> basically given up hope...
>>>>> There's bit quite a bit written during the years about this. It can all
>>>>> be found in the archives.
>>>>>
>>>>> usb-serial-for-android is one solution. It's a bit harder to test,
>>>>> because it's can't be tested outside of android but it supports a bunch
>>>>> of different devices and it only uses official api's.
>>>>>
>>>>> I haven't seen any libusb (or similar abstractions) based cp210x
>>>>> userspace drivers. Sure, the code can be written, but I think its way
>>>>> too much work to be worth the effort.
>>>>>
>>>>>
>>>>> I'd suggest you look into writing a custom-serial glue-layer which uses
>>>>> jni to call into usb-serial-for-android. I'd suggest looking at the
>>>>> QtAndroidExtras/QAndroidJniObject layer which makes jni way less
>>>>> horrible.
>>>>>
>>>>>
>>>>> I've bin hoping to get around to doing some of this, but due to personal
>>>>> reasons I haven't had the time and energy. I'm glad to see you take up
>>>>> this work and get it done.
>>>>>
>>>>>
>>>>> //Anton
>>>> Hi Anton, Hi Dirk,
>>>>
>>>> Thank you very much for the pointers!
>>>>
>>>> So from a coding perspective I think the tasks became quite clear:
>>>> - Check the pull-request for android libusb-compatibility at
>>>> https://github.com/Subsurface-divelog/subsurface/pull/2309, and finish
>>>> implementing the functionality to get the USB fd by JNI. This will fix
>>>> the existing serial_ftdi implementation, and libusb-based dive computers.
>>>> - Import usb-serial-for-android (probably in the build.gradl
>>>> configuration to keep things easy), and write a glue-layer using JNI.
>>>> Thanks for the info about QAndroidJniObject!
>>>> - Probably remove the serial_ftdi implementation if the
>>>> usb-serial-for-android one works fine.
>>>>
>>>> Best regards
>>>> Christof
>>>>
>>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.subsurface-divelog.org/pipermail/subsurface/attachments/20200306/3898797e/attachment-0001.html>