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

Update iCubGenova09 to the use of the latest NWS/NWC layers #319

Merged
merged 4 commits into from
Dec 23, 2021

Conversation

pattacini
Copy link
Member

@pattacini
Copy link
Member Author

iCubGenova09 has been successfully tested ✔️
Thanks to @randaz81 and @S-Dafarra!

I'll keep this open to receive:

  • from @davidetome the mods to include also the files to start up single limbs
  • from @S-Dafarra the mods to include the latest changes to mirror AMI's fork

@davidetome
Copy link
Contributor

I've added the missing files as requested, please @randaz81 take a look to them 👍🏻

cc @pattacini @S-Dafarra

@pattacini
Copy link
Member Author

Thanks @davidetome 👍🏻

@S-Dafarra
Copy link
Contributor

Thanks @davidetome for adding the missing files. While reading the changes, I realized that having multiple calibrator files for the arms might be risky. Right now, if we need to modify some calibration parameters of some fingers, we tend to modify the file calibrators/left_arm-calib.xml for example. With the other calibration files, we should duplicate the same modification to another file. Hence, the risk of forgetting or human error increases a lot.

Clearly, we had the same problem before, but at this point, I would exploit this occasion to avoid this kind of issue. Sorry for noticing it only after asking to resuscitate those files.

In short, I would kindly ask to delete the duplicate calibration files and the xml files that were needing them.

@davidetome
Copy link
Contributor

Thanks @davidetome for adding the missing files. While reading the changes, I realized that having multiple calibrator files for the arms might be risky. Right now, if we need to modify some calibration parameters of some fingers, we tend to modify the file calibrators/left_arm-calib.xml for example. With the other calibration files, we should duplicate the same modification to another file. Hence, the risk of forgetting or human error increases a lot.

Clearly, we had the same problem before, but at this point, I would exploit this occasion to avoid this kind of issue. Sorry for noticing it only after asking to resuscitate those files.

In short, I would kindly ask to delete the duplicate calibration files and the xml files that were needing them.

Ciao @S-Dafarra , I think you refer to forearms calibrators.
Honestly, I simply ported all the files that were present in iCubGenova09 because I really don't know if you use it all or part of it.
So I ported also the forearm calibrators called by some XML files you had before.

Anyway, if you do not need them I can surely remove those files 👍🏻
Give me light green to proceed!

@S-Dafarra
Copy link
Contributor

Ciao @S-Dafarra , I think you refer to forearms calibrators. Honestly, I simply ported all the files that were present in iCubGenova09 because I really don't know if you use it all or part of it. So I ported also the forearm calibrators called by some XML files you had before.

Anyway, if you do not need them I can surely remove those files 👍🏻 Give me light green to proceed!

Yes please! Yes, those files were already there, but I think this is a nice occasion to avoid having the duplication anymore. I realized the problem only this morning when I actually needed to change some calibration parameters and initially I ended up in the wrong calibration file

@davidetome
Copy link
Contributor

@S-Dafarra , done! 👍🏻

@S-Dafarra
Copy link
Contributor

@S-Dafarra , done! 👍🏻

Thanks!

@pattacini
Copy link
Member Author

Hi @S-Dafarra
Do you have other commits to push to this branch?

@S-Dafarra
Copy link
Contributor

Actually I rebased this branch on top of devel_iCubGenova09 altough I haven't tested the single parts xmls. I can open the PR myself later if you don't mind, since those would be the files that were tested

@pattacini
Copy link
Member Author

Actually I rebased this branch on top of devel_iCubGenova09 altough I haven't tested the single parts xmls. I can open the PR myself later if you don't mind, since those would be the files that were tested

Fantastic!
So, I'll merge this PR straight away, waiting possibly for your further contributions.

@pattacini pattacini marked this pull request as ready for review December 23, 2021 17:44
@pattacini pattacini merged commit 5bc2a39 into devel Dec 23, 2021
@pattacini pattacini deleted the iCubGenova09nws branch December 23, 2021 17:44
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