InfiniTimeOrg / InfiniTime

Firmware for Pinetime smartwatch written in C++ and based on FreeRTOS
GNU General Public License v3.0
2.76k stars 944 forks source link

Heartrate Value jumps to 000 #1881

Closed 0x0000ff closed 7 months ago

0x0000ff commented 1 year ago

Verification

What happened?

The heartrate monitor will sporadically jump to 000, and will sometimes stay at that value. for a long time

What should happen instead?

No response

Reproduction steps

Wearing the watch, I select the heart rate app and leave it running.

More details?

No response

Version

v1.13.0

Companion app

Not currently paired with phone.

minacode commented 1 year ago

Isn't this is the expected behavior when the measurement fails?

Edit: if it is, the watch should communicate that state better.

mark9064 commented 1 year ago

Yeah, when the algorithm can't find a clean signal it reports 0

Would --- make more sense than 000? I think that'd be better - same goes for watchfaces (they could be just -) or even an ascii spinner if we're feeling a bit fun

hellodarkness commented 1 year ago

Yeah, when the algorithm can't find a clean signal it reports 0

Would --- make more sense than 000? I think that'd be better - same goes for watchfaces (they could be just -) or even an ascii spinner if we're feeling a bit fun

I agree with this. --- would match what's standard for a null reading on most devices.

I think feeding back a null value for watchface designers to choose how to handle might be good? Some of the fancier custom watch faces wouldn't suit an ascii spininer in my opinon.

minacode commented 1 year ago

Are these false zero values sent via Bluetooth? Might be correct to send nothing instead, if they are.

mark9064 commented 1 year ago

They are sent by bluetooth, but it's the way to go there. There's no way in the bluetooth specification to signal that there is no value available, so zero is recognised as a null there. The only flag is to say whether sensor touch is present, and that's something quite different from an algorithm not being able to lock onto the HR.

minacode commented 1 year ago

Shouldn't we have another flag for "no good measurement" then? It's nice that the algorithm has the detection, but I think that we can use it better.

mark9064 commented 1 year ago

For bluetooth - no, as the characteristic is standardised. Every BLE device exposing heart rate values does so in the exact same format (in the BT SIG GATT spec)

On the watch - well there are only 3 states: off, on + no reading, on + reading. Arguably, there's also on + waiting for data (when it's just been started), but this is lumped into on + no reading currently. Currently, the on + no reading state reports a 0 on watchfaces / the HR app, and the idea here is to display --- / - instead of 0

I wasn't sure of the purpose of the flag you suggested here - these are the two ways I can interpret it. Or did you mean something else?

minacode commented 1 year ago

Yes, that was helpful. Let me rephrase my question: Do we send 0 via Bluetooth when the state is on + no reading?

mark9064 commented 1 year ago

Let's think about only reading the heart rate characteristic for a second (rather than notifications). Bluetooth characteristics (supporting read) always have a value present. So even if the sensor is completely off, the heart rate characteristic must have a value for the bpm (an integer). The only value that makes sense here is 0.

Since we always keep the state of client subscribed with notifications consistent with the value returned by reading a characteristic, it is correct to send a 0 whenever a value isn't available.

minacode commented 1 year ago

Ok, thank you!