space-ros / docker

Docker images to facilitate Docker-based development.
21 stars 32 forks source link

space_nav2 build failing due to upstream feature addition #140

Closed mkhansenbot closed 5 months ago

mkhansenbot commented 7 months ago

The space_nav2 build started failing https://github.com/space-ros/docker/actions/runs/8013829132/job/21891470012

The error appears to be due to this adding of the nav2_graceful_controller and corresponding test that was merged into upstream Nav2: https://github.com/ros-planning/navigation2/blob/2e491b18328ede064c75bfa024f57c7e49578eba/nav2_graceful_controller/test/test_graceful_controller.cpp#L817

https://github.com/ros-planning/navigation2/pull/4119

I think this can be fixed by submitting a PR to the upstream nav2 repo branches (main, humble) to typecast the comparison to a unsigned long from an int. However, we may need to make that change in multiple places.

Ideally we should see if we can get the nav2 repo to enable the same build flags we're using, so these get caught before they're merged upstream

xfiderek commented 7 months ago

hi @mkhansenbot. could not we follow the same pattern that we have for main repo, i.e. pin versions of repositories in navigation2.repos (+ add navigation2 there)?

I know this is not a solution to the problem, but maybe it would help us to maintain stability of demos. Maybe we can consider it after fixing this particular issue.

mkhansenbot commented 5 months ago

@xfiderek - It looks like this was fixed upstream by this PR: https://github.com/ros-planning/navigation2/pull/4216

However, I do agree we should pin the Navigation2 repo version number going forward, I'll file a new issue for that and close this one

xfiderek commented 5 months ago

@xfiderek - It looks like this was fixed upstream by this PR: ros-planning/navigation2#4216

However, I do agree we should pin the Navigation2 repo version number going forward, I'll file a new issue for that and close this one

hi @mkhansenbot. I've noticed the same today :). see the reply in the new issue.