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

Fix CLI load_controller verb #585

Merged
merged 6 commits into from
Sep 12, 2024
Merged

Fix CLI load_controller verb #585

merged 6 commits into from
Sep 12, 2024

Conversation

christophfroehlich
Copy link
Contributor

This fixes the CLI commands of example_1 with the new option introduced with ros-controls/ros2_control#1703

Fixes #555

@christophfroehlich christophfroehlich marked this pull request as ready for review September 12, 2024 08:49
@saikishor
Copy link
Member

@christophfroehlich should we wait for the binary release?

@christophfroehlich
Copy link
Contributor Author

@christophfroehlich should we wait for the binary release?

No, because the master version is neither working with the current release, or am I wrong?

@saikishor
Copy link
Member

@christophfroehlich should we wait for the binary release?

No, because the master version is neither working with the current release, or am I wrong?

Well in theory, the PR: ros-controls/ros2_control#1703 is merged already. The current failing builds are both binary builds (main and testing)

@christophfroehlich
Copy link
Contributor Author

But I mean the 1703 is not included in the 4.17 release. And the current version of load_controller from the head of ros2_control_demos is not working with 4.17

@saikishor
Copy link
Member

saikishor commented Sep 12, 2024

But I mean the 1703 is not included in the 4.17 release. And the current version of load_controller from the head of ros2_control_demos is not working with 4.17

Shall we wait for the release then?

Copy link
Member

@saikishor saikishor left a comment

Choose a reason for hiding this comment

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

LGTM

@christophfroehlich
Copy link
Contributor Author

The part of the demo is not working at all without merging this PR 😬 after merge, at least it works for users with source builds. And I can live with red CI of the binary builds.

@christophfroehlich christophfroehlich merged commit 297ca13 into master Sep 12, 2024
12 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Example 1: load_controller cli does not work
2 participants