-
-
Notifications
You must be signed in to change notification settings - Fork 1
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
Ignore Invalid HR on HR Zone #68
Conversation
raditzlawliet
commented
Dec 4, 2023
•
edited
Loading
edited
- Compare record with valid HR and seek next valid record till end of records
- Adjust percentage to ignore HR value outside of zone (e.g., Zone 1 will have 95-105, all HR <95 will ignored in time & percentage dispaly)
- Adjust on collapse HR to same with other module
- Optimize finding HR Zone with local cache
for (let i = 0; i < session.records.length - 1; i++) { | ||
const entry = session.records[i] | ||
const nextEntry = session.records[i + 1] | ||
|
||
const hrZoneIndex = this.getHeartRateZoneIndex(entry.heartRate || 0) | ||
const nextHrZoneIndex = this.getHeartRateZoneIndex(nextEntry.heartRate || 0) | ||
if (entry.heartRate == null || nextEntry.heartRate == null) continue |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we have these records pattern, do we still get zone? It seems like will be skipped entirely, no? What if for nextEntry instead of [i+1], we try find up to next record with valid heartRate? Then we move our i
cursor to that record index.
records[0].heartRate = 120
records[1].heartRate = null
records[2].heartRate = 130
records[3].heartRate = null
records[4].heartRate = 135
records[5].heartRate = null
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will skipped entirely.
is it possible to have that case commonly?
if such case exists as rare, we can adjust how will we recognize this data case.
What if for nextEntry instead of [i+1], we try find up to next record with valid heartRate?
If this process is valid, we can adjust to it.
Assume there a case
records[0].heartRate = 120
records[1].heartRate = null
records[2].heartRate = null
records[3].heartRate = null
records[4].heartRate = null
records[5].heartRate = 135
what should we do then? compare 120 and 135?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it possible to have that case commonly?
We can't be sure since the data is from sensor, sometimes the sensor is not working properly due to low battery or position, network fail between sensor to cyclocomp, etc.
what should we do then? compare 120 and 135?
Yes, compare let's compare 120 to 135, and calculate the elapse between them as usual.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated, latest commit now will do compare 120 and 135.
then next record will do 135 and next valid HR (if no valid HR, it will compare with themself for diff time 1s)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since i don't have invalid data, i'll do test manually via code. kindly help me check with your real data
hr zone, now seeking next HR Record for comparator (till end of record). if no record valid, will use current record as comparator and end the calculation. hr zone, fix percentage time on not-included hr zone (e.g., HeartRate under minimal visibility zone)
for (let i = 0; i < session.records.length - 1; i++) { | ||
const entry = session.records[i] | ||
const nextEntry = session.records[i + 1] | ||
if (entry == null || entry.heartRate == null) continue | ||
|
||
if (entry.heartRate == null || nextEntry.heartRate == null) continue | ||
let { nextEntry, nextIndex } = this.getNextValidEntry(session, entry, i) | ||
i = nextIndex // skip loop to latest valid entry |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if i = nextIndex
, next loop i
will be +1
since i++
records[0] -> entry
records[1] -> nextEntry
records[2] -> next loop entry will be this ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, it should be i = nextIndex -1
for (let i = 0; i < session.records.length - 1; i++) { | ||
const entry = session.records[i] | ||
const nextEntry = session.records[i + 1] | ||
if (entry == null || entry.heartRate == null) continue |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually entry will never be null, it's guaranteed. but this is okay too, no worries.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually entry will never be null, it's guaranteed. but this is okay too, no worries.
i think this leftover from previous one that Record can be null (cause linter red), i'll check more on this
: Math.min( | ||
Math.max(this.hrZones[zoneIndex].minmax[0], nextEntry.heartRate ?? 0), | ||
this.hrZones[zoneIndex].minmax[1] | ||
) | ||
return 1 + total + (Math.max(end, start) - Math.min(end, start)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this can be simplified though
return 1 + total + Math.abs(start-end)
btw one question pls, if zoneIndex == -1
what will happen then? Can you give a little illustration regarding this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
entry.heartRate = 50
nextEntry.heartRate = 200
assume 50 is below zone 1, and 200 is above zone 5, it will belong to the zone who has (200-50) 150bpm in it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe more real world case:
entry.heartRate = 50 (assume not belong to zone)
entry.heartRate = 90 (assume zone 1)
it will add 40 steps then distribute them evenly based on elapsed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
entry.heartRate = 50
nextEntry.heartRate = 200
It will do transition 50 (inclusive) to 200 (inclusive) with each value added time to zone (1/151 * deltaTime)
roughly figure like this
200 BPM = 1/151 * deltaTime -> Zone 5
...
50 BPM = 1/151 * deltaTime -> No Zone
because Zone 5 is 90% to Infinity, therefore 200+ still counted as Zone 5.
because Zone 1 is 50%-60%, therefore 50 didn't counted as Zone 1 and discarded.
… loop. remove record check null (from previous method calculation). remove some log
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
optimize finding hr zone using temporary local cache to efficiency finding hr zone on large records
Minor optimize performance |