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

Calibration check optional #65

Merged
merged 2 commits into from
May 25, 2021

Conversation

fmauch
Copy link
Collaborator

@fmauch fmauch commented May 10, 2021

As not all downstream packages support extracting calibration information from the robot we make this check optional. This moves calling the calibration check towards the application layer.

@urmahp This should remove the error output from the ISAAC driver.

@urrsk FYI, as we've been talking about that.

As not all downstream packages support extracting calibration information
from the robot we make this check optional. This moves calling the calibration
check towards the application layer.
@urmahp
Copy link
Collaborator

urmahp commented May 12, 2021

When testing this with the ROS driver, it is working very well.

When I test it with the Isaac driver, i get an error saying that the call to the constructor without the calibration checksum is ambiguous. I suspect that it might be because the constructor with the checksum, has a default value for the calibration checksum. But then I would expect it to happen for the ROS driver as well. I will try to investigate this further on friday.

@fmauch
Copy link
Collaborator Author

fmauch commented May 12, 2021

When testing this with the ROS driver, it is working very well.

When I test it with the Isaac driver, i get an error saying that the call to the constructor without the calibration checksum is ambiguous. I suspect that it might be because the constructor with the checksum, has a default value for the calibration checksum. But then I would expect it to happen for the ROS driver as well. I will try to investigate this further on friday.

The ROS driver still uses the constructor with the calibration checksum for backwards compatibility, that's why it isn't happening there.

It would probably make sense to remove the default value, as it isn't needed anymore with that.

@urmahp
Copy link
Collaborator

urmahp commented May 14, 2021

The ROS driver still uses the constructor with the calibration checksum for backwards compatibility, that's why it isn't happening there.

When testing it locally I updated the ROS driver to use the constructor without the checksum, but I will investigate it further today.

@urmahp
Copy link
Collaborator

urmahp commented May 14, 2021

I found the issue, it is in the Isaac driver and not in the library.

Currently NULL is inserted as the tool communication setup parameter in the constructor in the Isaac driver. This means that this constructor and this constructor are ambiguous, because NULL could both be the calibration string or the tool communication setup. Instead of NULL I updated it to use an empty ToolCommSetup pointer instead, which fixes the issue.

So the update removes the calibration error, and it is working with both the Isaac driver and the ROS driver.

@fmauch
Copy link
Collaborator Author

fmauch commented May 25, 2021

thanks for testing @urmahp

@fmauch fmauch merged commit d4e6a89 into UniversalRobots:master May 25, 2021
@fmauch fmauch deleted the calibration_check_optional branch May 25, 2021 19:15
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.

None yet

2 participants