robotology-legacy / codyco-modules

Whole-body Compliant Dynamical Contacts in Cognitive Humanoids
www.codyco.eu
Other
19 stars 13 forks source link

[wholeBodyInterface] General Redesign #38

Closed traversaro closed 7 years ago

traversaro commented 10 years ago

A lot of aspects of the wholeBodyInterface need a redesign: this issue works as an entry point to all other issues related to wholeBodyInterface.

francesco-romano commented 10 years ago

I would like to add another point in this issue. I think that all the initializations should be performed in the init function. This should also include object creations. One reason to do this in the init method is that in C++ constructors cannot fail if not by throwing an exception (and we do not like exceptions, do we?). I'm not against exceptions IMHO they should be confined and used sparsely. To recap: I think the constructors can take arguments, but they should be just shortcuts to frequently used setters. All the initialization logic should be performed in the init method. (and undo in its companion termination method).

lornat75 commented 10 years ago

+1

From: Francesco Romano [mailto:notifications@github.com] Sent: mercoledì 23 aprile 2014 10:33 To: robotology/codyco Subject: Re: [codyco] [wholeBodyInterface] General Redesign (#38)

I would like to add another point in this issue. I think that all the initializations should be performed in the init function. This should also include object creations. One reason to do this in the init method is that in C++ constructors cannot fail if not by throwing an exception (and we do not like exceptions, do we?). I'm not against exceptions IMHO they should be confined and used sparsely. To recap: I think the constructors can take arguments, but they should be just shortcuts to frequently used setters. All the initialization logic should be performed in the init method. (and undo in its companion termination method).

— Reply to this email directly or view it on GitHubhttps://github.com/robotology/codyco/issues/38#issuecomment-41136890.

traversaro commented 10 years ago

Stupid issue to remember: wbi as a header directory is probably not very descriptive and can be confusing.

francesco-romano commented 10 years ago

I want to add another trivial point: computeH should become computeRotoTranslation.

andreadelprete commented 10 years ago

I've recently been told that "rototranslation" is only known in Italy. A better name could be "homogeneous matrix" or "transformation matrix", even if I think they are not as explicit as "rototranslation matrix".

francesco-romano commented 10 years ago

@andreadelprete I forgot! We already discussed about this. I like computeTransformationMatrix

hu-yue commented 10 years ago

In the documentation it would be better to specify that the parameters like double J, double dJdq, double *M etc of the functions in icubWholeBodyModel (and maybe also somewhere else?) have to be instantiated before calling the function, otherwise it can be misunderstood that the functions take care of the instantiation. Or anyway specify something regarding this.

francesco-romano commented 10 years ago

@hu-yue for sure we can be more explicit about it. But, consider that if a function working on a vector data takes a double*, then it can't allocate memory for it. The doubt can exist only if the function takes double** or if the function returns a double* as return value.

hu-yue commented 10 years ago

Then please put it for people like me =) it would help! Il 23/lug/2014 13:27 "Francesco Romano" notifications@github.com ha scritto:

@hu-yue https://github.com/hu-yue for sure we can be more explicit about it. But, consider that if a function working on a vector data takes a double, then it can't allocate memory for it. The doubt can exist only if the function takes double*.

— Reply to this email directly or view it on GitHub https://github.com/robotology/codyco-modules/issues/38#issuecomment-49861771 .

traversaro commented 10 years ago

After nearly an year after the original issue #6, we are starting to have a initial working version of the new wbi. For now I incorporated the changes discussed in #6, #63, #18 (and #76).

Given that the required modifications are spread all across the codyco software, I create a new_wbi_ID branch on all affected repos (wholebodyinterface,yarp-wholebodyinterface,codyco-modules and codyco-superbuild).

As a on road test for the new system I am going to try to run wholeBodyDynamicsTree on the iCubHeidelberg01, using the new URDF models extracted directly from the CAD.

For all other modules in codyco-modules I will probably require some help from the authors of the modules. I tryed to port most of them, but for all of them I found points where I was enable to complete the porting. I will push non-working stubs of the ported modules this evening.

For the affected libraries I bumped the version number from 0.0.1 (old id system) to 0.0.2 (new id system).

traversaro commented 10 years ago

Discussion on 0.2.0 is moved to https://github.com/robotology/codyco-modules/issues/6 , this issue will remain for discussing the general redesign of the wbi.

traversaro commented 7 years ago

I think that this issue can be closed, as the idea is to move away from the wholeBodyInterface towards a direct use of ControlBoardRemapper, iDynTree::KinDynComputations and future YARP interface to read inertial sensors https://github.com/robotology/yarp/issues/1273 .