-
Notifications
You must be signed in to change notification settings - Fork 28
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
Add secondary calibration #164
Conversation
Thank you! This indeed proved to be very useful during the preparation of the RAI demo. While using it we noticed that if we calibrate twice, the result is screwed up. I guess it may be worth resetting the previously computed calibration before setting a new one. The second thing I was thinking, was to add the possibility of calibrating a subtree starting from a link. The idea is that by calling an RPC method like
the chest, the arms, and the head are calibrated all together. The idea would be to iteratively add to the calibration list the children of the starting link, and all the children of the children and so on. I am not quite sure iDynTree has a ready to use method for this, but I guess that the entry points may be the method |
fixed with 1bef99e |
1bef99e
to
03a8c5b
Compare
I discovered that there is currently a problem when using the retargeting on iCub. In fact we are using fake link that are rigidly attached to the model, and those links are not present in the reduced model that is generated (with e.g. this is the reduced model for the chain
Clearly we can not detect the links to calibrate neither reading the links in the reduced model, or the frames. So for the moment this option is useless using models with "fake links". |
The positive side is that actually this options makes the fake links almost useless. In fact, we may just use the regular links of the robot and perform a whole-body calibration once at the beginning so that we have the calibration matrix between the robot and the human link orientation. |
Sorry, I did not understand 🤔 The reduced model seems right. Initially, I found weird that |
I am not saying it is wrong, but it is not exactly what we were looking for. It doesn't have any information regarding the link (fake links) that are present along the chain and we want to calibrate. |
Ah ok, I see! |
Codacy review is still failing (https://app.codacy.com/manual/diegoferigo/human-dynamics-estimation/pullRequest?prid=4567113) claming that the @diegoferigo Since this is something we didn't succede to address the problem for long time (#142 (comment), #137 (comment), #133 (comment), #121 (comment)), at this point I would remove the check for the time being. |
@lrapetti Are you sure your codacy points to robotology and not your fork? I tried to enable devel from my account, can you check if it's ok now? |
@diegoferigo still not running, please try if you can launch it manually. |
@lrapetti probably we had to wait a bit more. I just checked and this PR has been successfully analysed. |
no I'm quite sure it was not running when triggered manually, it started again triggered by new changes. |
3cb8d43
to
b484dfe
Compare
3f722be
to
39a3a26
Compare
I have squashed the commits, waiting for the checks before merging. |
39a3a26
to
9dba8e1
Compare
@diegoferigo I would ignore the issue raised by Codacy (https://app.codacy.com/manual/diegoferigo/human-dynamics-estimation/pullRequest?prid=4567113) but I don't have the rights, if you agree could you please do that so we got green flag ✔️? |
Done, triggered a new analysis, waiting the update of the check. |
Still, it is showing as |
I am afraid the issues are in "New duplicates". However, what about removing Codacy check, or at least making it optional to merge a PR? In my experience the amount of false positives and hiccups of Codacy it non-negligible. |
I agree on making it optional, if this means that we get the green flag even when its failing. |
I just set only Travis as required check, let's try again :) |
What I'm not sure is if the check mark of the PR will become green even though Codacy is not a required check |
Unfortunately seems it is not like that. Anyway I will procede merging since this PR has been ready for a while. In general I think having the green flag helps a lot when searching for deprecation points, I don't know if it would make sense indeed to remove Codacy. |
Adding secondary calibration as described in #158.
The PR still need to be cleaned, and the rpc interfaces changed accordingly to what described in #159 (unfortunately this is a duplicate of the same changes done in the feature branch, merging the branch will need to be handled correctly)