Skip to content

Enhance openambit2gpx to add heart rate and improve commandline-handling - #215

Merged
centic9 merged 6 commits into
openambitproject:masterfrom
vascotenner:gpxconvert
Feb 27, 2021
Merged

Enhance openambit2gpx to add heart rate and improve commandline-handling#215
centic9 merged 6 commits into
openambitproject:masterfrom
vascotenner:gpxconvert

Conversation

@vascotenner

Copy link
Copy Markdown
Contributor

Added more advanced argument parsing. Make output file optional. Allows for averaging heart rate data.

@vascotenner

Copy link
Copy Markdown
Contributor Author

@myadzel: I have been thinking about averaging. My first thought was indeed that is should be not in this script. However, after some more thoughts about what information is really important for the average user I decided that averaging is very usefull. For that reason I have added a switch to switch off the averaging behaviour (--no-avg-hr). The converter does not do an 1-to-1 conversion to gpx anyway; The original log file contains more data: the sensor data is only written at the location of a GPS point.

I am not aware of any normalizers in use. Do you have a reference to such normalizers? In point of usability of the script, the inclusion of a normalizer is nice.

@myadzel

myadzel commented Jan 23, 2019

Copy link
Copy Markdown
Contributor

@vascotenner

We are talking about different things. I mean that you code filtering the pulse values less than 40 and more than 220 (my initial comment for this line in the code) and that it does not need to be done in this function. Its purpose is just to convert IBI data to HR and all:

HR = 60000 / IBI

The script does not have to filter values that go beyond the threshold. This is the work of "normalizers", but in this case it is not necessary - openambit receives data, and normalization is already their processing, which should be in another program (if someone needs it).

@vascotenner

vascotenner commented Jan 24, 2019

Copy link
Copy Markdown
Contributor Author

The issue there is that in the conversion process, HR is dropped or extrapolated. In that case noise in the sensor data can be amplified. Reasonable and traceble assumptions on the data can reduce the noise in the data. But I agree, this a certain choice that might be overriden by a power-user. The question is than if we need more customable "features", that is good for a power-user, or a more simpler (easier to use and to maintain) script that is working for 90% of the users?

I hope to push and update to this PR that makes it possible to turn of this behaviour.

@centic9

centic9 commented Jan 1, 2021

Copy link
Copy Markdown
Contributor

If there is still interest in these changes, it would be good to split them into separate changes so they can be applied and tested separately.

@centic9 centic9 changed the title Gpxconvert Enhance openambit2gpx to add hear rate and improve commandline-handling Jan 1, 2021
@vascotenner

Copy link
Copy Markdown
Contributor Author

I am using this script every week, and hence it would be nice of it ends up in master. However, I do not have time to rework this part in separated changes. It is quite a rebuild of the previous scripts, because during development I encountered some additional bugs. If interested, I can resolve the conflict with master.

@centic9

centic9 commented Feb 14, 2021

Copy link
Copy Markdown
Contributor

I would be interested in improving the GPX creation some more, especially now that it becomes harder to let Suunto Movescount do the GPX creation.

However there were a few bugfixes applied to the version on master as well, how can we keep these included?

@centic9

centic9 commented Feb 14, 2021

Copy link
Copy Markdown
Contributor

PR #211 contains only the changes to how HR are computed.

@centic9 centic9 changed the title Enhance openambit2gpx to add hear rate and improve commandline-handling Enhance openambit2gpx to add heart rate and improve commandline-handling Feb 17, 2021
@vascotenner

Copy link
Copy Markdown
Contributor Author

When #271 is merged, I will work on integrating this one.

Are there currently any tests with input log and output gpx files available? I then could extend/update them to match this new additions.

@centic9

centic9 commented Feb 19, 2021

Copy link
Copy Markdown
Contributor

There is a little bit of testing as part of test.sh, but it currently only performs the transformation of a single log to gpx and verifies it against a "template".

I just saw that this is actually broken by the IBI changes as we do not export HR "0" any longer (which is useful anyway), but the test is easy to adjust by updating the stored .gpx file.

@vascotenner

vascotenner commented Feb 19, 2021

Copy link
Copy Markdown
Contributor Author

We actually did HR=0, but not in the old version. I have update the testscript, and changed to IBI to HR code to only report HR when there is something. This solves the hr=0 issues

@centic9
centic9 merged commit f332cbb into openambitproject:master Feb 27, 2021
@centic9

centic9 commented Feb 27, 2021

Copy link
Copy Markdown
Contributor

Thanks for the work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants