-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Move Nav2 CI to 24.04 / Rolling #4298
Move Nav2 CI to 24.04 / Rolling #4298
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll let @SteveMacenski comment on the CMake diffs, but only a few comments on the Docker/CI bits.
@tonynajjar, your PR has failed to build. Please check CI outputs and resolve issues. |
@tonynajjar my only beef left here is the cmake changes - I don't want to remove flags for our project due to dependency issues. Do you know how many files-ish (not exact; is it a lot or a little) have a problem due to this? If small, we can do what we did with other dependencies (example):
Though, I'm not sure why |
@tonynajjar, your PR has failed to build. Please check CI outputs and resolve issues. |
Okay, I restored the global compile flags and disabled them only where necessary. Currently we only know that the nav2_behavior_tree error will be resolved at the next sync. I'm not sure how to resolve the ones from nav2_system_tests and nav2_mppi_controller since the error originate from third-party code. |
@tonynajjar, your PR has failed to build. Please check CI outputs and resolve issues. |
At last review, this looks good to me. @ruffsl any thoughts? Otherwise, CI is failing to build (still looks to be on Jammy?) but no more code-related concerns. I'll probably file a few tickets when we merge as reminders but nothing that I'm going to concern you with |
OK, fixed. It looks like another instance of the destruction not working like described in the WPF comment above. To my unsophisticated eye, it looks like some of the context / RMW items aren't still active during the pre-shutdown callbacks, causing exceptions / internal ROS 2 things to crash / deadlock. When we fully set things down so that our pre-shutdown callback doesn't have to do any work or is avoided altogether by destroying the object before the rclcpp shutdown, then we avoid the problem. I think this last, short one does a good job at highlighting this:
@tonynajjar pull in my BR2 branch and this should be good to go pending any other issues that comes up from the CircleCI job versions of this |
@tonynajjar do you see the |
Just tried on rolling locally 10 times and I didn't get it |
Maybe break the cache with navigation2/.circleci/config.yml Line 488 in f671d38
|
Signed-off-by: Steve Macenski <[email protected]>
Signed-off-by: Steve Macenski <[email protected]>
* Fix devcontainer * fix * . * . * . * fix * pr comments * bust cache * test with docker image * Revert "test with docker image" This reverts commit 17b434e. * fixes * fixes * remove comment * Update URI for new GitHub org and GHCR repo * replace fix * replace fix * ignore maybe uninitialized error * inline comment * fix linting * Add comments * fix bt test * fix nav2 test * fix waypoint follower test * More improvements towards Jazzy migration * fix more tests * do a few more * last for the day * fix remaining BT tests * fixing last test issues * change linting image * comment out tests needing gazebo * break the cache cuz why not * fix dynamic param test * revert changes to test_bt_utils * fix dyn param * comment out turtlebot3_gazebo * test with spin_all * Update Dockerfile Signed-off-by: Steve Macenski <[email protected]> * Update .circleci/config.yml Signed-off-by: Steve Macenski <[email protected]> --------- Signed-off-by: Steve Macenski <[email protected]> Co-authored-by: Ruffin <[email protected]> Co-authored-by: Steve Macenski <[email protected]>
* Fix devcontainer * fix * . * . * . * fix * pr comments * bust cache * test with docker image * Revert "test with docker image" This reverts commit 17b434e. * fixes * fixes * remove comment * Update URI for new GitHub org and GHCR repo * replace fix * replace fix * ignore maybe uninitialized error * inline comment * fix linting * Add comments * fix bt test * fix nav2 test * fix waypoint follower test * More improvements towards Jazzy migration * fix more tests * do a few more * last for the day * fix remaining BT tests * fixing last test issues * change linting image * comment out tests needing gazebo * break the cache cuz why not * fix dynamic param test * revert changes to test_bt_utils * fix dyn param * comment out turtlebot3_gazebo * test with spin_all * Update Dockerfile Signed-off-by: Steve Macenski <[email protected]> * Update .circleci/config.yml Signed-off-by: Steve Macenski <[email protected]> --------- Signed-off-by: Steve Macenski <[email protected]> Co-authored-by: Ruffin <[email protected]> Co-authored-by: Steve Macenski <[email protected]>
* Fix devcontainer * fix * . * . * . * fix * pr comments * bust cache * test with docker image * Revert "test with docker image" This reverts commit 17b434e. * fixes * fixes * remove comment * Update URI for new GitHub org and GHCR repo * replace fix * replace fix * ignore maybe uninitialized error * inline comment * fix linting * Add comments * fix bt test * fix nav2 test * fix waypoint follower test * More improvements towards Jazzy migration * fix more tests * do a few more * last for the day * fix remaining BT tests * fixing last test issues * change linting image * comment out tests needing gazebo * break the cache cuz why not * fix dynamic param test * revert changes to test_bt_utils * fix dyn param * comment out turtlebot3_gazebo * test with spin_all * Update Dockerfile Signed-off-by: Steve Macenski <[email protected]> * Update .circleci/config.yml Signed-off-by: Steve Macenski <[email protected]> --------- Signed-off-by: Steve Macenski <[email protected]> Co-authored-by: Ruffin <[email protected]> Co-authored-by: Steve Macenski <[email protected]>
* Fix devcontainer * fix * . * . * . * fix * pr comments * bust cache * test with docker image * Revert "test with docker image" This reverts commit 17b434e. * fixes * fixes * remove comment * Update URI for new GitHub org and GHCR repo * replace fix * replace fix * ignore maybe uninitialized error * inline comment * fix linting * Add comments * fix bt test * fix nav2 test * fix waypoint follower test * More improvements towards Jazzy migration * fix more tests * do a few more * last for the day * fix remaining BT tests * fixing last test issues * change linting image * comment out tests needing gazebo * break the cache cuz why not * fix dynamic param test * revert changes to test_bt_utils * fix dyn param * comment out turtlebot3_gazebo * test with spin_all * Update Dockerfile Signed-off-by: Steve Macenski <[email protected]> * Update .circleci/config.yml Signed-off-by: Steve Macenski <[email protected]> --------- Signed-off-by: Steve Macenski <[email protected]> Co-authored-by: Ruffin <[email protected]> Co-authored-by: Steve Macenski <[email protected]>
Basic Info
Description of contribution in a few bullet points
Various fixes for Nav2 Rolling on Noble
Description of documentation updates required from your changes
Future work that may be required in bullet points
For Maintainers: