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

Refactor plugins to use double colons for consistency #4220

Merged
merged 3 commits into from
Apr 2, 2024

Conversation

alanxuefei
Copy link
Contributor

@alanxuefei alanxuefei commented Mar 27, 2024

Tested in Gazobo world.
The turtle bot runs as normal.

The CI/CD process fails due to the rosdep issue during the Docker build stage. I encountered the same issue on my local PC and resolved it by using apt install.

ERROR: the following packages/stacks could not have their rosdep keys resolved
to system dependencies:
dwb_msgs: No definition of [rosidl_default_runtime] for OS version [jammy]
costmap_queue: No definition of [rclcpp] for OS version [jammy]
nav2_behaviors: No definition of [pluginlib] for OS version [jammy]
nav2_common: No definition of [rclpy] for OS version [jammy] 

Basic Info

Info Please fill out this column
Ticket(s) this addresses (add tickets here #4194)
Primary OS tested on (Ubuntu,)
Robotic platform tested on (gazebo simulation of turtlebot)
Does this PR contain AI generated software? (No)

Description of contribution in a few bullet points

Description of documentation updates required from your changes

N.A.


Future work that may be required in bullet points

N.A.

For Maintainers:

  • Check that any new parameters added are updated in navigation.ros.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

Copy link
Contributor

mergify bot commented Mar 27, 2024

@alanxuefei, 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 Mar 27, 2024

@alanxuefei, 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 Mar 27, 2024

@alanxuefei, 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 Mar 27, 2024

@alanxuefei, 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 Mar 27, 2024

@alanxuefei, 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 Mar 27, 2024

@alanxuefei, 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 Mar 27, 2024

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

@alanxuefei alanxuefei marked this pull request as ready for review March 27, 2024 13:58
Copy link
Member

@SteveMacenski SteveMacenski left a comment

Choose a reason for hiding this comment

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

Please update the docs with the updated consistency (tutorials/config guide/navigation plugins list/etc) + add to the migration guide since this is a breaking change

@alanxuefei
Copy link
Contributor Author

Please update the docs with the updated consistency (tutorials/config guide/navigation plugins list/etc) + add to the migration guide since this is a breaking change

Ok will create a pull request in https://github.com/ros-planning/navigation.ros.org

@alanxuefei
Copy link
Contributor Author

Please update the docs with the updated consistency (tutorials/config guide/navigation plugins list/etc) + add to the migration guide since this is a breaking change

A pull request is created at ros-navigation/docs.nav2.org#539

@alanxuefei
Copy link
Contributor Author

The following files only appeared on the Nav2 website, not in Nav2 Codes. Therefore, there is no need to factor them in this PR. The Nav2 website has been updated accordingly.

  • nav2_sms_behavior/SendSms
  • nav2_gradient_costmap_plugin/GradientLayer
  • nav2_straightline_planner/StraightLine

@SteveMacenski
Copy link
Member

OK! I'll review shortly. Last thing on this PR is that the DCO has failed

Copy link
Contributor

mergify bot commented Mar 29, 2024

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

@alanxuefei
Copy link
Contributor Author

**DCO ** — DCO

DCO passed. The signature is added.

@SteveMacenski
Copy link
Member

@alanxuefei minor clarification is needed in the docs PR, then will merge the pair, thanks!

@SteveMacenski
Copy link
Member

Actually, please pull in main, CI should be back online again and I want to see that we didn't miss anything here

@SteveMacenski
Copy link
Member

Docs approved, just need to have main pulled in to run CI and make sure nothing glaring is failing

Signed-off-by: Alan Xue <alan.xuefei@googlemail.com>
Signed-off-by: Alan Xue <alan.xuefei@googlemail.com>
Signed-off-by: Alan Xue <alan.xuefei@googlemail.com>
@alanxuefei
Copy link
Contributor Author

alanxuefei commented Mar 31, 2024

Docs approved, just need to have main pulled in to run CI and make sure nothing glaring is failing

Rebased to main.

The number of failed tests are the same as the main branch.
Summary: 3812 tests, 5 errors, 8 failures, 757 skipped

By the way, should we fix the failed tests?

@SteveMacenski
Copy link
Member

SteveMacenski commented Apr 2, 2024

By the way, should we fix the failed tests?

Yes! I just haven't gotten to it yet. I only fixed CI with the Rolling transient issues on Friday. If you wanted to take a few moments to look over the system tests to see what's wrong, that would be greatly appreciated! Only so many hours in the day unfortunately for myself :(

@SteveMacenski SteveMacenski merged commit b0e6d3d into ros-navigation:main Apr 2, 2024
9 of 11 checks passed
@alanxuefei
Copy link
Contributor Author

By the way, should we fix the failed tests?

Yes! I just haven't gotten to it yet. I only fixed CI with the Rolling transient issues on Friday. If you wanted to take a few moments to look over the system tests to see what's wrong, that would be greatly appreciated! Only so many hours in the day unfortunately for myself :(

I'll try to fix some tests in my spare time. Thanks for the clarification. I thought that some failures might be due to the randomness inherent in the robotic algorithm. Unfortunately, or fortunately, everyone has only 24 hours in a day.

@SteveMacenski
Copy link
Member

SteveMacenski commented Apr 4, 2024

I actually fixed them already :-)

The spin/backup tests are flaky and I've gone over and fixed and refixed them now a dozen times. I'm at really half a mind just to delete them and restart they really shouldn't be rocket science of all the tests we have 😆

ajtudela pushed a commit to grupo-avispa/navigation2 that referenced this pull request Apr 19, 2024
…#4220)

* Refactor the plugin names for bt_navigator to use double colons

Signed-off-by: Alan Xue <alan.xuefei@googlemail.com>

* Refactor the plugin names for planner_server to use double colons

Signed-off-by: Alan Xue <alan.xuefei@googlemail.com>

* Refactor the plugin names for behavior_server to use double colons

Signed-off-by: Alan Xue <alan.xuefei@googlemail.com>

---------

Signed-off-by: Alan Xue <alan.xuefei@googlemail.com>
enricosutera pushed a commit to enricosutera/navigation2 that referenced this pull request May 19, 2024
…#4220)

* Refactor the plugin names for bt_navigator to use double colons

Signed-off-by: Alan Xue <alan.xuefei@googlemail.com>

* Refactor the plugin names for planner_server to use double colons

Signed-off-by: Alan Xue <alan.xuefei@googlemail.com>

* Refactor the plugin names for behavior_server to use double colons

Signed-off-by: Alan Xue <alan.xuefei@googlemail.com>

---------

Signed-off-by: Alan Xue <alan.xuefei@googlemail.com>
Signed-off-by: enricosutera <enricosutera@outlook.com>
Marc-Morcos pushed a commit to Marc-Morcos/navigation2 that referenced this pull request Jul 4, 2024
…#4220)

* Refactor the plugin names for bt_navigator to use double colons

Signed-off-by: Alan Xue <alan.xuefei@googlemail.com>

* Refactor the plugin names for planner_server to use double colons

Signed-off-by: Alan Xue <alan.xuefei@googlemail.com>

* Refactor the plugin names for behavior_server to use double colons

Signed-off-by: Alan Xue <alan.xuefei@googlemail.com>

---------

Signed-off-by: Alan Xue <alan.xuefei@googlemail.com>
savalena pushed a commit to savalena/navigation2 that referenced this pull request Jul 5, 2024
…#4220)

* Refactor the plugin names for bt_navigator to use double colons

Signed-off-by: Alan Xue <alan.xuefei@googlemail.com>

* Refactor the plugin names for planner_server to use double colons

Signed-off-by: Alan Xue <alan.xuefei@googlemail.com>

* Refactor the plugin names for behavior_server to use double colons

Signed-off-by: Alan Xue <alan.xuefei@googlemail.com>

---------

Signed-off-by: Alan Xue <alan.xuefei@googlemail.com>
Manos-G pushed a commit to Manos-G/navigation2 that referenced this pull request Aug 1, 2024
…#4220)

* Refactor the plugin names for bt_navigator to use double colons

Signed-off-by: Alan Xue <alan.xuefei@googlemail.com>

* Refactor the plugin names for planner_server to use double colons

Signed-off-by: Alan Xue <alan.xuefei@googlemail.com>

* Refactor the plugin names for behavior_server to use double colons

Signed-off-by: Alan Xue <alan.xuefei@googlemail.com>

---------

Signed-off-by: Alan Xue <alan.xuefei@googlemail.com>
masf7g pushed a commit to quasi-robotics/navigation2 that referenced this pull request Oct 22, 2024
…#4220)

* Refactor the plugin names for bt_navigator to use double colons

Signed-off-by: Alan Xue <alan.xuefei@googlemail.com>

* Refactor the plugin names for planner_server to use double colons

Signed-off-by: Alan Xue <alan.xuefei@googlemail.com>

* Refactor the plugin names for behavior_server to use double colons

Signed-off-by: Alan Xue <alan.xuefei@googlemail.com>

---------

Signed-off-by: Alan Xue <alan.xuefei@googlemail.com>
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