robotology-legacy / mex-wholebodymodel

Matlab MEX interface to the iWholeBodyModel interface.
11 stars 9 forks source link

Avoid having different matlab files with the same name #45

Closed traversaro closed 8 years ago

traversaro commented 8 years ago

This is necessary because a random user could just add to the path all the directories in mex-wholebodymodel recursivly, and thus there would be in ambiguity on which function/script it is called, that could cause hideous bugs (ask to @claudia-lat).

I repeat: NEVER EVER EVER [1] use the same name for two different matlab files, even if they are in two different folders.

Examples of duplicates files (tick when solved):

[1] : https://youtu.be/o7MF-FHra4o?t=2m21s

traversaro commented 8 years ago

Working on https://github.com/robotology/mex-wholebodymodel/issues/30 ... the two forwardDynamics_zeroExternalForces.m are quite different.. From the return values I will assume that the one in the root folder (https://github.com/robotology/mex-wholebodymodel/blob/master/forwardDynamics_zeroExternalForces.m) is the one used by the kinetic energy test.. moving it to tests directory and changint its name.

naveenoid commented 8 years ago

capito! this came because of multiple developments on top of my original forwardDynamics. Addressing soon.

On Thu, Oct 22, 2015 at 10:50 AM, Silvio Traversaro < notifications@github.com> wrote:

This is necessary because a random user could just add to the path all the directories in mex-wholebodymodel recursivly, and thus there would be in ambiguity on which function/script it is called, that could cause hideous bugs (ask to @claudia-lat https://github.com/claudia-lat).

I repeat: NEVER EVER EVER [1] use the same name for two different matlab files, even if they are in two different folders.

Examples of duplicates files (tick when solved):

  • forwardDynamics.m (this is one is tragic because the different versions have totally different semantic meaning)
  • integrateForwardDynamics.m
  • forwardDynamics_zeroExternalForces.m

[1] : https://youtu.be/o7MF-FHra4o?t=2m21s

— Reply to this email directly or view it on GitHub https://github.com/robotology/mex-wholebodymodel/issues/45.

traversaro commented 8 years ago

Actually the one in the root is just an outdated version, the real version of forwardDynamics_zeroExternalForce is of https://github.com/robotology/mex-wholebodymodel/blob/master/forwardDynamics_zeroExternalForces.m . In any case, both version should just be calls of forwardDynamics with zero input torques and null constraint list, see https://github.com/robotology/mex-wholebodymodel/issues/42 .

DanielePucci commented 8 years ago

I think that sometimes having the same name for different files may make sense. For instance, in

https://github.com/robotology-playground/WBI-Toolbox-controllers/tree/master/controllers/torqueBalancing/robots

we use several matlab files called

gains.m

associated with each robot, which contains the gains for the torqueBalancing. This is reminiscent of what we do with the torqueBalancing.c++. Opinions?

traversaro commented 8 years ago

Agreed [1], mostly because those are more configuration file in matlab syntax than actual scripts.

There are also other situations where it may be convenient to have two function with the same signature/name, in that case I think they should be placed in different packages.

[1] :+1: (Perhaps NEVER EVER EVER can become NEVER EVER SOMETIMES :D ).

naveenoid commented 8 years ago

Isnt this addressed in https://github.com/robotology/mex-wholebodymodel/pull/49?

traversaro commented 8 years ago

Not all of them, there are still three forwardDynamics, two integrateForwardDynamics, etc etc.

gabrielenava commented 8 years ago

fixed in branch feature/torques_linear