Heartrate measurements in background

[patricgruber]

This implements heart rate measurements when the screen is turned off.
The ticket I found for this is: #183

When starting the heart rate measurements through the HeartRate screen and turning off the screen, the heart rate task doesn't stop but keeps running in the background until the heart rate measurement is stopped through the screen again.
The task wait delay is set to ~10 seconds (10k ticks) so the task doesn't run all the time and drains the battery too much.

Already tested it on my PineTime and works great.
Right now it was more a proof of concept, therefore the interval between background measurements is hard-coded to ~5 minutes. But I'd be happy to implement a settings screen to configure the interval or other features that are wanted.

[github-actions]

Build size and comparison to main:

Section	Size	Difference
text	374032B	1088B
data	948B	0B
bss	22552B	16B

[eliedrian]

Was about to implement the same thing!

Another idea that I was thinking of was to measure heart rate after a number of steps taken. Perhaps taking measurements after a certain number of steps in a time window.

[lman0]

Another idea that I was thinking of was to measure heart rate after a number of steps taken.

This idea should be a setting and not a default,
Because there is people with leg disability that could use the pine time with the heart rate.
And this people wouldn't be have the heart rate function.
But for fitness related event, without leg disability it could be .

[patricgruber]

Another idea that I was thinking of was to measure heart rate after a number of steps taken.

This idea should be a setting and not a default, Because there is people with leg disability that could use the pine time with the heart rate. And this people wouldn't be have the heart rate function. But for fitness related event, without leg disability it could be .

It could maybe be an additional trigger for measuring, but not the only one.

[btdogan]

I use hr monitor for my sleep. this shouldn't prevent that.

[patricgruber]

I use hr monitor for my sleep. this shouldn't prevent that.

The code just runs measurements every 5 minutes when the heart rate task is started (through the heart rate screen) and the watch is locked/the screen is off.

All the other functionality is exactly as before. The new changes don't interfere with monitoring the heart rate while staying in the heart rate screen.

[patricgruber]

I noticed during testing that notifications interrupt the background measurement delay and make it reset, so I removed the reset for the GoToSleep message.
Now the behavior is slightly different as the background measurement will every 5 minutes from the last background measurement or as soon as the watch is going to sleep. Instead of measuring every 5 minutes starting from the time it goes to sleep.

Notifications and "just checking the time quickly" won't reset the delay now and the measurements will run more consistently over time.

[LinuxinaBit]

This idea should be a setting and not a default, Because there is people with leg disability that could use the pine time with the heart rate. And this people wouldn't be have the heart rate function. But for fitness related event, without leg disability it could be .

And the heart rate sensor likes to use a lot of battery power, so for those feeling battery conscious it would need to be a setting somewhere.
Maybe in the HR app?

[patricgruber]

Maybe in the HR app?

The background measurements are only running, if the user activates the heart rate measurement in the HR app.
If the user doesn't activate the HR measurements in the HR app or if they are disabled again, no background measurement will be taken.

So there is no new default that HR measurements are taken in the background all the time, just when activating them through the HR app.

The behavior when activating the measurement and leaving the HR app is that when the screen is on the HR is measured all the time. If the screen is off, HR is not measured.
If the HR measurement is not activated in the app no measurement is taken ever.

My code does not change the case when measurements are activated and the screen is on (stays at "measure as often as possible"). Also when the measurement is not activated in the HR app nothing changes (stays at "never measure").
Only for the case when the measurement is activated in the app AND the screen is off, then the new mode "measure every 5 minutes" is activated.

So the the user still always has the option to just don't activate HR measurements at all, but if they are activated, then additionally to measuring when the screen is on there are also occasional measurements when the screen is off.

I'm not sure if it makes sense to have another setting that switches between the current mode and the new "measure additionally in the background" mode.
I think that would be a bit confusing if you have to activate two things in separate places to activate the background measurements. But if that is needed and wanted I'll of course add it.

[LinuxinaBit]

That sounds pretty reasonable.

One other concern is just that it takes so long to take a HR measurement, and the first reading is often wildly inaccurate especially during movement.
Maybe wait for 3 measurements and average the last two, as well as checking for excess movement from the accelerometer before measurement.

