Discussion:
this looks suspiciously easy
Dirk Hohndel
2018-04-19 04:25:45 UTC
Permalink
Hey Linus,

I am surprised how simple this seems to be. What am I missing?

/D


From 4da077bcb6ef06655abd5216dce742b2b185efeb Mon Sep 17 00:00:00 2001
From: Dirk Hohndel <***@hohndel.org>
Date: Wed, 18 Apr 2018 21:19:14 -0700
Subject: [PATCH] Only offer dive computers with supported transports

On Android we still need to do more filtering as only some of the USB
divecomputers are supported. But on iOS this takes care of it without
the hard coded list.

Additionally, if built without BT or BLE support, the corresponding dive
computers are no longer shown (e.g. Perdix AI on Windows).

Signed-off-by: Dirk Hohndel <***@hohndel.org>
---
core/downloadfromdcthread.cpp | 35 +++++++++++++++++++++--------------
1 file changed, 21 insertions(+), 14 deletions(-)

diff --git a/core/downloadfromdcthread.cpp b/core/downloadfromdcthread.cpp
index b3bf8ba28334..e67d71403ad2 100644
--- a/core/downloadfromdcthread.cpp
+++ b/core/downloadfromdcthread.cpp
@@ -101,20 +101,6 @@ static void fill_supported_mobile_list()
mobileProductList["Atomic Aquatics"] =
QStringList({{"Cobalt"}, {"Cobalt 2"}});

-#endif
-#if defined(Q_OS_IOS)
- /* BLE only, Qt does not support classic BT on iOS */
- mobileProductList["Heinrichs Weikamp"] =
- QStringList({{"OSTC 2"}, {"OSTC 3"}, {"OSTC 3+"}, {"OSTC 4"}, {"OSTC Plus"}, {"OSTC Sport"}, {"OSTC 2 TR"}});
- mobileProductList["Mares"] =
- QStringList({{"Puck Pro"}, {"Smart"}, {"Quad"}});
- mobileProductList["Scubapro"] =
- QStringList({{"Aladin Sport Matrix"}, {"Aladin Square"}, {"G2"}});
- mobileProductList["Shearwater"] =
- QStringList({{"Perdix"}, {"Perdix AI"}, {"Petrel"}, {"Petrel 2"}});
- mobileProductList["Suunto"] =
- QStringList({{"EON Core"}, {"EON Steel"}});
-
#endif
}

@@ -123,10 +109,31 @@ void fill_computer_list()
dc_iterator_t *iterator = NULL;
dc_descriptor_t *descriptor = NULL;

+ int transportMask = 0;
+#if defined(BT_SUPPORT)
+ transportMask |= DC_TRANSPORT_BLUETOOTH;
+#endif
+#if defined(BLE_SUPPORT)
+ transportMask |= DC_TRANSPORT_BLE;
+#endif
+#if !defined(Q_OS_ANDROID) && !defined(Q_OS_IOS) && !defined(Q_OS_MAC)
+ transportMask |= DC_TRANSPORT_IRDA;
+#endif
+#if !defined(Q_OS_IOS)
+ transportMask |= DC_TRANSPORT_USB | DC_TRANSPORT_USBHID;
+#endif
+#if !defined(Q_OS_ANDROID) && !defined(Q_OS_IOS)
+ transportMask |= DC_TRANSPORT_SERIAL;
+#endif
+
fill_supported_mobile_list();

