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 type 14 update on calibration value parser #885

Merged
merged 1 commit into from
Jun 9, 2023

Conversation

MSECode
Copy link
Contributor

@MSECode MSECode commented Jun 5, 2023

This PR brings an update to the current version of calibration type 14. Specifically the following things have changed:

  • FAP calibration data are not passed anymore only through the CALIBRATION group in the POS service configuration file but the final correct values should be defined at parameters 3, 4 and 5 of the calibration configuration file as defined in the documentation. Moreover in this file offset and rotation values should be passed in icubDegrees.
  • Anyway, CALIBRATION group in MC and POS is still kept for debugging reasons. Values in calibrators will override the one configured initially by the POS service.
  • Then, with this update the calibration will do the following steps:
    • calibrate the FAP absolute encoder
    • bring the motor pulley to the hard stop
  • New data structure have been introduced to simplify the conversion between eoThePOS datatype and icubDegrees to be used by both icub-main and icub-firmware
  • Add check for calib.params.type14.invertdirection value, which must be a boolean value
  • Further specification can be found in the documentation

@CLAassistant
Copy link

CLAassistant commented Jun 5, 2023

CLA assistant check
All committers have signed the CLA.

@pattacini pattacini marked this pull request as draft June 5, 2023 12:58
Copy link
Member

@pattacini pattacini left a comment

Choose a reason for hiding this comment

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

Hi @MSECode

I guess this is a sister PR of robotology/icub-firmware-shared#83.

You need to:

  • Handle the versioning correctly (you may ask @valegagge).
  • Sign up for the CLA here below.
  • Keep this PR in draft until the one in icub-firmware-shared gets merged in order to make CI work as intended.

Copy link
Member

@pattacini pattacini left a comment

Choose a reason for hiding this comment

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

Hi @MSECode

I guess this is a sister PR of robotology/icub-firmware-shared#83.

You need to:

  • Handle the versioning correctly (you may ask @valegagge).
  • Sign up for the CLA here below.
  • Keep this PR in draft until the one in icub-firmware-shared gets merged in order to make CI work as intended.

@pattacini
Copy link
Member

Also, please fill in the body of the PR.

@MSECode MSECode marked this pull request as ready for review June 5, 2023 13:31
@MSECode MSECode marked this pull request as draft June 5, 2023 13:40
Copy link
Contributor

@marcoaccame marcoaccame left a comment

Choose a reason for hiding this comment

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

The PR is OK @MSECode, thanks.

@pattacini
Copy link
Member

We're still missing the version check for icub-firmware-shared.

Copy link
Member

@pattacini pattacini left a comment

Choose a reason for hiding this comment

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

Thanks @MSECode 👍🏻
Once the upstream PR is merged, I'll put this in ready for review to activate the CI.

Remove change rotation to enum and add check of double value
Update check of rotation param using enum type
Fix error in if clause
Update required icub_firmware_shared_VERSION in conf
Add check on invertdirection value (must be bolean)
@MSECode MSECode marked this pull request as ready for review June 9, 2023 12:27
@pattacini pattacini merged commit 4bcb6d1 into robotology:devel Jun 9, 2023
@MSECode MSECode deleted the feature/calib14 branch June 9, 2023 13:06
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.

4 participants