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

Add Octave Mex support #305

Merged
merged 7 commits into from
Jun 2, 2017
Merged

Add Octave Mex support #305

merged 7 commits into from
Jun 2, 2017

Conversation

traversaro
Copy link
Member

@traversaro traversaro commented May 29, 2017

Add support for compiling the mex interface that we have been using for over an year with Matlab.
This is done by solving (probably not with the best possible solution) #188 and updating the used version of swig-matlb to the latest version available at https://github.com/jaeandersson/swig , to exploit all the improvements explained in jaeandersson/swig#67 .

This has the following advantages over the existing octave bindings, that use the system version of Swig to create bindings that use the C++ interface of Octave:

  • We have a way to test the mex interface on Travis (this is particularly useful to catch when the commited version goes out of sync with the C++ interfaces, i.e. a re-generation of the bindings is necessary).
  • We expose in iDynTree exactly the same interface that we expose in matlab (the existing Octave interface had several differences with the Matlab interface, for example you had to type iDynTree; before being able to use the bindings.
  • All additional helper methods (such as the one contained in https://github.com/robotology/idyntree/blob/master/bindings/matlab/matlab_matvec.i ) need to be written and maintained only for the mex-interface .
  • The mex C-based interface is less subject to change, while the C++ interface of Octave can have breaking changes across minor releases (see Octave bindings are not compiling in macOS with homebrew that ships Octave 4.2 #238)
  • The mex interface is C-based, so it works even if Octave was compiled with a compiler that has a C++ ABI different from the compiler used to compile the bindings (mainly a problem on Windows with mingw/Visual Studio)

As a downside, the mex-based bindings of Octave are probably slower than the one based on the C++ octave interface, but I think it is a downside with which we can cope, as Octave will be typically used for testing or for running calibration routines such as the one in https://github.com/robotology-playground/sensors-calib-inertial on the PC of the robots).

My idea (for the reason explained in the PR) is to merge this version of the bindings (even if there are some existing issues: ) , and in a short time remove completely from the repo the support for building the C++ Octave bindings, and just least to the user the possibility to compile the mex-based Octave bindings.

cc @francesco-romano @nunoguedelha @diegoferigo @gabrielenava @fjandrad @S-Dafarra

*Please note that this PR will need to be re-generate the bindings (with the new commit jaeandersson/swig@260ed47, as documented in the README) but I will commit the auto-generated files just before merging to simplify the review of the PR.

@traversaro
Copy link
Member Author

Related issue: #190 .

@traversaro
Copy link
Member Author

An update to the documentation is coming in the next PR, for the time being if you want to try the Octave bindings you just need to add the <prefix>/oct directory to the Octave path.

@traversaro
Copy link
Member Author

If no one has comments I will merge this PR this evening.

@traversaro
Copy link
Member Author

Ready to merge, and the tests are already running in Travis!
We already got a bug (fixed in ea3ee6a) thanks to this mex-based Octave bindings.

@traversaro traversaro merged commit 755db06 into devel Jun 2, 2017
traversaro added a commit that referenced this pull request Jun 2, 2017
@traversaro traversaro deleted the addOctaveMexSupport branch June 2, 2017 10:19
traversaro added a commit that referenced this pull request Jun 6, 2017
For some reason, Octave tests started failing in master. 
Given that we will soon merge `devel` in `master` and in the `devel` the bindings for Octave changed completely (see #305),  I think for the time being we can just disable testing the Octave bindings in master.
traversaro added a commit that referenced this pull request Jun 7, 2017
For some reason, Octave tests started failing in master. 
Given that we will soon merge `devel` in `master` and in the `devel` the bindings for Octave changed completely (see #305),  I think for the time being we can just disable testing the Octave bindings in master.
traversaro added a commit to robotology/yarp-matlab-bindings that referenced this pull request Jun 14, 2017
* Port improvements from iDynTree in robotology/idyntree#305 .
  This includes full Octave support.
* Stop including a copy of yarp.i locally, and just use the upstream copy of yarp.i
  (depends on robotology/yarp#1270)
traversaro added a commit to robotology/yarp-matlab-bindings that referenced this pull request Jun 14, 2017
* Port improvements from iDynTree in robotology/idyntree#305 .
  This includes full Octave support.
* Stop including a copy of yarp.i locally, and just use the upstream copy of yarp.i
  (depends on robotology/yarp#1270)
traversaro pushed a commit that referenced this pull request Jun 30, 2017
* added primatic joint to CMAKE

* added primatic joint to urdf model import

* added primatic joint

* minor change

* prismatic joint source and header

* primatic joint included in unit test

* minor fix

* included prismatic joint

* translation methods added to axis class

* variable name changes

* translation transform clean up

* minor fix

* minor fix - setting transform rotation using Identity()

* translation transform derivative changes

* translation transform derivative - rotation derivative set to zero

* minor fix - unused parameter commented

* minor fix-indentation

* prismatic resetAxisBuffers

* [travis] Disable testing of Octave bindings in master

For some reason, Octave tests started failing in master. 
Given that we will soon merge `devel` in `master` and in the `devel` the bindings for Octave changed completely (see #305),  I think for the time being we can just disable testing the Octave bindings in master.

* Fix big size of dox generation

* minor fix - comma followed space correction

* minor fix

* Add release notes on prismatic joint support
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 this pull request may close these issues.

2 participants