Another idea is to pause readings when absolutely zero accelerometer movement is detected for the past few readings, i.e. when the watch has not been on one's wrist for a while (of course the accelerometer would still be checked every 5-ish minutes even when off one's wrist and would also resume periodic readings when the watch is woken up).

These would both help the battery life and improve accuracy immensely.

[Itai-Nelken]

One other concern is just that it takes so long to take a HR measurement, and the first reading is often wildly inaccurate especially during movement.

After #1486 is merged, measurements will be almost instantaneous and much more accurate.

[LinuxinaBit]

Alright, though I still think excessive and no movement detection should be added to preserve some amount of battery life...

[pankk]

I've been testing #1486 for a couple of days now and while the updates are almost instantaneous the first reading usually takes up to 10 seconds for me. I can't say much about the accuracy, but it's similar to the miband 3. I found that for best results, wearing the watch higher up the forearm (about 1/3 of the way) and having it face the inside of the arm works. I was not able to get a measurement restart when I wore it like that (i.e. the reading did not reset to 000, like it does when moving around while having it on the wrist). PSA: It also might be worth checking if the protective film has been removed from the sensor "window".
(I assume mentioning the PPG PR here is enough and that I don't have to add this feedback to that PR as well.)

[khimaros]

i'm also testing the new PPG algorithm and sometimes it takes upward of 15-20 seconds to get a fix on the heart rate. i've checked out @patricgruber PR and built it. i'll take it for a spin.

also, @pankk thank you for the tip with the film, i'd totally forgotten to remove mine! that might explain the 15-20s delay :)

[khimaros]

i'm testing this PR on my device and i'm happy to say it is working quite well. (removing the sensor film reduced the fix time considerably).

to start, i tested heart rate characteristic notifications in bluetoothctl:

$ bluetoothctl
[bluetoothctl]# devices
Device C9:2D:E5:XX:XX:XX InfiniTime
[bluetoothctl]# pair C9:2D:E5:XX:XX:XX
[bluetoothctl]# connect C9:2D:E5:XX:XX:XX
[InfiniTime]# menu gatt
[InfiniTime]# select-attribute 00002a37-0000-1000-8000-00805f9b34fb
[InfiniTime:/service005b/char005c]# notify on

i received notifications almost exactly every five minutes with the screen off:

[CHG] Attribute /org/bluez/hci0/dev_C9_2D_E5_XX_XX_XX/service005b/char005c Value:
  00 53                                            .N              
  00 53                                            .N    

after that, i installed nrfConnect (unfortunately, a proprietary app and there doesn't seem to be an open source equivalent) and logged all of the notifications overnight.

inspecting the logs, the heart rate accuracy and success rate of notifications seemed very reliable and consistent. awesome!

i estimate that total battery drain overnight was less than 10%, but i did not look at the exact percentage before sleep. i will take more accurate battery measurement tonight.

[lman0]

@khimaros could you share the firmware ?
I would like to test it too

[khimaros]

@lman0 -- i'm open to sending this privately if you provide an email address. unfortunately, the build artifact contains private information (usernames and unabridged filesystem paths, possibly more). alternatively, i've written up detailed reproducible build instructions in the comments on this PR: #1761

[mark9064]

So I've been testing this PR along with the related gadgetbridge PR. Super cool to have the HR measurements showing up. A few thoughts on it

• Previous HR values are changed to no longer be cleared upon screen wake with this PR, or upon the PPG algorithm losing fix on the HR
  • I think this is confusing as it is difficult to tell when the HR value on screen is fresh and when it is the last reading.
  • It's also difficult to tell when it goes from a live reading to holding the last value when the PPG algorithm can't find a value
• The running time is unlimited when doing a background measurement
  • In the extreme case, if there is no sensor contact, the PPG will run continuously

Suggestions

• I think the UI needs to convey that a HR measurement is stale, or it should not show it at all
  • Not sure how this applies to all watchfaces
• Limit runtime to 15-20s
  • Can be done by checking the running duration using the tick count inside HandleSensorData

Regarding the measurement interval: I was thinking about what it works best as. There are a few options I see regarding when to trigger a measurement

• Measurement begins every 5 minutes, ignoring whether the device has been woken or not (close to what is currently implemented)
  • In this case it needs to be wary of a possible BackgroundMeasuring -> (screen on) Measuring (total time < limit) -> (screen off) BackgroundWaiting
    • A measurement is then skipped
  • Also possible to have Measuring -> background timer expires -> (screen off) BackgroundWaiting -> BackgroundMeasuring
    • i.e the PPG stops and then immediately gets started after, discarding all state and forcing a refix
• Measurement runs at least every 5 minutes, taking into account measurements from being awake
  • In other words, every time a HR fix is achieved, the measurement timer is set to expire in 5 minutes
  • The other condition is that the expiry timer must be reset when a background measurement is attempted but fails due to the time limit
  • This way solves the second problem above by design and also seems more sensible to me intuitively
    • I think solving the first problem might require another state?

Not sure if I made any sense so interested to hear any thoughts :)

[khimaros]

@mark9064 i think these are great suggestions. i'll just mention that, even though the data is stale, it is sometimes useful to be able to quickly turn the watch on to see what the most recent HRM measurement was. maybe it can be displayed in another color to indicate staleness?

[khimaros]

FYI, slightly off topic here, but possibly useful for testers of this PR. phyphox also works as a tool for graphing and exporting heart rate data from InfiniTime on android: https://codeberg.org/Freeyourgadget/Gadgetbridge/issues/2383#issuecomment-915776

[patricgruber]

Thanks for all the suggestions.

Regarding the timeout reset for background measurements:
Before I implemented the checks so that whenever a measurement started, the timer reset. Which was a problem since just checking the time reset the timer.
I then changed it to the current implementation that only resets the timer when specifically a background measurement is done. This way having the screen up doesn't effect the background measurement at all and so if the watch goes to sleep, the code pretends as if nothing happened and will immediately measure the heart rate if the screen was on for longer than the interval.
A nice change would be to change it so whenever a measurement is done, either in the foreground or background, the interval is reset.

Regarding the reset to 0 when the screen wakes up when ambient light is detected: I personally rather have a slightly older/stale value then no value at all. If I want a fresh value I can just wait, but at least I can just check the screen and see in which region I am, if I had some more or less constant state for at least 10 minutes or so. But if the heart rate is reset to 0, then I don't have that. I have the background measurements that are either sent to companion app or go no where, but I won't ever see them on the watch. I think a good compromise is having a different color for stale values so I know that the values are old, but I also see the last measurement.

Regarding the ambient light sensor and running forever: I think implementing a timer to stop the sensor if there is ambient light for more than 10-20 seconds and then just try again for the next "scheduled" measurement seems reasonable.

[mark9064]

Sounds sensible overall 👍 One thing to note re running time limits: if it is dark but the PPG has no contact it will still run forever if it just checks ambient light. If the PPG has been running for say 30s with no success the chance it's going to succeed is probably pretty low so I think giving up makes more sense? I guess it depends on how power hungry running the PPG is relatively. I'm planning on making a continuous measurement patch for testing so I should have some ideas on that soonish

[patricgruber]

@mark9064
I would just implement it to always stop trying to measure after 30s, no matter if it is because of ambient light or other reasons.

Also as side note: my first test was to just let it run continuously in the background and I think it drained around 50% battery or more over night. Also the watch got warm. I think letting it run for that long without breaks is probably not the best idea. But maybe with the updated PPG code made it possible. I have only tested with the old code.

[patricgruber]

I implemented a timeout of 30s for the background measurement. If there is no data within these 30s, then the measurement is stopped and retried after 10 mins.

[mark9064]

So I tried implementing functionality (mark9064@225be81 and mark9064@97d894e) that allows the user to choose between no background measurements, periodic background measurements and continuous measurement. It seems to work as expected but as you noted battery life with continuous measuring is poor (~24h). I don't know where the majority of the power is consumed but it makes sense that it would be the PPG LED as it's on about 50% of the time and has a high drive current. The data is interesting though, sleep zones are clearly visible, but the UI for switching between modes sucks. Not sure if this is a feature users would want or if we should just keep it simple

[khimaros]

@mark9064 i'm using your testing branch now on my Pinetime device and it is working quite well. thank you for putting this together and sharing it! i especially like the new raise to wake/lower to sleep functionality. it's very reliable and accurate and i haven't seen any unintended wakes yet.

battery life is, expectedly, worse with the continuous mode on. apart from the battery used to power the LED/hardware, i suspect continuous mode is also preventing the main CPU from going to sleep, since it is always busy with the measurement task? this is just a guess, i don't know any of the inside details of InfiniTime/FreeRTOS power management.

i agree with you the UI for choosing the HRM mode could be a bit clearer, but as a power user it was intuitive enough. maybe it's just a matter of choosing more descriptive names for the toggle? alternative descriptions: "Always", "Periodic", "Foreground". another option is to make the "background delay" configurable, but then we'd likely need two controls; one for the delay and one to toggle background mode on/off.

overall, however, i think this is a great feature and would love to see it land in a release version! it's really incredible to be able to raise my wrist, look at the screen, and see instantaneous heart rate information! i suspect i'm not the only person who would think so.

[mark9064]

Finally got time to do some proper power testing using the patchset in my last message
With low brightness, raise wake and lower to sleep (latest PR versions), sleep mode ~8h/day, wearing watch ~97% of the time: 8 days battery from a full charge. So it would probably be feasible to enable unconditionally if we wanted to, though I think having controls is still nicer

[khimaros]

@mark9064 is this using the testing branch at /~https://github.com/mark9064/InfiniTime/tree/testing ? which measurement mode were you using? periodic? continuous?

[GottemHams]

2. Most people would take the watch off for charging though, so while this may be a legitimate use case it probably isn't relevant most of the time. An explicit toggle would work for everyone's cases, as opposed to InfiniTime forcing it a certain way.

[Saarsk]

Absolutely. 😊 I should've written it clearer. I of course wouldn't want it to be the only option. But a toggle so people can choose.

It's true it won't be relevant for most people (now at least), but I think it opens up some very interesting possibilities. Who knows, if snap-clip like this was made available (open 3D-printable file or finished adapter) it might go from being a nerdy niche to a more popular accessory. :)

It would make it able to sustain continuous tracking for days without having to remove and leave the watch.

[minacode]

I do not think we should introduce an option by default which only makes sense when you modify the hardware.

[mark9064]

I'm of the opinion that the PPG processing code should detect no touch and stop measurement in this case / reduce measurement rate. So I think it's out of scope for this PR and that it will be cleanest to ignore charging. Does anyone get anything other than no reading when on the charger currently?

[pyrotelekinetic]

Does anyone get anything other than no reading when on the charger currently?

I just tested for >10min, I didn't get a single read

[tituscmd]

I think I get the occasional read of something absurd like 130 - 150 BPM while it's on the charger, but it's pretty rare.

[patricgruber]

I'm sorry for not participating in the discussion, but for some reason I haven't received any notifications from Github about the comments since November.

@Seneral

Update, a few days ago it broke, in the sense that the heartrate monitor started flashing about once a second (single flash, not strobing), and no heartrate measurement worked. Turning the background measurements (and active measurements) off for a bit and trying manual measurement later did not fix that.
I ended up restarting the watch to return to the old firmware and then heartrate monitoring worked again.
I just now installed the exact same firmware back again and it also works and will keep using it.
Maybe it just needed a restart, nevertheless, it happened and I couldn't fix it without a restart.

I actually had this issue like 3-4 times before (during my nearly two years of running this code), but I have not idea what causes it. As you said a restart resets it properly. But I don't know what's the actual cause of this behavior. My assumption is that there might be a race condition when starting/stopping the sensor, but since it occurs so rarely I can't say for sure.

[patricgruber]

I've been using this for a bit now and would like to mention two things that I think have not been mentioned yet.

1. I find it a bit irritating, that it's not possible to only have the background measurements enabled but keep the active continuous measurements when the screen is on turned off. I'd have thought that when I have the 'normal' heart rate measuring stopped but set the background interval to let's say 5 minutes, that the device would still measure every 5 minutes, regardless of whether the screen is on or
2. If it doesn't already, the background measurements should probably be paused while the device is charging as in that case the device is certainly not on the users arm and will just read incorrect data.

Ad 1: I agree that this could be unexpected. The idea for this PR was to only add the background measurements functionality without removing any existing one. It is possible to have backgrounds measurements independent of foreground measurements. I think it would need a lot more effort to coordinate between the two methods. Since there is only one sensor and potentially two different tasks/threads that want to use it simultaneously.

Ad 2: I think it makes sense to add the pausing when charging, since also for me it shows around 130BPM when charging. Will add that if possible

[tituscmd]

@patricgruber, would you mind rebasing this PR for 1.15 when you have the chance? I see this feature as a very useful addition and would love to include it in my projects, but it’s currently a bit behind main, which makes merging a bit tricky. Thanks so much for your work on this!

[JF002]

@patricgruber Thank you so much for your work on this PR, and thanks to everyone else who took part in the review and testing and who provided feedback!
HR measurement in the background is effectively a feature that is often requested, and I'm sorry the Core developers team couldn't work on it sooner!
But we definitively want to add this feature to the next release of InfiniTime, so you might see some activity on this PR soon. I'm currently having a look at the code and the comments. It might take some time since I have very little free time right now, but I'm making some progress :)
Note that I already really like the state machine diagram, and I think it would be worth it to add it somewhere in the code or in the documentation! 🥇

