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

Update Mavlink git repo, remove hot-patching of mavros and mavlink code #358

Conversation

amarburg
Copy link
Collaborator

Changes Made

This PR updates the current main which still builds mavros and mavlink from source .... with the understanding that it's not long for this world (Issue #324). But it un-breaks CI until that happens.

  • Clones mavlink from https://github.com/ros2-gbp/mavlink-gbp-release.git rather than mavlink/mav-gdp-release.git. The latter is out of date.
  • Removes the sed-based hot-patching of the Mavlink and Mavros code.

Associated Issues

Please provide a list of all open issues that this PR will close or contribute
toward closing.

Notably, does not fix #324

@amarburg amarburg self-assigned this Dec 12, 2024
@amarburg
Copy link
Collaborator Author

Depending on the state of #324 , this may be obsolete....

Copy link
Collaborator

@evan-palmer evan-palmer left a comment

Choose a reason for hiding this comment

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

Just need to revert the target organization in the build then this should be good to go.

.github/workflows/docker.yaml Outdated Show resolved Hide resolved
@amarburg
Copy link
Collaborator Author

I'm still mystified at how I end up with such a long trail of commits. Let me do a little rebasing and I'll push a more surgical update.

@amarburg amarburg marked this pull request as draft December 13, 2024 00:22
@amarburg amarburg force-pushed the dev/update_mavlink_git_repo branch from d734edf to 4c4d003 Compare December 13, 2024 00:34
@amarburg
Copy link
Collaborator Author

For those keeping track at home, I'm able to replicate the build error (ERROR: EOF) by upgrading docker buildx on the desktop from version 0.17.x to 'v0.19.2 1fc5647' (the latest version available for 24.04).

No solution yet...

@amarburg amarburg marked this pull request as ready for review December 13, 2024 01:39
@amarburg
Copy link
Collaborator Author

Appears to be an upstream change in the parser for docker-buildx v0.19.x (possibly related to the Release note for v0.19.1, though v0.19.1/v0.19.2 do not fix it).

These latest versions of buildx don't like the syntax *.cache-to= to unset the cache-to attribute. Trying various combinations of [], '' and "" didn't fix it.

As a hack, pin buildx to v0.18.0 in the Github workflow.

Added the fix to this PR rather than starting another because I'm not (much of) a sadist. Simple to cherry-pick the change into #324 if we want to abandon this PR.

@evan-palmer
Copy link
Collaborator

Thanks for working on this. I don't see any problem with pinning the version as a quick fix. Would you be willing to submit an issue so that we don't forget about that?

@evan-palmer evan-palmer merged commit ab705d2 into Robotic-Decision-Making-Lab:main Dec 13, 2024
4 checks passed
@evan-palmer evan-palmer added the backport-jazzy Backport this PR to jazzy label Dec 13, 2024
mergify bot pushed a commit that referenced this pull request Dec 13, 2024
…de (#358)

* Remove the sed-munging on mavlink and mavros

* Pin docker buildx to v0.18.0 in Github workflow

(cherry picked from commit ab705d2)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-jazzy Backport this PR to jazzy
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants