Skip to content

Global: Add units to the method that calculates the distance meter between waypoin ts #30007

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

muramura
Copy link
Contributor

@muramura muramura commented May 7, 2025

The distance between WAYPOINTs is in centimeters.
I will add the unit to the method.

@rmackay9
Copy link
Contributor

rmackay9 commented May 7, 2025

Thanks, I'm happy with this change but we should check that it doesn't interfere with @lthall and @peterbarker's ongoing work to rationalise the units of follow mode and the position controller

@peterbarker
Copy link
Contributor

This is a good patch, it's definitely in-line with what @lthall has been up to.

However, I would also suggest we could merge a PR which renames this method to add _m and starts to return metres rather than cm.

The reason we can do this is that you are changing all of the relevant code here, whereas the work Leonard is doing is going to lead to a bunch of subsequent PRs to start using base-SI-units.

@muramura could you put together an alternate PR which renames the current method and makes it return metres, and then change all callers to use the new method and cope with the metres return value, please?

@muramura muramura force-pushed the AP_Add_units_to_the_method_that_calculates_the_distance_between_WAYPOINTs branch from 79ae6d4 to f729b21 Compare May 10, 2025 01:17
@muramura muramura changed the title Global: Add units to the method that calculates the distance between waypoin ts Global: Add units to the method that calculates the distance meter between waypoin ts May 10, 2025
@muramura
Copy link
Contributor Author

Convert centimeters to meters.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

3 participants