-
Notifications
You must be signed in to change notification settings - Fork 60
Migrate to Lyrical (rolling) #95
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
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.
self review
We need to (unfortunately) build Nav2 or use the nav2_docker Rolling Nightly containers for the testing job in order to have Nav2 installed & have a workspace you can source with its packages. |
* update msg for Kilted * [opennav_coverage_bt] migrate BT xml files * migrate coverage_navigator * rm error_msg from ComputeCoveragePath * use newer rostooling docker images, run test for Kilted * run linter on Kilted * use base image
due to wrong includes and to methods that moved from nav2_util to nav2 namespace
to avoid clashes between navigators
The two options would be to Or, we change the base container to use Nav2 Rolling's Nightly, create a new workspace that sources from that, and build (much faster, but more github actions coding) |
2388748
to
6db223b
Compare
686f20c
to
d5756d6
Compare
This is just a migration error for Rolling t hat something needs to be changed properly to match
|
Missed migrating the node in the tests to nav2::LifecycleNode |
Now I don't get it. Locally the
On the CI, the test seems to fail because the node doesn't shutdown
I am not sure about the error |
I think that's the issue, its trying to find it as a dependency and failing so it quits (I think). I think that exception being thrown is why you see the other error about lifecycle because it is no longer in a recoverable state (either crashed or in the |
It looks like it is building (I see |
Add Nav2 behavior tree -- I think you're just missing the library of BT nodes perhaps. This is a good step forward though |
finally passed 🎉 |
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.
Thanks so much for your diligence, I know this was a pain! Merging 🥳
Migrate the code to rolling:
declare_or_get_parameter
action_server_result_timeout
(Includes changes from #94, since current
main
is for Jazzy)