On 2018-04-20 20:25, Linus Torvalds wrote:
> On Thu, Apr 19, 2018 at 4:29 AM, Jef Driesen <jef at libdivecomputer.org>
> wrote:
>> On 2018-04-19 12:10, Dirk Hohndel wrote:
>>>
>>> To the best of my knowledge, no version of the Predator / Petrel /
>>> Nerd
>>> family has a wired serial interface, all they do is BT or BLE (I
>>> actually
>>> checked the website to make sure).
>>
>> You forgot about the serial port emulation feature of classic
>> bluetooth
>> (rfcomm). Basically any devices that supports bluetooth classic
>> (DC_TRANSPORT_BLUETOOTH) can also use the serial port emulation mode
>> (DC_TRANSPORT_SERIAL). This remains supported, and that's why they are
>> listed both.
>
> I think this would be better left to the user of the library.
>
> *ANY* dive computer that does rfcomm is probably able to just say
> "open as serial line to /dev/rfcomm*".
>
> So by that logic, if a dive computer supports DC_TRANSPORT_BLUETOOTH,
> then it always supports DC_TRANSPORT_SERIAL.
Correct, and that's also why they are both listed explicitly.
> And the thing is, it's actually actively incorrect. The /dev/rfcomm
> model only exists on Linux, afaik. So now that DC_TRANSPORT_SERIAL is
> actively wrong on other platforms, like Windows, iOS and OS X.
You are wrong about that. This is certainly not a Linux only feature.
Windows and OS X also support it, and I think the BSD's too. So
basically it's supported by all desktop OS'es. In fact, before the I/O
refactoring, this serial emulation was the only way to communicate with
bluetooth enabled dive computers!
I suspect the reason behind the existence of this feature is backwards
compatibility. Applications that did already support serial
communication, can easily migrate to bluetooth because the underlying
api remained the same. For the mobile platforms this does not apply, and
hence they also don't provide the legacy serial emulation support.
> In fact, while Android is Linux too, it doesn't do the whole "rfcomm
> bind" thing, so it's wrong there too.
>
> Let's say that you have a platform that supports real serial lines,
> but doesn't support legacy bluetooth, now that DC_TRANSPORT_SERIAL is
> actively wrong.
>
> So adding that DC_TRANSPORT_SERIAL doesn't actually buy you anything,
> but it does make it harder for the actual users.
>
> I think it would be better for us to remove the thing, or perhaps at
> least make it more accurate by making the serial bit depend on the
> platform being Linux.
>
> We could also do that automatically in subsurface instead. And then we
> could make it actually depend on finding a /dev/rfcomm* device.
The dc_descriptor_get_transports() function tells you which transports
the dive computer supports, and not underlying platform! That are two
completely independent things. If you have a platform that doesn't
support a certain transport, then it's up to the application to ignore
that bit. That's why there is the new dc_context_get_transports()
function, to tell you which (built-in) transports are available on the
platform. The intersection of both tells you which transport(s) are
usable.
> I guess we could also do a dc_serial_iterator thing to see if we find
> any serial devices at all, but sadly, that isn't really all that
> useful. That does't actually limit the devices searched for, and
> generally you'll have at least the hardcoded /dev/ttyS* devices
> available on Linux whether anything is connected or not. So that will
> always report serial devices as available, even if they aren't useful
> (and particularly not useful for a rfcomm connection).
>
> Maybe dc_descriptor_get_transports() could do something like
>
> unsigned int
> dc_descriptor_get_transports (dc_descriptor_t *descriptor)
> {
> unsigned int transports = DC_TRANSPORT_NONE;
> DC_TRANSPORT_NONE
> if (descriptor) {
> transports = descriptor->transports;
> if ((transports & DC_TRANSPORT_BLUETOOTH) &&
> devfnmatch("rfcomm*"))
> transports |= DC_TRANSPORT_SERIAL;
> }
> return transports;
> }
>
> where that "devfnmatch()" obviously does the whole
> "opendir("/dev")+readdir+fnmatch" loop that dc_serial_iterator_next()
> already does, but limited to just rfcomm.
>
> Then DC_TRANSPORT_SERIAL would only show up if you have actually done
> that rfcomm binding that makes it possible..
What you describe here, can already be done with a filter function. With
such a function you can easily let the dc_serial_iterator_next() filter
out entries that don't match a certain pattern.
The only reason why I didn't commit those patches yet, is because on
Windows both rfcomm and (usb-)serial ports are named COMx. It might be
possible to get the type of the serial port somehow. And on Mac OS X,
I'm also not sure whether there is a standard pattern or not.
Jef