OpenTracksApp / OpenTracks

OpenTracks is a sport tracking application that completely respects your privacy.
https://OpenTracksApp.com
Apache License 2.0
1.09k stars 192 forks source link

gpxtpx:cad should be RPM and not SPM? #1164

Closed brianjmurrell closed 2 years ago

brianjmurrell commented 2 years ago

Describe the bug According to the schema (the URL of which is wrong in GPX exports), it seems that cadence is supposed to be in RPM, not SPM.

In GPX exports, it seems to be in SPM.

Am I misunderstanding something?

dennisguse commented 2 years ago

Don't know - what does SPM stands for?

For cycling: We are just taking the BLE crank rotation and value and compute the cadence from it. Here is the data description: https://github.com/oesmith/gatt-xml/blob/master/org.bluetooth.characteristic.csc_measurement.xml

For running: cadence it provided by the sensor directly. Sadly, I didn't find a better link to the XML file: https://github.com/sputnikdev/bluetooth-gatt-parser/blob/master/src/main/resources/gatt/characteristic/org.bluetooth.characteristic.rsc_measurement.xml

PS/ links in the code to these files are broken as the Bluetooth SIG decided to refactor their URLs :/ You can just google for the file name...

brianjmurrell commented 2 years ago

SPM is presumably steps per minute. For running, walking, etc., RPM is the SPM of one leg, so 1 RPM == 2 SPM.

Given that the above schema describes cad as:

<xsd:element name="cad" type="RevolutionsPerMinute_t" minOccurs="0"/>

https://github.com/sputnikdev/bluetooth-gatt-parser/blob/master/src/main/resources/gatt/characteristic/org.bluetooth.characteristic.rsc_measurement.xml does say:

\Unit is in 1/minute (or RPM) with a resolutions of 1 1/min (or 1 RPM) \

But if you are just recording the reading from my Tickr X exactly as it's read (~120), then clearly my sensor is providing SPM, not RPM as the above seems to indicate that it should.

The Wahoo app provides the same kind of numbers (~120) but reports those as SPM. Maybe a bug in this sensor that might need a "knob" in the app to work around? Or maybe derivable from the device's BLE address?

brianjmurrell commented 2 years ago

So it seems that a BT sensor is supposed to report cadence in RPM, but the Tickr X is reporting in SPM. I know that the app somehow determines the device to be a "Tickr X 4150" as that is what it displays in the sensor preference config.

Can that name detection be used in a conditional statement to half the value being read as cadence so that it's properly RPM?

I mean, surely it must be, but how? I have no idea about reading BT sensors and where that name might be stored and accessed at cadence reading time. Any suggestions?

dennisguse commented 2 years ago

Good to know - we should probably document this somehow.

I see two things:

  1. do we need to report SPM (UI and voice) for running sensors?
  2. Regarding the Tickr X @brianjmurrell Can you check what applications are exporting using your Tickr X? And is the software on the device update to date? And could you complain to the vendor (before we add a workaround)?
brianjmurrell commented 2 years ago

Good to know - we should probably document this somehow.

Indeed. I did add some notes to the supported devices page when I added the device.

I see two things:

  1. do we need to report SPM (UI and voice) for running sensors?

I don't think so. It seems standard to report RPM, so we should just continue doing that I think.

  1. Regarding the Tickr X @brianjmurrell Can you check what applications are exporting using your Tickr X?

Not sure what you mean exactly. The only other application that I know of that reads the cadence value from the Wahoo (or any other even) HRM is the Wahoo Fitness app and it only provides the data in it's own app. It does report SPM.

And is the software on the device update to date?

The firmware on the Tickr X? I can only presume so. Wahoo's support sucks so there isn't going to be much (read, any) help there.

And could you complain to the vendor (before we add a workaround)?

Already have, until I am blue in the face. They are not going to do anything about it. There are many issues with their software none of which they are doing anything about.

dennisguse commented 2 years ago

:man_shrugging: what can I say here... 1st class quality service...

Anyhow. Then let's add the workaround and for this we need to identify the faulty sensor. Can you put a breakpoint here: https://github.com/OpenTracksApp/OpenTracks/blob/12c21b79402526db490e25e1fe893b0130517014/src/main/java/de/dennisguse/opentracks/sensors/BluetoothUtils.java#L146

and check if we can identify it from the BluetoothGattCharacteristic? Something that is unique. So not the your device's name as ends with a random identifier (their was a number at the end right?).

And about running sensors: all that I had in my hands were quite okay. https://github.com/OpenTracksApp/OpenTracks/blob/main/README_TESTED_SENSORS.md#0x1814-running-cadence-and-speed One company even provided a device for integration testing (3 month) for free. And had an amazing customer service - they provided quite a lot of technical details beforehand.

brianjmurrell commented 2 years ago

Can you put a breakpoint here:

I don't have the whole Android Studio framework set up here. I just run gradlew to build. Does BluetoothGattCharacteristic have a reasonable string repr that I can put into a Log.i()?

dennisguse commented 2 years ago

@brianjmurrell Happy testing: #1170. You already gave me the sensor name, so it's not rocket science. Only the solution is not really OOP, but hey.

brianjmurrell commented 2 years ago

Unfortunately while I went out for a walk today, I was unable to get any results due to #1171. Maybe tomorrow will be more fruitful.

brianjmurrell commented 2 years ago

Did some testing. The sensorName is actually TICKR X 4150. But otherwise this is working.

In anticipation that some day, Wahoo fixes this, is the firmware version from the BLE HRM available to be queried?

dennisguse commented 2 years ago

Let's just add the workaround for now.

Getting the firmware version is not that much fun:

And I am not sure that Wahoo will ever change it...

What we could do is a setting to switch between rpm/spm. ... It just clutters the settings.

brianjmurrell commented 2 years ago

Yeah, that's all fair enough. It was more just in inquiry for a potential future event. I agree that we don't need to address if and until it happens.

What do we need to do to land this? Just update the sensor name? It seems to be working as intended once the sensor name is updated.

dennisguse commented 2 years ago

@brianjmurrell I just updated the sensor name to the TICKR X prefix - often the number is kind of a device identifier. At least for the sensors I have - should be working; will be in tomorrows nightly.