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

Planner node #6

Merged
merged 13 commits into from
Jul 10, 2023
Merged

Planner node #6

merged 13 commits into from
Jul 10, 2023

Conversation

rdyro
Copy link
Contributor

@rdyro rdyro commented May 2, 2023

Adding the first draft of the path planning node.

The main path planning code is contained in ./ff_path_planning.

This node defines a path planning service, the new service definition is in ./ff_srvs.

See ./ff_path_planning/README.md for more details.

@rdyro
Copy link
Contributor Author

rdyro commented May 31, 2023

I added two important improvements:

  • hotloading of costs and dynamics files is now possible (in the ff_path_planning package source)
  • cost functions now accept and extra argument params_json in the service request in ros as a string
    • this json will be deserialized by the path planning node
    • params will be provided as an extra argument to the cost function
    • this allows costs that change over time, like the end goal or evolving reference trajectory or adding and removing obstacles

Copy link
Contributor

@alvinsunyixiao alvinsunyixiao left a comment

Choose a reason for hiding this comment

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

Looks good overall! Left a few comments on small details.

ff_path_planning/package.xml Show resolved Hide resolved
freeflyer/launch/sim_path_planning.launch.py Outdated Show resolved Hide resolved
ff_path_planning/ff_path_planning/costs/default_cost.py Outdated Show resolved Hide resolved
ff_path_planning/ff_path_planning/examples/setting_goal.py Outdated Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

This file can be removed since we are using black for style checking

Copy link
Contributor

Choose a reason for hiding this comment

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

Also can be removed.

Comment on lines 21 to 27
# the following line skips the linter which checks for copyrights
# comment the line when a copyright and license is added to all source files
set(ament_cmake_copyright_FOUND TRUE)
# the following line skips cpplint (only works in a git repo)
# comment the line when this package is in a git repo and when
# a copyright and license is added to all source files
set(ament_cmake_cpplint_FOUND TRUE)
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove to enable testing

Suggested change
# the following line skips the linter which checks for copyrights
# comment the line when a copyright and license is added to all source files
set(ament_cmake_copyright_FOUND TRUE)
# the following line skips cpplint (only works in a git repo)
# comment the line when this package is in a git repo and when
# a copyright and license is added to all source files
set(ament_cmake_cpplint_FOUND TRUE)

Copy link
Contributor Author

@rdyro rdyro left a comment

Choose a reason for hiding this comment

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

Reviewed and incorporated changes

@rdyro rdyro merged commit c17e10f into main Jul 10, 2023
2 checks passed
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