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

purePursuit: migrate parameters to library #23438

Merged
merged 3 commits into from
Jul 30, 2024

Conversation

chfriedrich98
Copy link
Contributor

@chfriedrich98 chfriedrich98 commented Jul 22, 2024

Solved Problem

The pure pursuit library introduced in #23387 requires some parameters that are currently defined in the ackermann module.
Migrating them to the library itself avoids having to redifine the same parameters for every module that uses the pure pursuit library in the future.
This effects the following parameters:

Parameter Description Unit
RA_LOOKAHD_GAIN $\rightarrow$ PP_LOOKAHD_GAIN Main tuning parameter -
RA_LOOKAHD_MAX $\rightarrow$ PP_LOOKAHD_MAX Maximum value for the look ahead radius m
RA_LOOKAHD_MIN $\rightarrow$ PP_LOOKAHD_MIN Minimum value for the look ahead radius m

@chfriedrich98 chfriedrich98 added the Rover 🚙 Rovers and other UGV label Jul 22, 2024
@chfriedrich98 chfriedrich98 self-assigned this Jul 22, 2024
sfuhrer
sfuhrer previously approved these changes Jul 26, 2024
Copy link
Contributor

@sfuhrer sfuhrer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a clean iteration!

@chfriedrich98 chfriedrich98 merged commit b93dd0e into PX4:main Jul 30, 2024
88 of 92 checks passed
@hamishwillee
Copy link
Contributor

hamishwillee commented Aug 7, 2024

@chfriedrich98 Good change.

FYI If you ever touch a parameter name, worth doing a search in the docs. If you add a parameter think "what difference might this make to end users" because perhaps new docs are needed.

The reason I note this is that the parameters auto updated and broke some links you fixed up previously in PX4/PX4-user_guide#3265.

I do plan to add some automation to remind developers, and perhaps move docs into this repo so you might find them with search and replace. Ultimately though we need to think about what end-user impacts changes might have.

P.S. Should also do search replace in the source - because these also mentioned in https://github.com/PX4/PX4-Autopilot/blob/main/src/lib/pure_pursuit/PurePursuit.hpp#L98-L100

@chfriedrich98
Copy link
Contributor Author

@hamishwillee Thanks for the reminder!
I intended to fix the parameter names with the rover restructure in PX4/PX4-user_guide#3311, but it makes sense to update it immediately to avoid any confusion in the meantime.

P.S. Should also do search replace in the source - because these also mentioned in https://github.com/PX4/PX4-Autopilot/blob/main/src/lib/pure_pursuit/PurePursuit.hpp#L98-L100

The fix for that is part of this PR: https://github.com/PX4/PX4-Autopilot/pull/23470/files.

vertiq-jordan pushed a commit to iq-motion-control/PX4-Autopilot that referenced this pull request Aug 21, 2024
@chfriedrich98 chfriedrich98 deleted the pure_pursuit_params branch August 29, 2024 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Rover 🚙 Rovers and other UGV
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants