Unify options for compilation of Matlab-dependent packages

[traversaro]

We are starting to have a lot of Matlab-dependent packages (WBI-Toolbox, mex-wholebodymodel, now qpOASES bindings). Given that the main two possible user configuration are "build everything except what requires Matlab" or "build everything, including what requires Matlab", we should simplify things. To streamline installation, I guess it would make sense to drop the project specific superbuild options (CODYCO_USES_WBI_TOOLBOX, CODYCO_USES_MEX_WHOLEBODYMODEL, etc.. ) and just have a single CODYCO_USES_MATLAB (suggestions for a better name are welcome) that enables download/compilation of everything that requires Matlab.

Eventually we can automatically check if Matlab is available and enable this option if Matlab is found.

cc @francesco-romano @naveenoid @jeljaik

[francesco-romano]

I rather add (instead of substituting) the CODYCO_USES_MATLAB option.
If that flag is true we enable by default all the other options. But the user can (and should) still have finer control.

[DanielePucci]

I like a lot the idea of automatic installation. I agree with @francesco-romano that leaving the users finer options may be useful (e.g. controllers may downloaded in other places).

[traversaro]

I would prefer to reduce as much as possible the complexity of both the cmake options available to the users and the CMakeLists.txt files structure, both for being more user-friendly and for maintainability and readability.

I agree that finer control of which projects are enabled on the superbuild must be useful, but this applies to all the projects, not only the matlab ones.

cc @DanielePucci I intentionally omitted the CODYCO_USES_WBI_TOOLBOX_CONTROLLERS option, as it is usually used separately (and strictly speaking it is not something that needs Matlab to compile).

[traversaro]

If we are really interesting in giving the user fine control of which package compile or not, I would work on this upstream on YCM, instead of manually hard coding a lot of options: robotology/ycm-cmake-modules#8 .

[traversaro]

cc @Ommac @DiegoRomeres

[DanielePucci]

Matlab does not find QPOasis. It seems to me that the option

CODYCO_USES_MATLAB 

does not enable the MATLAB binding

QPOASES_BUILD_BINDINGS_MATLAB 

of the project external/qpOasis. Aren't I right? @traversaro @francesco-romano

[traversaro]

You are right. We should add in https://github.com/robotology/codyco-superbuild/blob/master/cmake/BuildqpOASES.cmake an option to enable the QPOASES_BUILD_BINDINGS_MATLAB if CODYCO_USES_MATLAB is enabled in the superbuild.
A similar patter is already done in the iDynTree, where IDYNTREE_USES_MATLAB is enabled if CODYCO_USES_MATLAB is enabled:
https://github.com/robotology/codyco-superbuild/blob/master/cmake/BuildiDynTree.cmake (relevant commit: a06d1d5 )

[DanielePucci]

@nunoguedelha will take care of this modification :)

[nunoguedelha]

After a short conversation with @francesco-romano on this topic, we agreed on clarifying the requirements before any further implementation.

So, enabling CODYCO_USES_MATLAB will:

1. enable xxx_USES_MATLAB for all dependent modules in the current settings

or

2. set the default value of xxx_USES_MATLAB to ON in the dependant modules. In this case, the variable would be enabled in a given module only after CMakeCache.txt is erased and regenerated

(These use cases do not address the possibility for users to fine control the individual superbuild components options).

I would go for the first option. Was this your choice?

[traversaro]

I would go for 1) !

[DanielePucci]

I would go for 1) also. I think that if @francesco-romano agrees, the poll is over and we can proceed with the modification :)

[nunoguedelha]

ok great. For validation I need to build with this option ON, but I'm getting a compile error when linking iDynTreeMATLAB_wrap (missing symbols referenced in SWIG_xxx() functions in iDynTreeMATLAB_wrap.o) . Silvio @traversaro, do we need a specific swig official/debug version for iDyntree bindings (I'm using 3.0.2) ?

[traversaro]

Yes it is using an experimental version of Swig, for this reason I commit the generated files (and I don't ask the user to generate the bindings, but just to compile them). Probably I forgot to regenerate the bindings on my local machine.

(FYI: to generate the iDynTree Matlab bindings, you should compile from source a swig experimental version (as in https://github.com/casadi/casadi/wiki/matlab) and then generate the bindings to commit with the iDynTree GENERATE_MATLAB CMake option).

Let me reboot on Linux and I will check.

[traversaro]

No, I actually don't have any problem on my computer with the current iDynTree master branch...
Can you post the complete error?

[nunoguedelha]

build command :
xcodebuild -verbose -configuration Debug -target iDynTree

error output:

=== BUILD TARGET iDynTreeMATLAB_wrap OF PROJECT iDynTree WITH CONFIGURATION Debug ===

Check dependencies

Ld /Users/nunoguedelha/dev/codyco-superbuild/build/libraries/iDynTree/lib/Debug/iDynTreeMATLAB_wrap.mexmaci64 normal x86_64
    cd /Users/nunoguedelha/dev/codyco-superbuild/libraries/iDynTree
    export MACOSX_DEPLOYMENT_TARGET=10.10
    /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang++ -arch x86_64 -bundle -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.10.sdk -L/Users/nunoguedelha/dev/codyco-superbuild/build/libraries/iDynTree/lib/Debug -F/Users/nunoguedelha/dev/codyco-superbuild/build/libraries/iDynTree/lib/Debug -filelist /Users/nunoguedelha/dev/codyco-superbuild/build/libraries/iDynTree/bindings/matlab/iDynTree.build/Debug/iDynTreeMATLAB_wrap.build/Objects-normal/x86_64/iDynTreeMATLAB_wrap.LinkFileList -Xlinker -rpath -Xlinker /Applications/MATLAB_R2012b.app/bin/maci64 -Xlinker -rpath -Xlinker /Users/nunoguedelha/dev/codyco-superbuild/build/libraries/iDynTree/lib/Debug -Xlinker -rpath -Xlinker /Users/nunoguedelha/dev/codyco-superbuild/build/install/lib -mmacosx-version-min=10.10 -Wl,-search_paths_first -Wl,-headerpad_max_install_names /Applications/MATLAB_R2012b.app/bin/maci64/libmex.dylib /Users/nunoguedelha/dev/codyco-superbuild/build/libraries/iDynTree/lib/Debug/libidyntree-modelio-symoro.dylib /Users/nunoguedelha/dev/codyco-superbuild/build/libraries/iDynTree/lib/Debug/libidyntree-modelio-ikin.dylib /Users/nunoguedelha/dev/codyco-superbuild/build/libraries/iDynTree/lib/Debug/libidyntree-yarp.0.3.5.dylib /Users/nunoguedelha/dev/codyco-superbuild/build/libraries/iDynTree/lib/Debug/libexpression-parser.dylib /Users/nunoguedelha/dev/codyco-superbuild/build/libraries/iDynTree/lib/Debug/libidyntree-regressors.0.3.5.dylib /Users/nunoguedelha/dev/codyco-superbuild/build/libraries/iDynTree/lib/Debug/libidyntree-sensors.dylib /Users/nunoguedelha/dev/codyco-superbuild/build/libraries/iDynTree/lib/Debug/libidyntree-modelio-urdf.dylib /Users/nunoguedelha/dev/codyco-superbuild/build/libraries/iDynTree/lib/Debug/libidyntree-core.dylib /Users/nunoguedelha/dev/codyco-superbuild/build/libraries/iDynTree/lib/Debug/libidyntree-core-exp.dylib /Users/nunoguedelha/dev/codyco-superbuild/build/install/lib/liborocos-kdl.1.2.3.dylib /Users/nunoguedelha/dev/codyco-superbuild/build/install/lib/liburdfdom_model.dylib /Users/nunoguedelha/dev/codyco-superbuild/build/install/lib/liburdfdom_world.dylib /Users/nunoguedelha/dev/codyco-superbuild/build/install/lib/libtinyxml.2.6.2.dylib /Users/nunoguedelha/dev/icub-main/build/lib/Debug/libiDyn.a /Users/nunoguedelha/dev/icub-main/build/lib/Debug/libiKin.a /Users/nunoguedelha/dev/icub-main/build/lib/Debug/libskinDynLib.a /Users/nunoguedelha/dev/icub-main/build/lib/Debug/libctrlLib.a /usr/local/lib/libgsl.dylib /usr/local/lib/libgslcblas.dylib /Users/nunoguedelha/dev/yarp/buildXcode/lib/Debug/libYARP_math.2.3.63.7.dylib /Users/nunoguedelha/dev/yarp/buildXcode/lib/Debug/libYARP_dev.2.3.63.7.dylib /Users/nunoguedelha/dev/yarp/buildXcode/lib/Debug/libYARP_sig.2.3.63.7.dylib /Users/nunoguedelha/dev/yarp/buildXcode/lib/Debug/libYARP_name.2.3.63.7.dylib /Users/nunoguedelha/dev/yarp/buildXcode/lib/Debug/libYARP_init.2.3.63.7.dylib /Users/nunoguedelha/dev/yarp/buildXcode/lib/Debug/libYARP_OS.2.3.63.7.dylib -Xlinker -dependency_info -Xlinker /Users/nunoguedelha/dev/codyco-superbuild/build/libraries/iDynTree/bindings/matlab/iDynTree.build/Debug/iDynTreeMATLAB_wrap.build/Objects-normal/x86_64/iDynTreeMATLAB_wrap_dependency_info.dat -o /Users/nunoguedelha/dev/codyco-superbuild/build/libraries/iDynTree/lib/Debug/iDynTreeMATLAB_wrap.mexmaci64
Undefined symbols for architecture x86_64:
  "_mxCreateCharArray_730", referenced from:
      SWIG_FromCharPtrAndSize(char const*, unsigned long) in iDynTreeMATLAB_wrap.o
  "_mxCreateDoubleScalar", referenced from:
      SWIG_From_double(double) in iDynTreeMATLAB_wrap.o
...
  "_mxIsLogicalScalarTrue", referenced from:
      SWIG_Matlab_ConvertPtrAndOwn(mxArray_tag*, void**, swig_type_info*, int, int*) in iDynTreeMATLAB_wrap.o
  "_mxSetProperty_730", referenced from:
      SWIG_Matlab_ConvertPtrAndOwn(mxArray_tag*, void**, swig_type_info*, int, int*) in iDynTreeMATLAB_wrap.o
      SWIG_Matlab_NewPointerObj(void*, swig_type_info*, int) in iDynTreeMATLAB_wrap.o
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)

** BUILD FAILED **

I get the same issue when building target ALL_BUILD.

[nunoguedelha]

in iDynTree, I have the variables :
GENERATE_MATLAB = OFF
IDYNTREE_COMPILE_BINDINGS = ON

and get the same issue...

[traversaro]

Mhh I should probably generate the bindings on an old version of Matlab (such as 2012b) to be sure of being forward compatible (or perhaps is an OS X-specific issue)... I guess for the time being we can remove this line: https://github.com/robotology/codyco-superbuild/blob/master/cmake/BuildiDynTree.cmake#L24 so we can compile the matlab support in the superbuild without problems.

[DanielePucci]

Any news on this? @nunoguedelha

[nunoguedelha]

I have a branch ready but I'm not able to push. Could someone grant me the permissions?

[traversaro]

@drdanz can you add @nunoguedelha to the group https://github.com/orgs/robotology/teams/codyco-developers ? Thanks!

[nunoguedelha]

while waiting for the permission, I forked the repo and pushed the branch here:
nunoguedelha@b11e7eb

[DanielePucci]

Well done 👍

[drdanz]

@nunoguedelha You are listed as pending, you should be able to join the group either by accepting the invitation on the mail you should have received, or by going to the group page and accepting (I think), otherwise I can try cancelling the invitation and inviting you again.

[nunoguedelha]

Sure. I didn't receive the mail, I'll try the group page...

[francesco-romano]

What about the status of this issue?
@nunoguedelha @traversaro

[traversaro]

We disable Matlab compilation for iDynTree because we had some issue, but now we can enable it and close the issue.

[traversaro]

Based on the experience in the past months, with @francesco-romano we propose to modify the option in this way:
If the CODYCO_USES_MATLAB is on, all the projects using Matlab will be enabled (i.e. the option to enabled them will be forced to be true in the cache). This will be the option used by first time users if they want to enable compilation of the matlab subprojects, and will be advertised in the installation guide (see #83). If CODYCO_USES_MATLAB is off, the user will be free to set to on/off all the flags of the subprojects, without having to set another (undocumented) variable such as CODYCO_MATLAB_DEPENDENCIES_CUSTOM.
cc @nunoguedelha @DanielePucci any opinions?

[traversaro]

I have yet another proposal based on how users use this options (to be implemented in #113 ):
Get rid of CODYCO_USES_WBI_TOOLBOX, CODYCO_USES_WB_TOOLBOX, CODYCO_MATLAB_DEPENDENCIES_CUSTOM, CODYCO_USES_MEX_WHOLEBODYMODEL .

Just keep CODYCO_USES_MATLAB that compiles all the matlab related projects (mex-wholebodymodel, wb-toolbox, idyntree matlab bindings) and CODYCO_USES_WBI_TOOLBOX_CONTROLLERS to download the controllers.

As far as I could see, the only problem difference is among users is if they want to download the controllers as part of the superbuild or not, so let's have an option only for that, and cut the unnecessary complexity.

[traversaro]

As for suggestion of @francesco-romano , I left the CODYCO_USES_WBI_TOOLBOX option scorporated from the CODYCO_USES_MATLAB option, so any user can just disable the legacy WBI-Toolbox at any time.

See ae316f6

[traversaro]

With the addition of the CODYCO_NOT_USE_SIMULINK in 822ff31 0d30270 I think the major issues related to this issues have been solved.