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

Shorten header structure #132

Closed
traversaro opened this issue Feb 2, 2016 · 5 comments · Fixed by #1104
Closed

Shorten header structure #132

traversaro opened this issue Feb 2, 2016 · 5 comments · Fixed by #1104

Comments

@traversaro
Copy link
Member

Currently, we are using and installing headers with the following structure:

#include <iDynTree/Component/Header.h> 

This was mostly inherited by the structure used in yarp and icub.

For several reasons, I am now inclined to migrate as soon as possible to a structure like:

#include <iDynTree/Header.h> 

The reasons are:

  • We are currently converging to having a single namespace iDynTree , for consistency with bindings in which classes and function are found in the iDynTree module in python, in the iDynTree package in Matlab, etc, etc. The double layer headers in yarp and icub is used also to be consistent with a double layer structure of namespace.
  • I wanted to merge the Model and the Sensors component in a single library (to create sensors-aware model transformers) but one big blocking issue is that cmake currently supports only one public header path for library unless you sensibly complicate the cmake (see https://github.com/robotology/idyntree/blob/master/src/sensors/CMakeLists.txt#L51). Hence moving headers between components wil always mean changing their header path, and that is clearly not desirable for backward compatibility with existing code. Having a single layer header structure will mean that we can move headers across components without problem, simplifyng refactoring.
  • A single layer header structure in the future could be useful to distribute iDynTree in OS X as a Framework.

The change will only affect the new iDynTree-exclusive data structures, that are not currently used by any code external to iDynTree, so it should be harmless.
Any opinions on this change?
cc @francesco-romano @nunoguedelha @DanielePucci @naveenoid @fjandrad

@DanielePucci
Copy link
Collaborator

It seems to me that the point 2 is blocking and I like a lot the possibility of distributing iDynTree in OS X as a framework.

@naveenoid
Copy link
Contributor

I like this idea a lot and I dont think issue 2 is a major problem since at
some point backward compatibility must be abandoned for any kind of
project. And as you mentioned, the compatibility issue is only for someone
working on iDynTree itself and not any user of it right?


Naveen Kuppuswamy, PhD
Post-doctoral Fellow,
Dynamic Interaction Control Lab,
iCub Facility,
Istituto Italiano di Tecnologia, Genova, Italy

On Wed, Feb 3, 2016 at 10:19 AM, Daniele notifications@github.com wrote:

It seems to me that the point 2 is blocking and I like a lot the
possibility of distributing iDynTree in OS X as a framework.


Reply to this email directly or view it on GitHub
#132 (comment)
.

@traversaro
Copy link
Member Author

Yes,

The change will only affect the new iDynTree-exclusive data structures, that are not currently used by any code external to iDynTree, so it should be harmless.

traversaro added a commit that referenced this issue Apr 28, 2016
Furthermore, fix various issues around in the code.

The spreading of code related to this in the library is suboptimal,
and mostly due to the fact that model functions can't depend on sensors data structure.
The definite fix for this is to merge the model and sensors libraries, but
this require that first we solve this issue: #132 .
traversaro added a commit that referenced this issue Aug 14, 2017
traversaro added a commit that referenced this issue Aug 14, 2017
traversaro added a commit that referenced this issue Aug 14, 2017
traversaro added a commit that referenced this issue Aug 16, 2017
@traversaro
Copy link
Member Author

I wanted to merge the Model and the Sensors component in a single library (to create sensors-aware model transformers) but one big blocking issue is that cmake currently supports only one public header path for library unless you sensibly complicate the cmake (see https://github.com/robotology/idyntree/blob/master/src/sensors/CMakeLists.txt#L51). Hence moving headers between components wil always mean changing their header path, and that is clearly not desirable for backward compatibility with existing code. Having a single layer header structure will mean that we can move headers across components without problem, simplifyng refactoring.

fyi @Nicogene , I am now working in solving this.

@traversaro
Copy link
Member Author

https://github.com/robotology/idyntree/tree/fix132

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

Successfully merging a pull request may close this issue.

3 participants