navigation2 icon indicating copy to clipboard operation
navigation2 copied to clipboard

Nav2 toolkit

Open pranavk-2003 opened this issue 7 months ago • 15 comments


Basic Info

Info Please fill out this column
Ticket(s) this addresses #5070
Primary OS tested on Ubuntu
Robotic platform tested on Turtlebot gazebo simulation.
Does this PR contain AI generated software? Somewhat

Description of testing performed

Functional tests were conducted on turtlebot3 and are working as expected

Description of contribution in a few bullet points

  • This Contribution adds a new package named nav2_toolkit which has QOL features, which can be handy while working with Nav2 .

Description of documentation updates required from your changes

  • Addition of 3 new parameterauto_start_saving, auto_restore_pose, save_interval_sec, so need to add that to nav2_params and documentation page

Description of how this change was tested

All the changes were tested multiple times in simulation over a course of 3 weeks.


Future work that may be required in bullet points

In total , this package will contain 3 tools i.e

  • [x] pose_saver & pose_restore with added launch file in nav2_bringup.
  • [ ] Relocation of BaseFootprintPublisher.
  • [ ] A node which can pause and resume nav2 goals.

For Maintainers:

  • [ ] Check that any new parameters added are updated in docs.nav2.org
  • [ ] Check that any significant change is added to the migration guide
  • [ ] Check that any new features OR changes to existing behaviors are reflected in the tuning guide
  • [ ] Check that any new functions have Doxygen added
  • [ ] Check that any new features have test coverage
  • [ ] Check that any new plugins is added to the plugins page
  • [ ] If BT Node, Additionally: add to BT's XML index of nodes for groot, BT package's readme table, and BT library lists

pranavk-2003 avatar Apr 30 '25 13:04 pranavk-2003

@pranavk-2003, your PR has failed to build. Please check CI outputs and resolve issues. You may need to rebase or pull in main due to API changes (or your contribution genuinely fails).

mergify[bot] avatar May 06 '25 07:05 mergify[bot]

@pranavk-2003 let me know when you want me to take a look again! I see you pushed some changes but a number of items are not yet addressed :smile:

SteveMacenski avatar May 07 '25 18:05 SteveMacenski

@pranavk-2003 let me know when you want me to take a look again! I see you pushed some changes but a number of items are not yet addressed 😄

Its somewhat figured out,I'll let you know soon , got busy with university stuff ☹ .the few pushed stuff i still have to test.I am right now finding it difficult to get the nav2_package() macro right for nav2_toolkit.

pranavk-2003 avatar May 11 '25 14:05 pranavk-2003

Hey @SteveMacenski ,I think i am done with the code updates , you can now have a look again throughout.The doxygen docs is in progress. : ) Also the checks are failing , let me know what has to be done for that too.

pranavk-2003 avatar May 12 '25 15:05 pranavk-2003

@pranavk-2003 open the failing jobs and fix the issues they lay out :-) It looks like alot of static code quality issues in linting

SteveMacenski avatar May 12 '25 19:05 SteveMacenski

Needs test coverage as well!

Hey okay, so what shall it be?test as in demos for using the tool?

pranavk-2003 avatar May 14 '25 14:05 pranavk-2003

@pranavk-2003 open the failing jobs and fix the issues they lay out :-) It looks like alot of static code quality issues in linting

Okay i understood how to solve those errors, i would solve these after all the code adjustments are done , as the code adjustments would introduce more of them

pranavk-2003 avatar May 14 '25 14:05 pranavk-2003

Hey okay, so what shall it be?test as in demos for using the tool?

You should create a unit test that runs the object and then publishes some data to the topic its listening to. Check after awhile that there's a file at the requested location and the data is correct.

Then run another tests with autostart on and test that it correctly reads from that file and publishes / calls the service to set the pose correctly (the test object should expose the service, so nothing else should be running).

Then another test should be with autostart / auto-store off and call the services to start / stop / reset and check that it does what it should do.

Finally, a series of unit tests that checks read_pose_from_file write_pose_to_file individual function's correctness individually

CI should then tell you the coverage metrics in the codecov job - we need it to be at least 90% to be merged

SteveMacenski avatar May 14 '25 23:05 SteveMacenski

Also as a matter of practice, do not resolve the PR topic discussions. I'll hit "resolve" when I verify that it was completed as intended. It makes my life easier as a maintainer to know that I validated each of my previous comments in your updated fixes without having to go back through all of the collapsed tabs each time :-)

SteveMacenski avatar May 14 '25 23:05 SteveMacenski

This pull request is in conflict. Could you fix it @pranavk-2003?

mergify[bot] avatar May 20 '25 16:05 mergify[bot]

Resolved the conflicts, @SteveMacenski i'll let you know when i want you to check this PR again.

pranavk-2003 avatar May 21 '25 07:05 pranavk-2003

@pranavk-2003, your PR has failed to build. Please check CI outputs and resolve issues. You may need to rebase or pull in main due to API changes (or your contribution genuinely fails).

mergify[bot] avatar May 21 '25 07:05 mergify[bot]

Hey @SteveMacenski its been a while.I have added tests, adjusted the code according to your recommendations and enhancement suggestions.I have tried to solve and fix most of what we discussed last time.Please review the changes and let me know any modifications.Also dont worry about the checks as i will fix them after all the code changes are done.I have also relocated the BaseFootprintPublisher to nav2_toolkit.:)

pranavk-2003 avatar Jun 23 '25 17:06 pranavk-2003

@pranavk-2003, your PR has failed to build. Please check CI outputs and resolve issues. You may need to rebase or pull in main due to API changes (or your contribution genuinely fails).

mergify[bot] avatar Jun 23 '25 18:06 mergify[bot]

Sincere apologies @SteveMacenski for the delay between last and this update and this dual mention .I understand its alot of work to review this PR from the start again , but i'll make sure to wrapup this PR to the earliest from now without any delays.

pranavk-2003 avatar Jun 27 '25 16:06 pranavk-2003

OK! Let me know when you're ready. Please also heed the linters + DCO sign offs. It will make it much easier for me to review if the styling is consistent and I'm not distracted with pointing out pedantic errors. It also doesn't seem to build - which would be a minimum requirement for me to review.

Try rebasing / pulling in main

SteveMacenski avatar Jul 02 '25 18:07 SteveMacenski

@pranavk-2003, your PR has failed to build. Please check CI outputs and resolve issues. You may need to rebase or pull in main due to API changes (or your contribution genuinely fails).

mergify[bot] avatar Jul 03 '25 04:07 mergify[bot]

@pranavk-2003, your PR has failed to build. Please check CI outputs and resolve issues. You may need to rebase or pull in main due to API changes (or your contribution genuinely fails).

mergify[bot] avatar Jul 03 '25 16:07 mergify[bot]

@pranavk-2003, your PR has failed to build. Please check CI outputs and resolve issues. You may need to rebase or pull in main due to API changes (or your contribution genuinely fails).

mergify[bot] avatar Jul 03 '25 17:07 mergify[bot]

Sorry for the email spams of builds, ill remove you as a maintainer for now so it would not spam you.

pranavk-2003 avatar Jul 03 '25 17:07 pranavk-2003

Hi @SteveMacenski i think i might have done something wrong while trying to sign off my old commits :( . What should i do? maybe make a new draft PR.Although i Fixed most of the Build check fails.i would guess the failing ones are because of DCO.Please guide me through this.Apologies for the extra steps — really appreciate your patience.

pranavk-2003 avatar Jul 05 '25 15:07 pranavk-2003

There are alot of merge conflicts. You could either resolve them or possibly try applying your work on an up-to-date branch and opening a new PR. I'm still seeing alot of build and linter errors if you check the failing red jobs above. They're mostly from linting that if you check on the linting jobs will tell you exactly how to fix them

SteveMacenski avatar Jul 14 '25 20:07 SteveMacenski

This pull request is in conflict. Could you fix it @pranavk-2003?

mergify[bot] avatar Jul 18 '25 15:07 mergify[bot]

@pranavk-2003, your PR has failed to build. Please check CI outputs and resolve issues. You may need to rebase or pull in main due to API changes (or your contribution genuinely fails).

mergify[bot] avatar Jul 18 '25 15:07 mergify[bot]