Hi,
I'm new here. I open a thread here to track this issue if someone is
interested to.
I chated with Dirk and Linus about Mares Smart computer and Bluelink
interface issue.
That's last Linus' mail with the problem analysis.
Bye
Fabio
---------- Forwarded message ---------
From: Linus Torvalds <[email protected]>
Date: dom 23 set 2018 alle ore 03:01
Subject: Re: Mares Smart Dive Computer
To: <[email protected]>, Subsurface Mailing List <
[email protected]>
Cc: Dirk Hohndel <[email protected]>
On Sat, Sep 22, 2018 at 2:48 PM Linus Torvalds
<[email protected]> wrote:
>
> This looks odd, [..]
>
> I need to stare at the code. I suspect this is a more complex set of
> descriptors than what we've seen before, and I'm thinking that our use
> of the QT GATT interfaces is just fundamentally wrong, but happened to
> work with the simpler serives exposed by the devices we already
> support.
Ok, I think I see what's going on.
We've done the service discovery, and then we pick the first
non-standard service and decide that's our preferred service. And we
wait for the discovery of the characteristics of *that* service to
end, but when there are multiple services, the other discovery may
still be going on.
This hasn't shown up on the Suunto or Shearwater dive computers I
have, simply because they only have one single non-standard service.
So we have two issues:
- I think we picked the wrong service to begin with for the Mares case
- the debug log looks horribly messy because some of the discovery is
still going on while we try to start doing IO on that service
I have a patch that I think works around the debug issue, and that I
think will also make it easier to do a better job at picking the right
service (but that second thing is *not* done yet).
Dirk (and anybody else who is interested in debugging BLE), does the
attached patch work for you? It *shouldn't* change any behavior, but
it should result in much better debug output, looking something like
this (this is my Garmin Descent, which I also don't have working, but
that has a fairly complex GATT setup:
Found service "{00001800-0000-1000-8000-00805f9b34fb}" "Generic Access"
c: "{00002a00-0000-1000-8000-00805f9b34fb}"
c: "{00002a01-0000-1000-8000-00805f9b34fb}"
c: "{00002a04-0000-1000-8000-00805f9b34fb}"
c: "{00002aa6-0000-1000-8000-00805f9b34fb}"
Found service "{00001801-0000-1000-8000-00805f9b34fb}" "Generic Attribute"
c: "{00002a05-0000-1000-8000-00805f9b34fb}"
d: "{00002902-0000-1000-8000-00805f9b34fb}"
Found service "{6a4e2401-667b-11e3-949a-0800200c9a66}" "Unknown Service"
c: "{6a4e4c80-667b-11e3-949a-0800200c9a66}"
c: "{6a4ecd28-667b-11e3-949a-0800200c9a66}"
d: "{00002902-0000-1000-8000-00805f9b34fb}"
Found service "{6a4e8022-667b-11e3-949a-0800200c9a66}" "Unknown Service"
c: "{6a4e4c80-667b-11e3-949a-0800200c9a66}"
c: "{6a4ecd28-667b-11e3-949a-0800200c9a66}"
d: "{00002902-0000-1000-8000-00805f9b34fb}"
Found service "{6a4e2500-667b-11e3-949a-0800200c9a66}" "Unknown Service"
c: "{6a4e2502-667b-11e3-949a-0800200c9a66}"
d: "{00002902-0000-1000-8000-00805f9b34fb}"
c: "{6a4e2503-667b-11e3-949a-0800200c9a66}"
d: "{00002902-0000-1000-8000-00805f9b34fb}"
c: "{6a4e2504-667b-11e3-949a-0800200c9a66}"
d: "{00002902-0000-1000-8000-00805f9b34fb}"
c: "{6a4e2505-667b-11e3-949a-0800200c9a66}"
d: "{00002902-0000-1000-8000-00805f9b34fb}"
c: "{6a4e2501-667b-11e3-949a-0800200c9a66}"
d: "{00002902-0000-1000-8000-00805f9b34fb}"
.. ignoring standard service "{00001800-0000-1000-8000-00805f9b34fb}"
.. ignoring standard service "{00001801-0000-1000-8000-00805f9b34fb}"
Using service "{6a4e2401-667b-11e3-949a-0800200c9a66}" as preferred service
which shows *all* the services, their characteristics, and the
descriptors. It also shows exactly which service we ended up using as
the preferred one.
You can see above how there are three non-standard services on the
Garmin (the "Unknown Service") ones, and how we just randomly pick the
first one as the preferred one. That's likely wrong for the Garmin,
but we have other issues too (ie unlike the Mares I haven't even
figured out what the protocol is).
I think the Mares has two non-standard services:
Found service "{544e326b-5b72-c6b0-1c46-41c1bc448118}"
Found service "{a86abc2d-d44c-442e-99f7-80059a873e36}"
but it would be interesting to see the whole service -> characteristic
-> descriptor tree.
I think I could figure it out by looking at your trace, but damn,
reading those GATT traces to figure out the descriptors is painful.
Anyway, still working on this all.
Linus
core/qt-ble.cpp | 112 +++++++++++++++++++++++++++++++++++++++++---------------
core/qt-ble.h | 6 +--
2 files changed, 85 insertions(+), 33 deletions(-)
diff --git a/core/qt-ble.cpp b/core/qt-ble.cpp
index b4678aa5c..96dc12ad9 100644
--- a/core/qt-ble.cpp
+++ b/core/qt-ble.cpp
@@ -105,20 +105,6 @@ void BLEObject::writeCompleted(const QLowEnergyDescriptor&, const QByteArray&)
void BLEObject::addService(const QBluetoothUuid &newService)
{
qDebug() << "Found service" << newService;
- bool isStandardUuid = false;
- newService.toUInt16(&isStandardUuid);
- if (IS_HW(device)) {
- /* The HW BT/BLE piece or hardware uses, what we
- * call here, "a Standard UUID. It is standard because the Telit/Stollmann
- * manufacturer applied for an own UUID for its product, and this was granted
- * by the Bluetooth SIG.
- */
- if (newService != QUuid("{0000fefb-0000-1000-8000-00805f9b34fb}"))
- return; // skip all services except the right one
- } else if (isStandardUuid) {
- qDebug () << " .. ignoring standard service";
- return;
- }
auto service = controller->createServiceObject(newService, this);
qDebug() << " .. created service object" << service;
@@ -206,6 +192,84 @@ dc_status_t BLEObject::read(void *data, size_t size, size_t *actual)
return DC_STATUS_SUCCESS;
}
+//
+// select_preferred_service() gets called after all services
+// have been discovered, and the discovery process has been
+// started (by addService(), which calls service->discoverDetails())
+//
+// The role of this function is to wait for all service
+// discovery to finish, and pick the preferred service.
+//
+// NOTE! Picking the preferred service is divecomputer-specific.
+// Right now we special-case the HW known service number, but for
+// others we just pick the first one that isn't a standard service.
+//
+// That's wrong, but works for the simple case.
+//
+dc_status_t BLEObject::select_preferred_service(void)
+{
+ QLowEnergyService *s;
+
+ // Wait for each service
+ foreach (s, services) {
+ int msec = BLE_TIMEOUT;
+ while (msec > 0 && s->state() == QLowEnergyService::DiscoveringServices) {
+ waitFor(100);
+ msec -= 100;
+ }
+ }
+
+ // Print out the services for debugging
+ foreach (s, services) {
+ qDebug() << "Found service" << s->serviceUuid() << s->serviceName();
+
+ QLowEnergyCharacteristic c;
+ foreach (c, s->characteristics()) {
+ qDebug() << " c:" << c.uuid();
+
+ QLowEnergyDescriptor d;
+ foreach (d, c.descriptors())
+ qDebug() << " d:" << d.uuid();
+ }
+ }
+
+ // Pick the preferred one
+ foreach (s, services) {
+ if (s->state() != QLowEnergyService::ServiceDiscovered)
+ continue;
+
+ bool isStandardUuid = false;
+ QBluetoothUuid uuid = s->serviceUuid();
+
+ uuid.toUInt16(&isStandardUuid);
+
+ if (IS_HW(device)) {
+ /* The HW BT/BLE piece or hardware uses, what we
+ * call here, "a Standard UUID. It is standard because the Telit/Stollmann
+ * manufacturer applied for an own UUID for its product, and this was granted
+ * by the Bluetooth SIG.
+ */
+ if (uuid != QUuid("{0000fefb-0000-1000-8000-00805f9b34fb}"))
+ continue; // skip all services except the right one
+ } else if (isStandardUuid) {
+ qDebug () << " .. ignoring standard service" << uuid;
+ continue;
+ }
+
+ preferred = s;
+ qDebug() << "Using service" << s->serviceUuid() << "as preferred service";
+ break;
+ }
+
+ if (!preferred) {
+ qDebug() << "failed to find suitable service";
+ report_error("Failed to find suitable BLE GATT service");
+ return DC_STATUS_IO;
+ }
+
+ return DC_STATUS_SUCCESS;
+}
+
dc_status_t BLEObject::setHwCredit(unsigned int c)
{
/* The Terminal I/O client transmits initial UART credits to the server (see 6.5).
@@ -351,34 +415,22 @@ dc_status_t qt_ble_open(void **io, dc_context_t *, const char *devaddr, dc_user_
controller->discoverServices();
msec = BLE_TIMEOUT;
- while (msec > 0 && controller->state() == QLowEnergyController::DiscoveringState) {
+ while (msec > 0 && controller->state() != QLowEnergyController::DiscoveredState) {
waitFor(100);
msec -= 100;
}
qDebug() << " .. done discovering services";
- if (ble->preferredService() == nullptr) {
- qDebug() << "failed to find suitable service on" << devaddr;
- report_error("Failed to find suitable service on '%s'", devaddr);
- delete ble;
- return DC_STATUS_IO;
- }
- qDebug() << " .. discovering details";
- msec = BLE_TIMEOUT;
- while (msec > 0 && ble->preferredService()->state() == QLowEnergyService::DiscoveringServices) {
- waitFor(100);
- msec -= 100;
- }
+ dc_status_t error = ble->select_preferred_service();
- if (ble->preferredService()->state() != QLowEnergyService::ServiceDiscovered) {
+ if (error != DC_STATUS_SUCCESS) {
qDebug() << "failed to find suitable service on" << devaddr;
report_error("Failed to find suitable service on '%s'", devaddr);
delete ble;
- return DC_STATUS_IO;
+ return error;
}
-
qDebug() << " .. enabling notifications";
/* Enable notifications */
diff --git a/core/qt-ble.h b/core/qt-ble.h
index 5b3991bef..ddf628e60 100644
--- a/core/qt-ble.h
+++ b/core/qt-ble.h
@@ -23,9 +23,8 @@ public:
dc_status_t write(const void* data, size_t size, size_t *actual);
dc_status_t read(void* data, size_t size, size_t *actual);
- //TODO: need better mode of selecting the desired service than below
- inline QLowEnergyService *preferredService()
- { return services.isEmpty() ? nullptr : services[0]; }
+ inline QLowEnergyService *preferredService() { return preferred; }
+ dc_status_t select_preferred_service(void);
public slots:
void addService(const QBluetoothUuid &newService);
@@ -39,6 +38,7 @@ private:
QVector<QLowEnergyService *> services;
QLowEnergyController *controller = nullptr;
+ QLowEnergyService *preferred = nullptr;
QList<QByteArray> receivedPackets;
bool isCharacteristicWritten;
dc_user_device_t *device;
_______________________________________________
subsurface mailing list
[email protected]
http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
