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

ros2 fixed async request #838

Closed
wants to merge 1 commit into from

Conversation

dariodenardi
Copy link

In the Humble version, there is a bug: when you click 'commit' after calibration, an error appears:

!!!!!!!!!!!!!!!!!!!!
Not available
!!!!!!!!!!!!!!!!!!!!

Upon inspecting the code, the issue seems to be related to asynchronous requests:

response = self.set_left_camera_info_service.call_async(req)
rv = rv and self.check_set_camera_info(response)

The request is sent, but the second function checks the response before it has actually arrived. I believe the solution is to send both asynchronous requests and then wait for the responses.

@dariodenardi dariodenardi changed the title fixed async request ros2 fixed async request Oct 22, 2023
@mikeferguson mikeferguson self-assigned this Jan 19, 2024
@mikeferguson
Copy link
Member

Assigning myself to test this out - going to do a pass through the calibration tickets in the next two weeks

@mikeferguson mikeferguson reopened this Jan 19, 2024
@mikeferguson mikeferguson changed the base branch from humble to rolling January 19, 2024 20:46
@mikeferguson mikeferguson changed the base branch from rolling to humble January 19, 2024 20:47
@mikeferguson mikeferguson added the needs testing Currently awaiting testing on physical hardware label Jan 22, 2024
@mikeferguson
Copy link
Member

It looks like #792 actually fixed this in a different way (by switching away from async entirely). I think we should instead port that back to humble (it's already in rolling and iron)

mikeferguson pushed a commit that referenced this pull request Aug 19, 2024
Backport from #792
to fix #838

Co-authored-by: Christian Rauch <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs testing Currently awaiting testing on physical hardware ros2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants