Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Heartrate measurements in background #1718

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

patricgruber
Copy link

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
Copy link

github-actions bot commented Mar 31, 2023

Build size and comparison to main:

Section Size Difference
text 374096B 1072B
data 948B 0B
bss 22552B 16B

Run in InfiniEmu

@eliedrian
Copy link
Contributor

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
Copy link

lman0 commented Apr 2, 2023

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
Copy link
Author

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
Copy link

btdogan commented Apr 3, 2023

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

@patricgruber
Copy link
Author

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
Copy link
Author

patricgruber commented Apr 3, 2023

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
Copy link

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
Copy link
Author

patricgruber commented Apr 16, 2023

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
Copy link

LinuxinaBit commented Apr 16, 2023

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
Copy link
Contributor

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
Copy link

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

@pankk
Copy link

pankk commented Apr 20, 2023

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.)

@patricgruber patricgruber force-pushed the heartrate-measurements-in-background branch from b967f9b to faec69e Compare May 11, 2023 21:47
@khimaros
Copy link

khimaros commented May 15, 2023

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
Copy link

khimaros commented May 16, 2023

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
Copy link

lman0 commented May 16, 2023

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

@khimaros
Copy link

@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
Copy link
Member

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
Copy link

@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
Copy link

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
Copy link
Author

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
Copy link
Member

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
Copy link
Author

@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
Copy link
Author

patricgruber commented May 25, 2023

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 added a commit to mark9064/InfiniTime that referenced this pull request May 27, 2023
commit d271670
Author: Patric Gruber <[email protected]>
Date:   Thu May 11 23:49:39 2023 +0200

    remove version change in CMakeLists.txt

commit faec69e
Author: Patric Gruber <[email protected]>
Date:   Thu May 11 23:47:31 2023 +0200

    rebase on main

commit c5d2e42
Author: Patric Gruber <[email protected]>
Date:   Mon Apr 3 21:29:17 2023 +0200

    remove background start timestamp reset on sleep

commit 7180646
Author: Patric Gruber <[email protected]>
Date:   Fri Mar 31 12:38:37 2023 +0200

    remove version change in CMakeLists.txt

commit 9186cd2
Author: Patric Gruber <[email protected]>
Date:   Fri Mar 31 10:25:36 2023 +0200

    increase task delay when waiting in the background to 10s

commit a3a30a2
Author: Patric Gruber <[email protected]>
Date:   Fri Mar 31 10:00:56 2023 +0200

    add heart rate measurments in the background
@mark9064
Copy link
Member

mark9064 commented May 27, 2023

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
Copy link

khimaros commented May 28, 2023

@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
Copy link
Member

mark9064 commented Jul 7, 2023

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
Copy link

khimaros commented Jul 7, 2023

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

@patricgruber patricgruber force-pushed the heartrate-measurements-in-background branch from fc8c1a5 to f62426f Compare January 24, 2025 23:14
@tituscmd
Copy link
Contributor

I'm not sure if this has been fixed already, but when I turn off my watch with HRM in BG enabled, the HR measurements keep running for about 30 seconds before going back to the set interval times. Is this expected behaviour? With ANCS (I'm on iOS) I'm having my screen light up much more than usual and everytime it does, when it then falls asleep i.e. the screen turns off again the HRM sensor runs for those 30secs, draining the battery quite a bit.

@patricgruber
Copy link
Author

I'm not sure if this has been fixed already, but when I turn off my watch with HRM in BG enabled, the HR measurements keep running for about 30 seconds before going back to the set interval times. Is this expected behaviour? With ANCS (I'm on iOS) I'm having my screen light up much more than usual and everytime it does, when it then falls asleep i.e. the screen turns off again the HRM sensor runs for those 30secs, draining the battery quite a bit.

Changed to go to waiting instead of measuring when screen is turned off.

@tituscmd
Copy link
Contributor

Awesome! Thank you so much for the quick work 😊

@minacode
Copy link
Contributor

Now you wait infinitely? 😄

@minacode
Copy link
Contributor

Another idea: you could trigger the measurement based on the current time. For example measuring every five minutes would be triggered at 10:05, 10:10, 10:15, ...
This makes it stable and more independent. Changing the time should be uncommon enough to be negligible.

@patricgruber
Copy link
Author

Another idea: you could trigger the measurement based on the current time. For example measuring every five minutes would be triggered at 10:05, 10:10, 10:15, ... This makes it stable and more independent. Changing the time should be uncommon enough to be negligible.

The code right now basically does exactly that, but doesn't depend on the DateTimeControllerto do it

@patricgruber
Copy link
Author

Now you wait infinitely? 😄

On my watch it doesn't and based on the code I don't think should. Do you have that issue on your watch?

@mark9064 mark9064 self-requested a review February 19, 2025 00:18
@minacode
Copy link
Contributor

No, I think I get it now. Every measurement resets the tick counter. And when the screen is on, the watch is measuring constantly. When the screen is turned off, you switch to the waiting state until the interval has passed and the first background measurement is started which restarts the loop.

@tituscmd
Copy link
Contributor

No, I think I get it now. Every measurement resets the tick counter. And when the screen is on, the watch is measuring constantly. When the screen is turned off, you switch to the waiting state until the interval has passed and the first background measurement is started which restarts the loop.

In that case, if the background measurement is set to 10 minutes and the screen turns on for a split second, not enough time to manually check the heart rate, every 9 minutes then the background measurement will never happen

@minacode
Copy link
Contributor

This was my initial point

@tituscmd
Copy link
Contributor

This was my initial point

I find your point very valid.

In my opinion, the background measurements and the manual screen-turned-on measurements should work independent of one another. On that note, I don't see why the HRM sensor is on full blast if the watch is just turned on. Why isn't it only running when in the HeartRate app is open (or when it's measuring in background)?

@minacode
Copy link
Contributor

I think this is the best solution :)

@tituscmd
Copy link
Contributor

@patricgruber What do you think?

@patricgruber
Copy link
Author

@patricgruber What do you think?

I think you are correct that when you have a long interval and short interruptions in-between that the watch will not do background measurements. I think it is easily solved by only setting the measurementStart timestamp when the screen is turned off.
However this might lead to the unexpected behavior of: "The background interval is 30s, screen is turned on for 60s, watch is sent to sleep, since the last background measurement was done over 60s ago, a new measurement is taken immediately, even tho there just was a measurement"
I think there was a discussion about that somewhere in this PR. But I'm fine with completely uncoupling the background measurements from the foreground measurements or have the interval consider any measurement in foreground or background.

@tituscmd
Copy link
Contributor

I like the idea of the background measurement considering every type of measurement, but I think unlinking the two is the overall best option 👍

@patricgruber
Copy link
Author

Changed it to do that. But still testing it

@minacode
Copy link
Contributor

What do you think about restricting the foreground measurements to the heart rate app and using background measurements everywhere else, even if the screen is on?

I like the idea. It feels a lot more polished for an end user product and should reduce this problem to nearly not existing.

Let's do a vote with 👍 and 👎 under this comment.

@tituscmd
Copy link
Contributor

But if I understand correctly, the only difference between the latest commit and the preferred version in @minacode 's comment is that the HRM sensor is still active when the screen is on, outside of the heartrate app. Is that right?

@mark9064
Copy link
Member

Hmm, this would mean that with HR on you wouldn't see a current value on your watchface, only if you go to the HR app right? I think this wouldn't be ideal, I would quite like 5 min background measurements but I would also like to see the up to date value if I switch the screen on. I agree that there's a bit more complexity this way, but I think we can accept it as IMO the user experience is better. At least from what I have seen from other smartwatches, if there is a HR indicator on the watchface it immediately begins acquiring HR on device wake

@savar
Copy link

savar commented Feb 21, 2025 via email

@tituscmd
Copy link
Contributor

Not to mention that it would mean that you might never get a reading IG your get notifications every 5s because of an active slack conversatio, as the "notification pop up" would interrupt the measiremenevery time.

In the approach we were talking about the background measurement would happen every, let's say, 10 minutes regardless of if the screen is turned on or off. So you could have your screen on for 30 minutes and you would still get 3 measurements, one every 10 minutes.

@minacode
Copy link
Contributor

I get the continuous measurement when the watch face show the heart rate. We want that.

Next iteration: we want the measurement to occur every 5 min (for example) OR if the screen is on.

The current state machine can be refactored into a background loop and a screen on/off state where a measurements happens if one of the states triggers it (loop finished or screen on).
You would get the correct automaton of the current form by constructing the product automaton from both of them, but that is not really necessary. Instead just handle both independently and calculate the above OR.

@9cxndy 9cxndy mentioned this pull request Mar 5, 2025
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement to an existing app/feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Heart rate measurement is stops, when the screen is turned off