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

option to use pace #33

Closed
wants to merge 4 commits into from
Closed

Conversation

davidedelvento
Copy link

I was hoping to include the pace (inverse of speed) with not these many lines of codes, but to do so I should have refactored many pieces of the existing infrastructure which has a fair bit of repetition. Instead, I opted for keeping the same infrastructure (and repetitions) as is, and duplicate it for the pace.

Moreover, the pace is a time over a fixed distance, and is usually shown as minute:seconds. To do so, I would have to change many things, including how the labels for the y axis are created in the plotting library, using strings instead of number. Instead, I choose to give the user the option to select either minutes or seconds (per either km or miles) and keeping them numbers instead of having to mess up with strings.

Last, but not least, being pace the inverse of speed, it diverges when one is not moving, which is exactly what one wants (how long would it take to cover a mile if you are standing still? It would take forever, or infinity -- how long if you are going slower than ant's step? Again almost forever or a very large number). Therefore, like in all systems that use pace, I set a threshold in globals.h, above which pace is not computed/plotted.

In the end, it seems to be working well.

@rnorris
Copy link
Collaborator

rnorris commented Aug 21, 2017

The calculations for pace appear wrong.
Viking's internal speed unit is m/s, whereas your definitions seem to be using it as kph.
Thus for instance in calculating Min/Km -> you have 60.0/(X),
whereas it should be 60.0/((X) * VIK_KPH_IN_MPS)
Similarly all the other conversions are wrong by the same factor.

Speed is also used in the file 'viktrwlayer_analysis.c'.
This used in the 'Statistics' window, available in the menu from right clicking on an aggregate layer or the Tracks sublayer in the layers panel.

Finally some small style points for the globals.h definitions, please use full numbers i.e. "1.0" rather than "1."
Also a space before the '?' would also make things clearer in my opinion.

@davidedelvento
Copy link
Author

davidedelvento commented Aug 27, 2017

@rnorris You are of course right. Besides myself assuming km/h instead of m/s, some test data I had to verify the numbers had also been off by a similar factor, masking the problem to my eyes.

I added the section about the statistics that you mentioned, used the style you suggested (I had been thinking of incorporating the 3.6 into my constants, but decided against it) and there was also a typo in a label.

Let's give it another try. Thanks for your patience.

@rnorris
Copy link
Collaborator

rnorris commented Oct 1, 2017

This has been integrated into the main branch.
I took liberty to squash these commits into a single commit and a small edit to remove a superfluous carriage return in the output in viktrwlayer_analysis.c (the new code was based on previous code which had this flaw).

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