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

[QMS-651] Add configuration option to handle distance rounding #657

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

mitxel-m
Copy link
Contributor

@mitxel-m mitxel-m commented Dec 20, 2023

What is the linked issue for this pull request:

QMS-#651

What you have done:

Add a box in the units configuration that allows the user to set from which value the rounding of distances will be done for metric and imperial units.

Setup units _001

  • The default value is 20 which corresponds to the current behaviour.
  • No configuration is necessary. If the Units config is not touched the current behaviour remains the same by default.
  • The modified value is saved in the configuration file for later use. It is named as roundLimit in the "Units" section.
[Units]
coordFormat=1
roundLimit=20
slopeMode=1
time\useShortFormat=true
timezone=@ByteArray(Europe/Madrid)
timezone\mode=3
type=0
  • It is not necessary that this value exists previously in the configuration file, if it does not exist it is set the first time we use the units dialog. This gives compatibility with existing conf files.

  • It respects the current concept (which I like) that the larger the distance the less decimal digits are displayed.

It is very simple to use. By default we have the value 20 which corresponds to the current value and gives us this:

range shown as
0-1 Km m
1-10 Km Km rounded to 2 decimals
10-20 Km Km rounded to 1 decimal
> 20 Km Km rounded to integer

If you set the value to 80 , it would use these ranges

range shown as
0-1 Km m
1-40 Km Km rounded to 2 decimals
40-80 Km Km rounded to 1 decimal
> 80 Km Km rounded to integer

and so on ...

Steps to perform a simple smoke test:

For convenience you can download this gpx file containing 5 tracks, named with different distances. Open it in QMS and go to edit the project, so that you see the summary with the data of all 5 tracks in the same view.

  1. First check that you see the distances with the decimals according to the current behaviour.

  2. Try with another value: 'Menu - View - Setup Units' Modify the value. You can use the mouse wheel. Then click on refresh (top right) on the project summary view and check how you see now the distances according to the value you have set. You can repeat it for different values.

  3. Check how the distances are displayed with the ruler.

  4. You can also change the unit to miles (Imperial) and see how it works.

Does the code comply to the coding rules and naming conventions Coding Guidelines:

  • [ x] yes

Is every user facing string in a tr() macro?

  • [ x] yes

Did you add the ticket number and title into the changelog? Keep the numeric order in each release block.

  • [ x] yes

Remark: My C++ skills are very limited and I have been able to write this by looking at other parts of the code. Probably my proposal can be refined.

@kiozen
Copy link
Collaborator

kiozen commented Jan 8, 2024

The PR does not take the differences between imperial and nautical units into account.

Internally QMS uses and stores metric values. When these values are passed to the UI or read from the UI you need to use one of the methods defined by the instance you get by calling IUnit::self(). This will also provide you with the correct unit string to be displayed.

Switching between the metric, imperial and nautical system must be a coherent user experience.

@mitxel-m mitxel-m changed the title Add configuration option to handle distance rounding [QMS-651] Add configuration option to handle distance rounding Jan 9, 2024
@mitxel-m
Copy link
Contributor Author

Ok, I will modify it.
The first proposal focused on making as few changes as possible in terms of usage.
That's why it only rounded distances for Metric and Imperial units, and kept them unrounded for Nautic and Aviation, exactly as is currently done in version 1.17.1

But I agree that to be coherent once the user gets the option to round distances, it should be applicable whatever units type he/she chooses.

So I'm planning to do :

  • make the rounding option applicable to all units types.
  • correctly display the unit string appended to the value in the UI box, according to the clicked option.

Setup units _002

would this meet the requirements?

Apart from that, and thanks to @kiozen 's previous comment, I have realised that this same thing is wrong in the UI of the speed filter for the tracks. It doesn't show the correct unit suffix, but always shows km/h even if you use mi/h, which leads to confusion.

I think I will be able to fix it, but I would like to do it in a different issue once we finish this one.

Display unit string in the UI box, according to the clicked option.
Minor fixes.
@mitxel-m
Copy link
Contributor Author

I think it is now ready to review with commit [1634499]

@kiozen
Copy link
Collaborator

kiozen commented Jan 16, 2024

I hope I find some time to work on this soon. Storing 20000m as 20 and applying all these hardcoded factors is hell to maintain. Imho the IUnit API should provide enough methods to handle this properly. However I can not sketch a simple solution for that. Therefore I need to find the time to create a patch.

I still doubt that this does make much sense with given resolution and precision of maps and GPS devices.

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