Skip to content

Refactor BiometricCollector to enhance heart rate data handling#2

Open
yelabb wants to merge 2 commits intomainfrom
use-health-snapshot-mesures
Open

Refactor BiometricCollector to enhance heart rate data handling#2
yelabb wants to merge 2 commits intomainfrom
use-health-snapshot-mesures

Conversation

@yelabb
Copy link
Owner

@yelabb yelabb commented Feb 3, 2026

No description provided.

Copilot AI review requested due to automatic review settings February 3, 2026 01:11
@yelabb yelabb self-assigned this Feb 3, 2026
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refactors BiometricCollector to use beat-to-beat interval data (RR intervals) via a sensor data listener to improve HRV/RMSSD accuracy, with a polling fallback and synthetic-data fallback behavior.

Changes:

  • Registers Sensor.registerSensorDataListener() to ingest real heartbeat intervals and compute RMSSD from true RR samples.
  • Adds listener state tracking (listenerRegistered, lastRealDataTick) and a cleanup() method intended to unregister the listener.
  • Simplifies the polling fallback to only capture heart rate (removing HR-derived RR estimation).

Comment on lines 98 to 99
// Called every second by the view timer
function update() {
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment says update() is called every second, but AffectView.onTick() calls biometricCollector.update() on a modulo of the animation phase (~every ~1.25s with current constants). Consider updating this comment (or the cadence) so the time-based logic like SYNTHETIC_TIMEOUT remains interpretable.

Copilot uses AI. Check for mistakes.
Comment on lines +107 to +108
// Fallback to synthetic if no real data for a while (simulator)
if ((tickCount - lastRealDataTick) > SYNTHETIC_TIMEOUT) {
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

update() always falls back to generateSyntheticData() after SYNTHETIC_TIMEOUT based solely on lastRealDataTick. On devices where the heartbeat-interval listener is unsupported (or intervals aren’t provided), lastRealDataTick never advances, so the widget will start injecting synthetic RR/HR on real hardware and corrupt RMSSD/HR. Consider gating synthetic generation strictly to the simulator (or only when no real HR is available) and avoid mixing synthetic intervals into real sessions.

Suggested change
// Fallback to synthetic if no real data for a while (simulator)
if ((tickCount - lastRealDataTick) > SYNTHETIC_TIMEOUT) {
// Fallback to synthetic only when no real HR is available
// and we haven't seen real interval data for a while (simulator, or no sensor)
if ((tickCount - lastRealDataTick) > SYNTHETIC_TIMEOUT && heartRate == null) {

Copilot uses AI. Check for mistakes.
// Get heart rate
// Get heart rate at minimum
if (info has :heartRate && info.heartRate != null && info.heartRate > 0) {
heartRate = info.heartRate;
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pollSensorFallback() updates heartRate but never updates lastRealDataTick (or clears usingSynthetic). This guarantees the synthetic timeout will trigger even while real HR is coming in, and once synthetic starts it will overwrite real HR in generateSyntheticData(). Update lastRealDataTick when real HR is observed (or introduce a separate “lastHeartRateTick”) and ensure the synthetic path can’t override real HR readings.

Suggested change
heartRate = info.heartRate;
heartRate = info.heartRate;
// Treat polled heart rate as real data to prevent synthetic override
usingSynthetic = false;
lastRealDataTick = tickCount;

Copilot uses AI. Check for mistakes.
Comment on lines +193 to +202
// Clean up sensor listener
function cleanup() {
if (listenerRegistered) {
try {
Sensor.unregisterSensorDataListener();
} catch (e) {
// Ignore cleanup errors
}
listenerRegistered = false;
}
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A cleanup() API was added, but there are currently no call sites in the codebase (e.g., AffectView.onHide() / AffectApp.onStop()), so the sensor listener may remain registered after the widget is hidden and keep the HR sensor active unnecessarily. Wire cleanup() into the view/app lifecycle (and consider unregistering during reset() if applicable).

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants