Idyntree: Update yarprobotstate publisher joints check

Created on 10 Apr 2019  路  10Comments  路  Source: robotology/idyntree

Currently, the yarprobotstatepublisher checks for the exact match between the number of joints present in the urdf model passed to --model parameter and the number of joints present in the /joints_states topic. Ideally, it is better to update this check to ensure if the /joint_states topic has at least the same number of joints as that of the urdf model. This avoids the problem where the urdf model used will generally have around 32 joints without considering the hand joints, whereas the real robot streams the /joint_states topic along with the hand joints through the configuration file

Most helpful comment

Yes, please, go on to modify the branch used in that PR.

All 10 comments

@traversaro Can I proceed to change this in the branch with the PR https://github.com/robotology/idyntree/pull/513

Yes, please, go on to modify the branch used in that PR.

@traversaro I do not have push access to upstream repo

Waiting for @traversaro you can open a PR that targets that branch

PR merged, closing this issue.

Hi @Yeshasvitvs , we tipically close issues when the problem is solved at least in the devel branch, I think we should keep the issue open until https://github.com/robotology/idyntree/pull/513 is merged.

We should allow also the fact that the message has less joint that the actual number of joint of the model, in this case, just the model for which we have the message would be updated.
I think @Yeshasvitvs has already implemented a preliminary version of those changes and he may add them in the PR.

I think @Yeshasvitvs has already implemented a preliminary version of those changes and he may add them in the PR.

First of all, thanks for providing those fixes. However, given that the PR https://github.com/robotology/idyntree/pull/513 has been open for quite a long time and already contains a lot of different features, I would suggest to get it merged without adding new feature to it. Once it is merged, we can add new features with a new PR.

Was this page helpful?
0 / 5 - 0 ratings