Discussion:
question about declared transports
Dirk Hohndel
2018-04-19 10:10:41 UTC
Permalink
Hi Jef,

I have a question about the meaning of the transport flags.
My understanding was that DC_TRANSPORT_SERIAL is an indication that the
dive computer supports some form of wired serial interface (Rs232 or more
typically serial over USB). What I don't understand in this context is
this:

/* Shearwater Predator */
{"Shearwater", "Predator", DC_FAMILY_SHEARWATER_PREDATOR, 2, DC_TRANSPORT_SERIAL | DC_TRANSPORT_BLUETOOTH, dc_filter_shearwater},
/* Shearwater Petrel */
{"Shearwater", "Petrel", DC_FAMILY_SHEARWATER_PETREL, 3, DC_TRANSPORT_SERIAL | DC_TRANSPORT_BLUETOOTH, dc_filter_shearwater},
{"Shearwater", "Petrel 2", DC_FAMILY_SHEARWATER_PETREL, 3, DC_TRANSPORT_SERIAL | DC_TRANSPORT_BLUETOOTH | DC_TRANSPORT_BLE, dc_filter_shearwater},
{"Shearwater", "Nerd", DC_FAMILY_SHEARWATER_PETREL, 4, DC_TRANSPORT_SERIAL | DC_TRANSPORT_BLUETOOTH, dc_filter_shearwater},
{"Shearwater", "Perdix", DC_FAMILY_SHEARWATER_PETREL, 5, DC_TRANSPORT_SERIAL | DC_TRANSPORT_BLUETOOTH | DC_TRANSPORT_BLE, dc_filter_shearwater},
{"Shearwater", "Perdix AI", DC_FAMILY_SHEARWATER_PETREL, 6, DC_TRANSPORT_BLE, dc_filter_shearwater},
{"Shearwater", "Nerd 2", DC_FAMILY_SHEARWATER_PETREL, 7, DC_TRANSPORT_BLE, dc_filter_shearwater},

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

What am I missing?

/D
Jef Driesen
2018-04-19 11:29:34 UTC
Permalink
Post by Dirk Hohndel
I have a question about the meaning of the transport flags.
My understanding was that DC_TRANSPORT_SERIAL is an indication that the
dive computer supports some form of wired serial interface (Rs232 or more
typically serial over USB). What I don't understand in this context is
/* Shearwater Predator */
{"Shearwater", "Predator", DC_FAMILY_SHEARWATER_PREDATOR, 2,
DC_TRANSPORT_SERIAL | DC_TRANSPORT_BLUETOOTH, dc_filter_shearwater},
/* Shearwater Petrel */
{"Shearwater", "Petrel", DC_FAMILY_SHEARWATER_PETREL, 3,
DC_TRANSPORT_SERIAL | DC_TRANSPORT_BLUETOOTH, dc_filter_shearwater},
{"Shearwater", "Petrel 2", DC_FAMILY_SHEARWATER_PETREL, 3,
DC_TRANSPORT_SERIAL | DC_TRANSPORT_BLUETOOTH | DC_TRANSPORT_BLE,
dc_filter_shearwater},
{"Shearwater", "Nerd", DC_FAMILY_SHEARWATER_PETREL, 4,
DC_TRANSPORT_SERIAL | DC_TRANSPORT_BLUETOOTH, dc_filter_shearwater},
{"Shearwater", "Perdix", DC_FAMILY_SHEARWATER_PETREL, 5,
DC_TRANSPORT_SERIAL | DC_TRANSPORT_BLUETOOTH | DC_TRANSPORT_BLE,
dc_filter_shearwater},
{"Shearwater", "Perdix AI", DC_FAMILY_SHEARWATER_PETREL, 6,
DC_TRANSPORT_BLE, dc_filter_shearwater},
{"Shearwater", "Nerd 2", DC_FAMILY_SHEARWATER_PETREL, 7,
DC_TRANSPORT_BLE, dc_filter_shearwater},
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).
What am I missing?
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.

Jef
Linus Torvalds
2018-04-20 18:25:59 UTC
Permalink
Post by Jef Driesen
Post by Dirk Hohndel
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.

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.

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.

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

Hmm?

Linus
Jef Driesen
2018-04-23 14:46:45 UTC
Permalink
Post by Linus Torvalds
Post by Jef Driesen
Post by Dirk Hohndel
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.
Post by Linus Torvalds
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.
Post by Linus Torvalds
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.
Post by Linus Torvalds
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
Dirk Hohndel
2018-04-23 15:02:21 UTC
Permalink
Post by Jef Driesen
Post by Linus Torvalds
Post by Jef Driesen
Post by Dirk Hohndel
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.
Post by Linus Torvalds
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!
But that's where you are wrong, Jef. "basically" doesn't cut it. For example, you can't do rfcomm on Android.
And for Android this whole notion of DC_TRANSPORT_SERIAL is way too generic. We support SOME FORM of serial transport and not others.
You are writing libdivecomputer - as your own comment above indicates, purely from a desktop OS perspective.
Post by Jef Driesen
Post by Linus Torvalds
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.
Exactly. So how do I tell on Android, with the information that libdivecomputer gives me, that I can support a Suunto D4 over serial, but not a Mares dive computer (not FTDI) and not a Shearwater divecomputer, unless we also have BT support.

I think the mistake is to assume that "DC_TRANSPORT_SERIAL" is actually a useful way to sufficiently describe the type of transport.

I know that you'll turn around and say "but Oh, the type of serial emulation is a feature of the cable, not the dive computer" and you are of course correct. But that doesn't change the fact that I don't have enough information. You LOVE abstractions. And black boxes. Love them. They are the only way you think about things. Ideally with a hidden binary format that is not documented. I get that. But please understand that the random level at which you decide to mangle information isn't always useful for the apps that want to use your library. That said, I think this is a step in the right direction and I'll just simply continue to filter for certain dive computers manually. Oh well. At least we lucked out and you got the BLE distinction right and we can filter for iOS without filtering by name. Let's celebrate victories.

/D

Loading...