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

Quaternion/Position order is not coherent between logger and thrift #413

Open
lrapetti opened this issue Apr 4, 2023 · 4 comments
Open

Comments

@lrapetti
Copy link
Member

lrapetti commented Apr 4, 2023

Describe the bug
With @Zweisteine96 we noticed difference in the linear/angular serialization used used for iFeel data:

  • in the thrift message we use angular/linear for the pose (see here and here), while for the velocity in the thift we use linear/angular (see here)
  • In the sensors interface implementation the order seems to be again orientation/position (see here and here)
  • In the logger, data are saved as position/orientation (see here and here)

The difference between logger and thrift has created some confusion.

This opens ups to some solutions:

  • Document property the way data are logged and transmitted
  • Change the order in the thrift (this will break backcompatibility of the dataset, even if it would be clear the order used)
  • Change the order in the logger (same as previous)
  • ... others?

Any suggestion/opinion @RiccardoGrieco @traversaro @diegoferigo ?

@traversaro
Copy link
Member

If we want to change for harmonization, I would stick to linear/angular serialization as much as possible. Recently I also investigated integrating iDynTree and pinocchio in https://github.com/ami-iit/idynfor, and also in pinocchio the serialization used is linear/angular, see https://github.com/ami-iit/idynfor/blob/97ff77e6f148cabe7d95f51ee1c67958e9c9f918/doc/theory_background.md#rigid-body-velocity .

@lrapetti
Copy link
Member Author

lrapetti commented Apr 4, 2023

If we want to change for harmonization, I would stick to linear/angular serialization as much as possible

Yess, in general I agree with this. It would actually be a very quick change in the code.

The main issue, however, is that it would break all the datasets that we logged in the past using the yarpdatdumper which is the default way to store datasets. I don't know if there is a way to have a smooth transition.

@traversaro
Copy link
Member

The main issue, however, is that it would break all the datasets that we logged in the past using the yarpdatdumper which is the default way to store datasets. I don't know if there is a way to have a smooth transition.

If we have them saved all somewhere, we could write a simple tool to convert the datasets between the old and the new version. But I do not know if doing that is worth the time/effort necessary to do that.

@lrapetti
Copy link
Member Author

lrapetti commented Apr 6, 2023

The main issue, however, is that it would break all the datasets that we logged in the past using the yarpdatdumper which is the default way to store datasets. I don't know if there is a way to have a smooth transition.

If we have them saved all somewhere, we could write a simple tool to convert the datasets between the old and the new version. But I do not know if doing that is worth the time/effort necessary to do that.

This solution is certainly feasible, but we have multiple datasets around so it might be difficult to track all the dataset changes

@traversaro traversaro transferred this issue from robotology/wearables Sep 26, 2024
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

No branches or pull requests

2 participants