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 calibration (backport #1017) #1023

Merged
merged 1 commit into from
Jun 17, 2024
Merged

Fix calibration (backport #1017) #1023

merged 1 commit into from
Jun 17, 2024

Conversation

mergify[bot]
Copy link

@mergify mergify bot commented Jun 17, 2024

It seems that for certain robots the calibration extraction and correction fails. This PR aims at fixing this.

ToDo:

  • Add a test showing that some calibrations don't get corrected correctly
  • Fix the code (We have a fix already, locally, I just need to clean it up)
  • Add more documentation on the algorithm (It took me way too long to understand my old code...)
  • Also backport things to ROS 1 (Fix calibration Universal_Robots_ROS_Driver#704)
    This is an automatic backport of pull request Fix calibration #1017 done by Mergify.

* Make calibration tests parametrizable

* Add two more real-world configurations to tests

Note: the second one fails with our current implementation

* Fix calibration correction

There was an error from using std::atan instead of std::atan2.
In most cases that seemed to work fine, but with certain calibrations the
calculated angle could end up in another quadrant, so atan was returning
the wrong angle.

We renamed a lot of variables and changed some of the docstrings in order
to hopefully make things more understandable in the future.

* Add robot model to calibration tests

* Add documentation to calibration algorithm

* Add a comment on test suite parametrization

(cherry picked from commit 557b57e)

# Conflicts:
#	ur_calibration/doc/index.rst
Copy link
Author

mergify bot commented Jun 17, 2024

Cherry-pick of 557b57e has failed:

On branch mergify/bp/iron/pr-1017
Your branch is up to date with 'origin/iron'.

You are currently cherry-picking commit 557b57e.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   .pre-commit-config.yaml
	new file:   ur_calibration/doc/algorithm.rst
	new file:   ur_calibration/doc/calibration_example.png
	new file:   ur_calibration/doc/calibration_example_corrected.png
	new file:   ur_calibration/doc/calibration_uncorrected.png
	new file:   ur_calibration/doc/conf.py
	new file:   ur_calibration/doc/usage.rst
	modified:   ur_calibration/src/calibration.cpp
	modified:   ur_calibration/test/calibration_test.cpp

Unmerged paths:
  (use "git add/rm <file>..." as appropriate to mark resolution)
	deleted by us:   ur_calibration/doc/index.rst

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

@fmauch fmauch removed the conflicts label Jun 17, 2024
Copy link
Collaborator

@fmauch fmauch left a comment

Choose a reason for hiding this comment

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

This seems fine. Conflict resolving automatically produced the correct result.

@fmauch fmauch merged commit 94a07be into iron Jun 17, 2024
7 checks passed
@fmauch fmauch deleted the mergify/bp/iron/pr-1017 branch June 17, 2024 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant