search

robotology / cer

Contains SW specific to the R1 robots
GNU General Public License v2.0
10 stars 13 forks source link

CER depends on iCub repo #2

Closed barbalberto closed 8 years ago

barbalberto commented 8 years ago

Just a question / dicussion:

Shall CER repo depends on iCub's repo? Right now there are some include files that throws in the dependency like: #include <iCub/iKin/iKinFwd.h>

Anyway, aside from this, the robot will use the iCub devices like the embObj* so the dependency is quite strong right now.

There was an idea to move the icubmod on a separated repo, in that case shall we move also some other reusable library like iKin?

pattacini commented 8 years ago

Moving iKin outside icub-main is not an easy job, hence for the time being the answer is ... Yes

lornat75 commented 8 years ago

The embObj dependency is dynamic right?

For iKin, I think we should take the occasion to start experimenting with YCM for CER and icub-main so that libraries get their own life (or we reduce iCub-main until it becomes a very thin collection of libraries).

@drdanz

pattacini commented 8 years ago

@lornat75

I deem this is not the right period to introduce the noise in our eco-system that such a change will bring about, since iKin is a widespread dependency for a huge amount of software. Could we wait after the first year demo?

Making iKin standalone is also one of my personal goal, so don't take this opinion as an a priori objection. It's just a matter of timing.

In the meantime, experimenting with YCM could be perfectly doable with the caveat not to touch the icub-main master.

drdanz commented 8 years ago

With "experimenting with YCM" you mean for the icub-main superbuild or as a dependency?

barbalberto commented 8 years ago

BTW, I updated all and I get the following include error: /usr/local/src/robot/cer_WS/cer/libraries/cer_kinematics/include/cer_kinematics/utils.h:23:31: fatal error: iCub/iKin/iKinFwd.h: No such file or directory

include <iCub/iKin/iKinFwd.h>

pattacini commented 8 years ago

it's compiling on Bluey... Probably some missing variables in the path.

randaz81 commented 8 years ago

I'm against moving anything out from icub-main UNLESS someone first setups a working superbuild...

barbalberto commented 8 years ago

The error shows up when the tripodMotionControl is enabled. Commit 10b03c9 introduced the include and therefore the dependency from iCub. The dependency was handled by the cmake of cer_kinematic library but not updated in the motionControl device. I'll add this include path in the motionControl as well, but it'll be fancy if this dependency remains in the lib and not propagated making this icub's include "private". Not a big deal however.

pattacini commented 8 years ago

Cannot be made "private". cer_kinematics needs to expose the iKin layer.

pattacini commented 8 years ago

Why should motionControl depend on iKin if the dependency is only required by tripodMotionControl?

barbalberto commented 8 years ago

By motionControl I mean tripodMotionControl (I'm lazy) My idea is that tripodMotionControl should not directly depend on (include by chains of includes) iKin since no iKin object are directly used by tripodMotionControl. The cer_kinematic library obviously depends on iKin but, by means of tripod.h, it can expose some methods to tripodMotionControl while keeping the dependency (implementation) private. In this way the implementation can change without the higher tripodMotionControl to notice. In this case the compilation would not have been affected.

pattacini commented 8 years ago

I know what you mean @barbalberto, but the library has grown and now contains other stuff for the whole arm that in turns needs to expose iKin layer.

Probably the export of the library needs to be tuned up so the user is not forced to explicitly put iCub dependent directives in the cmake files.