[patricgruber]

would you mind rebasing this PR for 1.15 when you have the chance?

Rebased to 1.15

[tituscmd]

would you mind rebasing this PR for 1.15 when you have the chance?

Rebased to 1.15

Thank you! 🙂

[JF002]

I've just done a few power measurements with the heart rate period set to 15s:

The power usage is 3.31mA average :

• 467µA when the heart rate sensor is OFF
• 6.10mA when it's acquiring data
  

Some comparison points:

• LCD low brightness : 7.59mA
• Always ON enabled : 3.41mA

The heart rate sensor uses quite a lot of power, but the impact on the battery life will be reduced if the acquisition period is set to a higher value.

Here is an example with the period set to 1 minute:

The watch used in average 1mA during 2 minutes. So the battery life is basically divided by 2 in this case (~0.5mA without heart rate sensor acquisition, ~1mA when it's acquiring once a minute).

[JF002]

Thanks for this nice PR! To be honest, I thought that adding this functionality would be much harder than this! I like the fact that most of the changes are located in HearRateTask.cpp and that the rest of the code didn't need much modifications!

I think it would be nice to add a bit of documentation about the implementation of HeartRateTask in the .cpp file to help future contributors understanding how it works.

I would also like to understand why it was not possible to add an int32 field in the settings. There is no reason that wouldn't work.

Other than these small details, the code looks pretty good to me, thank you again for your work

[JF002]

I think this has already been discussed, but it would make much more sense to me if this enum was split into 2 fields:

• one bool that enables/disables the background measurement
• one uint or std::chrono::duration to specify the measurement interval/period

I can't think of any reason why a 32bits value would not be correctly stored in the settings file.

If that still doesn't work, I'll try to have a closer look at this issue.

[patricgruber]

I'm not sure what the issue was initially, but I will try again. Might work better, since I have more experience with the code base now

[JF002]

You could probably document the state machine here (with the state diagram from this comment

[patricgruber]

I will check and update the state machine and add it here

[marigoldfish]

I have been testing this out for a few days and I'm really digging it! This is going to be really cool functionality to have in main. I do have some notes, which I've tried to divide into their own sections by issue for readability. (Even though I have a lot of notes, I want to emphasize I love the other 99% of this!)

~

I re-flashed the latest CI build tonight for more testing, so the first thing I did was go into the heart rate settings to set my interval to my current preference of 1m. I noticed there was no indication of what the default interval is:

After I turned on heart rate monitoring, it still did not have any setting visibly selected, until I picked one myself.

~

Once I pick an interval, it behaves mostly as expected. I haven't found my preferred interval for maximum Gadgetbridge readability yet, but it's still been interesting to see my heart rate visibly spike when I'm dealing with stress.

I agree with Seneral's Nov 6 comment that another interval between 1 and 5 minutes would be nice to have. (In my perfect world we would have some kind of selector to choose any value between 15s and 30m, but I recognize that may be impractical to implement.)

~

There is a place in the interval settings where I'm not sure if I'm encountering a bug, or if I'm misunderstanding the settings:

If I choose "Off," the behavior appears to be the same as "Cont" - the heart rate sensor runs continuously while the screen is off. My expectation was that selecting "off" would make it behave like stock Infinitime, and the heart rate sensor would read continuously, but only while the screen is on.

~

I also continue to get heart rate readings when the watch is in the blue "sleep"/"moon icon" mode. Not sure if this is the intended behavior. I don't strictly mind having to turn heart rate monitoring off overnight and on again in the morning, but it would be nice if it was all tied to the single "sleep mode" setting instead of having to remember to switch two things manually.

~

Finally, I agree with disabling the heart rate sensor while charging. I had wondered if it was chimes or notifications causing the random spikes that others were reporting, but ultimately I couldn't intentionally trigger one.

For my test, I put my watch in continuous monitoring mode on the charger for about an hour. I messed around with it, sending test notifications to make it rattle and fidgeting with it in the charging cradle. I got 2 spikes in that time, but they only occurred during periods I was not touching it, and it sat idle on the charger.

~

Hope some of this feedback helps! Thanks again for all your hard work on this cool feature!

[AcidWizard]

I would prefer keeping heart rate monitoring on during "Moon"/"Do not disturb" mode as i am specifically interrested in continous heartrate monitoring at night to keep an eye on my resting heartrate. That is what makes the measuring "continuous"

[tituscmd]

I would prefer keeping heart rate monitoring on during "Moon"/"Do not disturb" mode as i am specifically interrested in continous heartrate monitoring at night to keep an eye on my resting heartrate.

I second this

[minacode]

@AcidWizard, interesting point. I think this issue stems from a missing distinction between two sleep states: the device is sleeping und the human is sleeping.

[patricgruber]

First of all, thank you all for your feedback and suggestions!

@marigoldfish

(In my perfect world we would have some kind of selector to choose any value between 15s and 30m, but I recognize that may be impractical to implement.)

I think this is an excellent idea. I planned to implement it like that from the beginning, but just did manage to do it. But I will give it another try!

Since probably a lot more people are going to want to set a custom interval.

My idea would be to completely rewrite the options screen and have a UI like this:

1. On top there is a toggle button to activate/deactivate the background measurements
2. Below there are two number inputs. The first one for the minutes, which goes from 0 to 30 in steps of 1 (maybe 2)
   The other one is the picker for the seconds from 0 to 59 in steps of 5
3. Another toggle button which decides if the background measurements should be paused during night mode. With a text like: "Ignore night mode" or "Pause during night mode" or something like that.

I think this would allow for the most freedom to choose exactly how the heart rate measurements should work.

[patricgruber]

so the first thing I did was go into the heart rate settings to set my interval to my current preference of 1m. I noticed there was no indication of what the default interval is

I think this should be fixed, just to make it more clear. My theory is that the memory was not initialized properly and it just used the value that was there already, which could have been anything but most likely not in the usable range for the enum. I didn't even think about that, thanks for pointing that out!

If I choose "Off," the behavior appears to be the same as "Cont"

This is definitely a bug and I can reproduce this, so will be fixed. Good catch!

Edit: Both of these issues should be fixed now

[patricgruber]

List of open todos (just so they are documented in one place):

• Add state machine as documentation in the code
• Stop measurements when the watch is charging
• Settings should store interval as number type instead of enum
• Settings screen should include on/off toggle, number pickers for interval
• Add toggle for pausing background measurements during night mode

[JF002]

@patricgruber Thanks for keeping all these TODOs organized 🥇

To make the review of this PR easier for us, I suggest we try to limit the scope of this PR to what is necessary for this feature to work an be maintainable. Other improvements and additions can be done in future PRs, of course.

In that regard, I think the 1st ("Add state machine as documentation in the code") and 3rd ("Settings should store interval as number type instead of enum") elements should be part of this PR. The other ones could be implemented in their own respective PR. Those will be smaller and much easier PRs to discuss and review.

[patricgruber]

@JF002 Sounds good! I'm always a fan of keeping PRs short.

I'll implement the two open requirements as soon as possible. Thank you for the feedback!