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

fixed admittance_controller/AdmittanceController::read_state_from_hardware() #808

Conversation

tpoignonec
Copy link

@tpoignonec tpoignonec commented Oct 30, 2023

Issue: only one state interface is read, but at least position AND velocity should be read.

Cause: "else if" statements used instead of "if" in AdmittanceController::read_state_from_hardware().

Solution: see edits.

@tpoignonec tpoignonec marked this pull request as ready for review October 30, 2023 12:18
@tpoignonec
Copy link
Author

N.B., actually this seems to be common to all branches (e.g., see main branch).

@bmagyar bmagyar changed the base branch from humble to master October 30, 2023 21:58
@bmagyar bmagyar changed the base branch from master to humble October 30, 2023 21:58
@bmagyar
Copy link
Member

bmagyar commented Oct 30, 2023

I tried simply retargeting the PR but it isn't that simple unfortunately. Please rebase this PR to the master branch and we can backport it to humble.

@bmagyar
Copy link
Member

bmagyar commented Oct 30, 2023

Additionally, if you could add a test that reproduces the issue that'd be great

@tpoignonec
Copy link
Author

OK thanks, will do.

@tpoignonec
Copy link
Author

tpoignonec commented Oct 30, 2023

Actually, we can probably just close this PR, I hadn't realized the controller was made for position interface specifically.
After investigation, my issue was more due to the fact that I tried to modify the code to do something it wasn't designed to do...

@bmagyar
If I find a test scenario, I'll reopen + rebase on main as you requested.
Sorry for the inconvenience.

@tpoignonec tpoignonec closed this Oct 30, 2023
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.

2 participants