Skip to content

Conversation

ajtudela
Copy link
Contributor


Basic Info

Info Please fill out this column
Ticket(s) this addresses
Primary OS tested on Ubuntu 24.04
Robotic platform tested on Morphia
Does this PR contain AI generated software? Nope
Was this PR description generated by AI software? Out of respect for maintainers, AI for human-to-human communications are banned

Description of contribution in a few bullet points

This PR introduces a new following_server, which provides a dynamic object-following task server.

Description of documentation updates required from your changes

  • New configuration and migration docs its needed.

Description of how this change was tested

  • I wrote unit tests that cover ~90% of changes and tested on my physical robot.
  • Performed linting validation using pre-commit run --all or colcon test

Future work that may be required in bullet points

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
  • Should this be backported to current distributions? If so, tag with backport-*.

Copy link
Contributor

mergify bot commented Sep 30, 2025

@ajtudela, 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).

@ajtudela
Copy link
Contributor Author

I'm not sure why the build failed :S

@SteveMacenski
Copy link
Member


Starting >>> opennav_following
[3407.392s] ERROR:colcon.colcon_cmake.task.cmake.build:Failed to find the following files:
- /opt/overlay_ws/install/opennav_docking_core/share/opennav_docking_core/package.sh
- /opt/overlay_ws/install/opennav_docking/share/opennav_docking/package.sh
Check that the following packages have been built:
- opennav_docking_core
- opennav_docking
Failed   <<< opennav_following [0.01s, exited with code 1]

Perhaps these are missing from the following server's package xml?

@SteveMacenski
Copy link
Member

Oh I see, add the package(s) to https://github.com/ros-navigation/navigation2/blob/main/.circleci/config.yml#L535 regex list (we split the build jobs into 2 now because we have so much fucking code now)

@SteveMacenski
Copy link
Member

SteveMacenski commented Sep 30, 2025

Also, adding the Python API to call the Following Server would be nice! As well as bringup using Nav2 Bringup's navigation.launch.py so its out-of-the-box

@ajtudela
Copy link
Contributor Author

ajtudela commented Oct 1, 2025

I have a problem with mypy I can't fix:

nav2_simple_commander/nav2_simple_commander/robot_navigator.py: note: In member "__init__" of class "BasicNavigator":
nav2_simple_commander/nav2_simple_commander/robot_navigator.py:110: error: Name "NavigateThroughPoses.Goal" is not defined  [name-defined]
nav2_simple_commander/nav2_simple_commander/robot_navigator.py:110: error: Name "NavigateThroughPoses.Result" is not defined  [name-defined]
nav2_simple_commander/nav2_simple_commander/robot_navigator.py:110: error: Name "NavigateThroughPoses.Feedback" is not defined  [name-defined

Using the line '# type: ignore[name-defined]' passes the pre-commit, but an error is shown in the build.

@ajtudela ajtudela force-pushed the following branch 2 times, most recently from c512483 to e98e968 Compare October 1, 2025 12:27
@SteveMacenski
Copy link
Member

@leander-dsouza can you look at the linting issue? Can this static analysis be lightened up? I think we're hitting a point that its more a bother than a help

@SteveMacenski
Copy link
Member

@ajtudela you have a couple of tests failing in appear to look like real ways as part of the test python code

@SteveMacenski
Copy link
Member

One more thing... Add the error code prefix default to bt_action_server_impl.hpp as well as in the nav2_params.yaml so the following error codes are considered

  std::vector<std::string> error_code_name_prefixes = {
    "assisted_teleop",
    "backup",
    "compute_path",
    "dock_robot",
    "drive_on_heading",
    "follow_path",
    "nav_thru_poses",
    "nav_to_pose",
    "spin",
    "undock_robot",
    "wait",
  };

@leander-dsouza
Copy link
Contributor

leander-dsouza commented Oct 2, 2025

@leander-dsouza can you look at the linting issue? Can this static analysis be lightened up? I think we're hitting a point that its more a bother than a help

Yes, I'll make a PR shortly to fix all the ament_mypy issues.
I think we can drop the --strict flag henceforth if the CI fails again.

@ajtudela
Copy link
Contributor Author

ajtudela commented Oct 2, 2025

@ajtudela you have a couple of tests failing in appear to look like real ways as part of the test python code

I fixed the tests but the CI fail in other tests, I think...

@SteveMacenski
Copy link
Member

@ajtudela can you pull in main? We have CI green again so anything failing after that point should be things only from this PR. the speed/keepout are something in CI that was fixed. The error codes I think may actually be real given this PR's changes (but TBD)

@ajtudela
Copy link
Contributor Author

ajtudela commented Oct 3, 2025

Everything is working now, except for mypy.

Also, it seems that the code coverage is looking for the old opennav_following_bt files.

@SteveMacenski
Copy link
Member

Ah, that'll happen. Update the v39 to v40 in the 3 places within this highlighted block and that should be fixed: https://github.com/ros-navigation/navigation2/blob/main/.circleci/config.yml#L36-L61

@SteveMacenski
Copy link
Member

Make sure to add them here: https://github.com/ros-navigation/navigation2/blob/main/navigation2/package.xml so they're part of the nav2 metapackage for installation!

Copy link
Contributor

mergify bot commented Oct 6, 2025

@ajtudela, 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).