dc_descriptor_iterator(&iterator);
while (dc_iterator_next(iterator, &descriptor) == DC_STATUS_SUCCESS) {
+ if ((dc_descriptor_get_transports(descriptor) & transportMask) == 0)
+ // none of the transports are available, skip
+ continue;
+
const char *vendor = dc_descriptor_get_vendor(descriptor);
const char *product = dc_descriptor_get_product(descriptor);
#if defined(Q_OS_ANDROID) || defined(Q_OS_IOS)
--
2.17.0
Linus Torvalds
2018-04-19 04:31:02 UTC
Permalink
Looks good to me.

Linus
Post by Dirk Hohndel
Hey Linus,
I am surprised how simple this seems to be. What am I missing?
/D
From 4da077bcb6ef06655abd5216dce742b2b185efeb Mon Sep 17 00:00:00 2001
Date: Wed, 18 Apr 2018 21:19:14 -0700
Subject: [PATCH] Only offer dive computers with supported transports
On Android we still need to do more filtering as only some of the USB
divecomputers are supported. But on iOS this takes care of it without
the hard coded list.
Additionally, if built without BT or BLE support, the corresponding dive
computers are no longer shown (e.g. Perdix AI on Windows).
---
core/downloadfromdcthread.cpp | 35 +++++++++++++++++++++--------------
1 file changed, 21 insertions(+), 14 deletions(-)
diff --git a/core/downloadfromdcthread.cpp b/core/downloadfromdcthread.cpp
index b3bf8ba28334..e67d71403ad2 100644
--- a/core/downloadfromdcthread.cpp
+++ b/core/downloadfromdcthread.cpp
@@ -101,20 +101,6 @@ static void fill_supported_mobile_list()
mobileProductList["Atomic Aquatics"] =
QStringList({{"Cobalt"}, {"Cobalt 2"}});
-#endif
-#if defined(Q_OS_IOS)
- /* BLE only, Qt does not support classic BT on iOS */
- mobileProductList["Heinrichs Weikamp"] =
- QStringList({{"OSTC 2"}, {"OSTC 3"}, {"OSTC 3+"}, {"OSTC
4"}, {"OSTC Plus"}, {"OSTC Sport"}, {"OSTC 2 TR"}});
- mobileProductList["Mares"] =
- QStringList({{"Puck Pro"}, {"Smart"}, {"Quad"}});
- mobileProductList["Scubapro"] =
- QStringList({{"Aladin Sport Matrix"}, {"Aladin Square"},
{"G2"}});
- mobileProductList["Shearwater"] =
- QStringList({{"Perdix"}, {"Perdix AI"}, {"Petrel"},
{"Petrel 2"}});
- mobileProductList["Suunto"] =
- QStringList({{"EON Core"}, {"EON Steel"}});
-
#endif
}
@@ -123,10 +109,31 @@ void fill_computer_list()
dc_iterator_t *iterator = NULL;
dc_descriptor_t *descriptor = NULL;
+ int transportMask = 0;
+#if defined(BT_SUPPORT)
+ transportMask |= DC_TRANSPORT_BLUETOOTH;
+#endif
+#if defined(BLE_SUPPORT)
+ transportMask |= DC_TRANSPORT_BLE;
+#endif
+#if !defined(Q_OS_ANDROID) && !defined(Q_OS_IOS) && !defined(Q_OS_MAC)
+ transportMask |= DC_TRANSPORT_IRDA;
+#endif
+#if !defined(Q_OS_IOS)
+ transportMask |= DC_TRANSPORT_USB | DC_TRANSPORT_USBHID;
+#endif
+#if !defined(Q_OS_ANDROID) && !defined(Q_OS_IOS)
+ transportMask |= DC_TRANSPORT_SERIAL;
+#endif
+
fill_supported_mobile_list();
dc_descriptor_iterator(&iterator);
while (dc_iterator_next(iterator, &descriptor) ==
DC_STATUS_SUCCESS) {
+ if ((dc_descriptor_get_transports(descriptor) &
transportMask) == 0)
+ // none of the transports are available, skip
+ continue;
+
const char *vendor = dc_descriptor_get_vendor(descriptor);
const char *product =
dc_descriptor_get_product(descriptor);
#if defined(Q_OS_ANDROID) || defined(Q_OS_IOS)
--
2.17.0
Dirk Hohndel
2018-04-19 05:46:42 UTC
Permalink
OK, this has been added to the NG branch, current binaries are at the link
below. They should only show supported devices on your system. Tested
here with a few combinations and it seems to do the right thing. I no longer
am offered IRDA computers on my Mac, for example.

As with everything in the NG branch, testing and reports of success or
failure would be very welcome. The more reports for different dive computers
we can get, the better. So if you have a Cobalt or older style serial dive computer
or anything else that hasn't been reported, yet, please try the binaries at
https://github.com/Subsurface-divelog/subsurface/releases/tag/continuous-NG <https://github.com/Subsurface-divelog/subsurface/releases/tag/continuous-NG>

It seems that right now BT rfcomm downloads are broken. Nonetheless, I'd
love to get more testing of this to understand if it's broken for everyone, on
all OSs.

Also, configuring dive computers is likely broken. Hopefully Anton can take
a look.

Thanks for everyone's help!

/D
Post by Linus Torvalds
Looks good to me.
Linus
Federico Masias
2018-04-19 06:27:14 UTC
Permalink
Tested an Aqualung i200 on macOS, downloads fine. When I get home in the
morning I can test on a Windows VM as well, to confirm.

As a total aside, when creating a new log, where does the Map initialize?
It seems to have initialized in downtown London, which is the last place I
looked up in google maps, and not at all close to my home or current
location. A bit weird, but also possibly not in Subsurface's control?

- Fede
Post by Dirk Hohndel
OK, this has been added to the NG branch, current binaries are at the link
below. They should only show supported devices on your system. Tested
here with a few combinations and it seems to do the right thing. I no longer
am offered IRDA computers on my Mac, for example.
As with everything in the NG branch, testing and reports of success or
failure would be very welcome. The more reports for different dive computers
we can get, the better. So if you have a Cobalt or older style serial dive computer
or anything else that hasn't been reported, yet, please try the binaries at
https://github.com/Subsurface-divelog/subsurface/releases/
tag/continuous-NG
It seems that right now BT rfcomm downloads are broken. Nonetheless, I'd
love to get more testing of this to understand if it's broken for everyone, on
all OSs.
Also, configuring dive computers is likely broken. Hopefully Anton can take
a look.
Thanks for everyone's help!
/D
Looks good to me.
Linus
_______________________________________________
subsurface mailing list
http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
Dirk Hohndel
2018-04-19 06:35:03 UTC
Permalink
Post by Federico Masias
Tested an Aqualung i200 on macOS, downloads fine. When I get home in the
morning I can test on a Windows VM as well, to confirm.
As a total aside, when creating a new log, where does the Map initialize?
It seems to have initialized in downtown London, which is the last place I
looked up in google maps, and not at all close to my home or current
location. A bit weird, but also possibly not in Subsurface's control?
It actually does always start in London. I keep thinking that with Linus
and I living in Portland we should have it start there. Or maybe start at
a dive paradise like Palau :-)

/D
Jef Driesen
2018-04-19 06:13:23 UTC
Permalink
Post by Dirk Hohndel
@@ -123,10 +109,31 @@ void fill_computer_list()
dc_iterator_t *iterator = NULL;
dc_descriptor_t *descriptor = NULL;
+ int transportMask = 0;
+#if defined(BT_SUPPORT)
+ transportMask |= DC_TRANSPORT_BLUETOOTH;
+#endif
+#if defined(BLE_SUPPORT)
+ transportMask |= DC_TRANSPORT_BLE;
+#endif
+#if !defined(Q_OS_ANDROID) && !defined(Q_OS_IOS) && !defined(Q_OS_MAC)
+ transportMask |= DC_TRANSPORT_IRDA;
+#endif
+#if !defined(Q_OS_IOS)
+ transportMask |= DC_TRANSPORT_USB | DC_TRANSPORT_USBHID;
+#endif
+#if !defined(Q_OS_ANDROID) && !defined(Q_OS_IOS)
+ transportMask |= DC_TRANSPORT_SERIAL;
+#endif
+
You can also get the mask with the built-in transports from
libdivecomputer with the dc_context_get_transports() function. With the
above, you'll for example show usb or usbhid devices even when
libdivecomputer was build without usb or usbhid support.

For transports where subsurface uses a custom I/O implementation
(bluetooth and ble), you of course need to set the bits as above.

Jef
Dirk Hohndel
2018-04-19 06:37:53 UTC
Permalink
You can also get the mask with the built-in transports from libdivecomputer
with the dc_context_get_transports() function. With the above, you'll for
example show usb or usbhid devices even when libdivecomputer was build
without usb or usbhid support.
For transports where subsurface uses a custom I/O implementation (bluetooth
and ble), you of course need to set the bits as above.
Cool - that's even better.
Of course I first wondered... darn, where do I get the context from in
this part of my code. Then I looked at the sources of libdivecomputer...

unsigned int
dc_context_get_transports (dc_context_t *context)
{
UNUSED(context);

return DC_TRANSPORT_SERIAL
#if defined(HAVE_LIBUSB)
| DC_TRANSPORT_USB
#endif
...

OK, I guess I'll just pass in a NULL pointer :-)

/D
Dirk Hohndel
2018-04-19 06:40:55 UTC
Permalink
Post by Dirk Hohndel
You can also get the mask with the built-in transports from libdivecomputer
with the dc_context_get_transports() function. With the above, you'll for
example show usb or usbhid devices even when libdivecomputer was build
without usb or usbhid support.
For transports where subsurface uses a custom I/O implementation (bluetooth
and ble), you of course need to set the bits as above.
Cool - that's even better.
Of course I first wondered... darn, where do I get the context from in
this part of my code. Then I looked at the sources of libdivecomputer...
unsigned int
dc_context_get_transports (dc_context_t *context)
{
UNUSED(context);
return DC_TRANSPORT_SERIAL
#if defined(HAVE_LIBUSB)
| DC_TRANSPORT_USB
#endif
...
OK, I guess I'll just pass in a NULL pointer :-)
... aaaaand of course... since it ALWAYS adds in the DC_TRANSPORT_SERIAL
flag, I guess I have to mask that back out on iOS where we don't actually
have support for serial dive computers...

/D
Jef Driesen
2018-04-19 07:18:34 UTC
Permalink
Post by Dirk Hohndel
Post by Dirk Hohndel
You can also get the mask with the built-in transports from libdivecomputer
with the dc_context_get_transports() function. With the above, you'll for
example show usb or usbhid devices even when libdivecomputer was build
without usb or usbhid support.
For transports where subsurface uses a custom I/O implementation (bluetooth
and ble), you of course need to set the bits as above.
Cool - that's even better.
Of course I first wondered... darn, where do I get the context from in
this part of my code. Then I looked at the sources of
libdivecomputer...
unsigned int
dc_context_get_transports (dc_context_t *context)
{
UNUSED(context);
return DC_TRANSPORT_SERIAL
#if defined(HAVE_LIBUSB)
| DC_TRANSPORT_USB
#endif
...
OK, I guess I'll just pass in a NULL pointer :-)
That should be fine. I only added the context pointer in case we ever
need it in the future. I doubt that will ever be necessary, but it was
easier to add now then having to change it later.
Post by Dirk Hohndel
... aaaaand of course... since it ALWAYS adds in the
DC_TRANSPORT_SERIAL
flag, I guess I have to mask that back out on iOS where we don't actually
have support for serial dive computers...
Yeah, that's indeed an annoying special case. The code builds just fine
and probably works too, but without any usb-serial drivers (or even the
ability to connect those devices) you can't really do much with it.

Jef

Loading...