Skip to content

Conversation

@b-bremer
Copy link
Contributor

@b-bremer b-bremer commented Dec 2, 2024

Calling correctChain() in the constructor doesn't add much overhead, but improves usability.

E.g. the chain is already correct when calling "calcForwardKinematics".

Calling correctChain() in the constructor doesn't add much overhead, but improves usability.

E.g. the chain is already correct when calling "calcForwardKinematics".
@urfeex
Copy link
Member

urfeex commented Dec 2, 2024

I would expect the unittests to fail with that change.

Edit: no, it doesn't fail the tests, but it makes them useless, as we would compare the result with the result (or more specifically, we would verify that calling correct Chain twice doesn't change anything.)

@urrsk
Copy link
Member

urrsk commented Jan 20, 2025

@b-bremer Thanks for your suggestion.
May I ask what your motivation is to add this?
The correctChain() function does not change the performance for the calcForwardKinematics() function but shrink the 4 DoF DH notation to a 6DoF pose notation. This is needed to place the graphics correct in the URDF file.

@b-bremer
Copy link
Contributor Author

Sorry, I forgot to close the pull request. I need to spend more time on a clean solution.

I was under the impression that every caller required correctChain() anyways.

@b-bremer b-bremer closed this Jan 20, 2025
@urfeex
Copy link
Member

urfeex commented Jan 20, 2025

@b-bremer coming back to this: What is your actual motivation for that?

E.g. the chain is already correct when calling "calcForwardKinematics".

Whether or not the chain is "corrected" is irrelevant in the context of calcForwardKinematics it should return the same result in both cases (which is confirmed by the aforementioned test.)

The naming is a bit misleading there, but the "correction" that we do is not applying the calibration, but merely change the geometric representation as described in https://github.com/UniversalRobots/Universal_Robots_ROS2_Driver/blob/main/ur_calibration/doc/algorithm.rst

Your statement from above is definitively true for getChain, getSimplified and yoYaml, though. The result of those will be significantly different depending on whether correctChain had been called.

One option to maintain testability but also have a class that will always use the corrected model would be to separate both concerns into separate classes, e.g. we could have a RobotModel and a ShrunkRobotModel class instead of the Calibration class. The ShrunkRobotModel would inherit from RobotModel and add the correctChain function in a private manor as you suggested.

While writing this, this PR got closed, so my thoughts are merely for anybody else who would require that functionality :-)

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.

3 participants