robotology-legacy / yarp-wholebodyinterface

Implementation of the wholeBodyInterface for YARP robots.
1 stars 3 forks source link

Reordering world-to-base position estimation #31

Closed naveenoid closed 9 years ago

naveenoid commented 9 years ago

Referencing https://github.com/robotology/codyco-modules/issues/58#issuecomment-88407973 Its better to organise the output of world-to-base position estimate to match Frame class constructor. @jeljaik @rlober @traversaro do you want to add anything to this wishlist?

traversaro commented 9 years ago

cc @francesco-romano

francesco-romano commented 9 years ago

You already know my opinion :p 1) find out a good way to strongly type the functions signature (aka remove the double*).

Because this is not possible in short (and medium) time to me it would be enough to make an utility function (yeah... plain old C function, just to stress this must remain temporary) which convert the 12 elements double* vector output by the base estimation into a Frame object, i.e. void frameFromSerialization(double* serialization, Frame &outputFrame) and its companion void serializationFromFrame(const Frame &outputFrame, double* outputSerialization) and document in the doxygen to use these functions.

traversaro commented 9 years ago

:+1: to the @francesco-romano solution of having the helper function to explicitly state the serialization rule, we can still anyway use a 16-elements serialization of the floating base position just to avoid confusing the users.

traversaro commented 9 years ago

It would also be nice to have a proper structure to encode estimates sizes, as we do for sensors https://github.com/robotology-playground/wholebodyinterface/blob/master/src/wbiConstants.cpp (so we avoid hardcoding them around).

rlober commented 9 years ago

@naveenoid From a readability standpoint I think it is nice that the position and rotation components are arranged in a logical order, this being said, as is, there is no way to construct a Frame without adding/modifying functions/constructors, which may be junked in the long term. Another option would be to just return the values in a 16x1 vector based on the 4x4 rototranslation matrix ordering (that the Frame constructor could take directly).

@francesco-romano @traversaro Just to cross post: in robotology/codyco-modules#58 @jeljaik and I brought up a question about the Frame constructor (double* d). I didn't want to forget about it.

francesco-romano commented 9 years ago

@rlober yeah I noticed it too when I submitted my PR this evening. @jeljaik already fixed it.

traversaro commented 9 years ago

@rlober I would go for your proposal of serializing the 4x4 world_H_base homogeneous transform matrix as a 16x1 vector using row major ordering, to be consistent with the rest of matrix serializations in the wbi and to simplify the use of the double * constructor of Frame. You can see in @francesco-romano pull request ( https://github.com/robotology-playground/wholebodyinterface/pull/10/files ) that he already implemented the serialization helpers using this logic. Now we have:

This transition would be desirable to be completed tomorrow at 13 CEST, to avoid further delay in codyco demo preparation.

naveenoid commented 9 years ago

To be closed by @francesco-romano after serialisation becomes a part of master.

naveenoid commented 9 years ago

I will take care of wholeBodyInterfaceIcubTest to reflect the changes

jeljaik commented 9 years ago

Yes please :+1:

Jorhabib Eljaik Gómez,Ph.D. Student - Istituto Italiano di Tecnologia Dept. of Robotics, Brain and Cognitive Sciences Cognitive Humanoids Laboratory e-mail: jorhabib.eljaik@iit.it Cel: +39 388 352 33 85 Tel: +39 010 71781 420 Fax: +39 010 7170817

On Thu, Apr 2, 2015 at 9:58 AM, Naveen Kuppuswamy notifications@github.com wrote:

I will take care of wholeBodyInterfaceIcubTest to reflect the changes

— Reply to this email directly or view it on GitHub https://github.com/robotology-playground/yarp-wholebodyinterface/issues/31#issuecomment-88807640 .

naveenoid commented 9 years ago

updated test in codyco modules. bugfix on serialisation : b3bb92584d9c6303582e248fde8bb03a0a41e4fc

@rlober i think it should be ok for you now