Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Feature/mw/fields2cover2 #53
base: main
Are you sure you want to change the base?
Feature/mw/fields2cover2 #53
Changes from 46 commits
c81ba02
2688106
0841d1d
4cbdcc7
06497c7
62d628a
3669e7d
a3948ec
2d48105
b85696b
99eadcf
aa2cf20
668abc4
1934a78
4767882
9c34271
4c401d0
e2d89e1
35b0ce5
c770d45
a221d67
9c1ccfb
fbbcca1
0590d7d
10758ad
1b0cb2b
363415d
9a38dc4
ed382a4
d44f82c
619b528
8486ed0
be161da
ba3a6c9
4628bf3
7649af9
3de3c4e
47d0890
54b19a5
33490ee
c30abd3
eb1488c
77d1c94
2db2ce9
d9c63ed
e14194f
6d4e370
5062803
fe41c6b
6a00d89
ffcb2e3
ec7ab9d
6499975
d2b9fd0
148c84a
01d5171
f73d34d
c4e9359
89821d3
ff68d03
36b818b
c9d4bf4
795800c
11e8e1a
51b714e
f213e2f
a4f275e
02ee9c0
8496db1
50f4758
43748b2
6539b0a
9d1ad83
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
This should not be necessary to call out each individual dependency, this is what rosdep is for
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 understand that rosdep is the way these items SHOULD be installed but rosdep was/is still?? not working for rolling on 22.04/jammy which is why I went in this direction . I know it is ugly and wrong and could be done in one step but I was taking bites at it one error at a time, the action build time getting progressively longer, now at 1h12m. This is how I actually made progress in my own environments when rosdep got broken. I use ansible to install the packages required.
There are currently no packages for ros-rolling-nav2* so this opennav_coverage with behavior tree v4 needs to build it. Iron uses behaviourtree_cpp_v3 https://github.com/ros-navigation/navigation2/blob/4e16d3c7e3cfac08c4b79841f19570c40ebbe14b/nav2_behavior_tree/package.xml#L20 so not an option.
The rosdep workaround "Attempt to fix Rolling 22.04 CI Until 24.04 Comes of Age" that ros-planning/navigation2 uses was not immediately obvious on how or if it even should be applied to the opennav_coverage repository. I assume duplicating that infra is not what you want, especially as its likely that the contents of this repository will be moved into nav2, soon after fields2cover packages correctly.
Is there is a better/faster solution where opennav_coverage can use a ci docker image of a built/installed rolling nav2 as a starting point?
In attempting to debug all this I was not able to get a local github actions environment setup. Do you use that?
NOTE: Before this is merged I also need to revert pointing this at my fork of fields2cover. I am STILL trying to help fight the build farm to get packages of fields2cover to build but we all live in different time zones and have other things going on so there is a lag between fix/apply/potentially revert that messes up this PR sometimes. See Fields2Cover/Fields2Cover#136
I will look into how to tweak the action to only run the opennav_coverage tests and not the nav2 and fields2cover tests.
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.
Related - Action ignores failed rosdep install ros-tooling/action-ros-ci#864
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.
Related - Rolling CI fails ros-tooling/action-ros-ci#844
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.
Am I correct, albeit slow to realise it if so, rolling is no longer going to be supported/function on ubuntu 22.04? Or is it just in a state of flux that has not yet been worked out?
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.
Did you try for Rolling on 24.04? That is what Rolling targets now, so it doesn't surprise me if you run into issues if you're not using a statically set rosdistro version pre-rolling update to 24.04. Instead of Jammy, use the Noble ones I think should fix that and not require specifically calling out every dependency this way. I think that's what the threads point to as well that you linked above! 😄
No, this is a permanent breakage. Its annoying and I communicated the distribution to the other core ros developers and while no fix for this time, its been noted for the future.
I haven't used the local Actions yet, no :(