Copy link
Contributor

mergify bot commented Oct 6, 2025

This pull request is in conflict. Could you fix it @ajtudela?

@SteveMacenski
Copy link
Member

@ajtudela I think just getting CI to turn over and the merge conflict is all that's needed to merge!

@SteveMacenski
Copy link
Member

SteveMacenski commented Oct 7, 2025

Retriggering CI, if it doesn't build when I look tomorrow I'll investigate more. Seems like a timing issue

Though mypy has one pedantic real issue left:

Error: nav2_simple_commander/nav2_simple_commander/robot_navigator.py:27:1: error: Module "nav2_msgs.action" has no attribute "FollowObject" [attr-defined]

Copy link
Contributor

mergify bot commented Oct 8, 2025

@ajtudela, 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).

Signed-off-by: Alberto Tudela <[email protected]>
Signed-off-by: Alberto Tudela <[email protected]>
Copy link
Contributor

mergify bot commented Oct 9, 2025

@ajtudela, 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).

@SteveMacenski SteveMacenski merged commit ce6b439 into ros-navigation:main Oct 9, 2025
12 of 13 checks passed
silanus23 pushed a commit to silanus23/navigation2 that referenced this pull request Oct 11, 2025
* Following server

Signed-off-by: Alberto Tudela <[email protected]>

* feat: migrate to nav2_msgs for FollowObject action and remove opennav_following_msgs

Signed-off-by: Alberto Tudela <[email protected]>

* Fix mypy test

Signed-off-by: Alberto Tudela <[email protected]>

* Move FollowObject and FollowObjectCancel action nodes from bt package

Signed-off-by: Alberto Tudela <[email protected]>

* Update package and circle

Signed-off-by: Alberto Tudela <[email protected]>

* feat: add FollowObject action support in robot navigator and fix mypy

Signed-off-by: Alberto Tudela <[email protected]>

* Fix mypy issues

Signed-off-by: Alberto Tudela <[email protected]>

* feat: add follow_object action to bt_navigator parameters

Signed-off-by: Alberto Tudela <[email protected]>

* Update key and package

Signed-off-by: Alberto Tudela <[email protected]>

* Fix precommit

Signed-off-by: Alberto Tudela <[email protected]>

* Fix mypy?

Signed-off-by: Alberto Tudela <[email protected]>

* Fix mypy, round two

Signed-off-by: Alberto Tudela <[email protected]>

---------

Signed-off-by: Alberto Tudela <[email protected]>
Jad-ELHAJJ pushed a commit to Jad-ELHAJJ/navigation2 that referenced this pull request Oct 16, 2025
* Following server

Signed-off-by: Alberto Tudela <[email protected]>

* feat: migrate to nav2_msgs for FollowObject action and remove opennav_following_msgs

Signed-off-by: Alberto Tudela <[email protected]>

* Fix mypy test

Signed-off-by: Alberto Tudela <[email protected]>

* Move FollowObject and FollowObjectCancel action nodes from bt package

Signed-off-by: Alberto Tudela <[email protected]>

* Update package and circle

Signed-off-by: Alberto Tudela <[email protected]>

* feat: add FollowObject action support in robot navigator and fix mypy

Signed-off-by: Alberto Tudela <[email protected]>

* Fix mypy issues

Signed-off-by: Alberto Tudela <[email protected]>

* feat: add follow_object action to bt_navigator parameters

Signed-off-by: Alberto Tudela <[email protected]>

* Update key and package

Signed-off-by: Alberto Tudela <[email protected]>

* Fix precommit

Signed-off-by: Alberto Tudela <[email protected]>

* Fix mypy?

Signed-off-by: Alberto Tudela <[email protected]>

* Fix mypy, round two

Signed-off-by: Alberto Tudela <[email protected]>

---------

Signed-off-by: Alberto Tudela <[email protected]>
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.

3 participants