iDynTree types in FloatingBaseEstimators for python bindings generation

[prashanthr05]

The process of creating bindings for FloatingBaseEstimator gave rise to the necessity of defining iDynTree types. See here the iDynTree types used in FloatingBaseEstimators API.
(If I had addressed #107, we would not have to go through this issue now. But, unfortunately I did not resolve that issue sooner. But maybe, this need will arise soon, maybe? In any case, I would need to define iDynTree types otherwise we get the error.)

In [1]: import bipedal_locomotion_framework.bindings as blf                                          

In [2]: x = blf.FloatingBaseEstimatorOutput()  
In [7]: y = x.base_pose                                                                              
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-7-d97583a07d89> in <module>
----> 1 y = x.base_pose

TypeError: Unable to convert function return value to a Python type! The signature was
	(self: bipedal_locomotion_framework.bindings.FloatingBaseEstimatorOutput) -> iDynTree::Transform

I notice that manif types were defined in BaseTypes.

So we come to a fork where we need to take a decision.
Shall I add iDynTree types similar to manif types or would anyone know a way of using the bindings generated by iDynTree here?

Any suggestions on tackling this problem? @GiulioRomualdi @diegoferigo @traversaro

[traversaro]

Shall I add iDynTree types similar to manif types or would anyone know a way of using the bindings generated by iDynTree here?

At the moment, the iDynTree Python bindings typically used (for example, the one installed by the robotology-superbuild's ROBOTOLOGY_USES_PYTHON) are the SWIG-based ones, so I would try to avoid depending in iDynTree pybind11-based Python bindings to be compiled. If the manif type are part of some blf package, I think we can do the same for iDynTree as you need. In this way, we can tackle later (and only if necessary) the problem of interoperability between iDynTree classes contained in iDynTree Python bindings, and iDynTree classes wrapped in blf python bindings.

[traversaro]

But in this specific case, it is not quicker to just address #107 ?

[prashanthr05]

But in this specific case, it is not quicker to just address #107 ?

Indeed, looks the easiest solution. I will proceed with that. I hope it doesn't cause a huge butterfly effect in some random branch of my fork. :D

[diegoferigo]

Designing public APIs is always challenging, and it becomes quite clear in this specific case :) Until now, blf used extensively Eigen and manif for the public methods. I would recommend to uniform all the APIs to only expose such types, and then performing the conversions privately when needed.

[prashanthr05]

For the moment, we will dodge this bullet thanks to #215.

I will close this issue.

[prashanthr05]

Ciao guys. I am forced to reopen this issue even after addressing #107.

So, there is still one elephant in the room that needs to be addressed for completing the bindings for the floating base estimators. In order to initialize, it requires the setting of shared pointer of KinDynComputations, https:/dic-iit/bipedal-locomotion-framework/blob/255e9ea9a338c3784ff4f5c1e9e159974df3d386/src/Estimators/include/BipedalLocomotion/FloatingBaseEstimators/FloatingBaseEstimator.h#L160-L161

Internally in the estimators, this is used to compute model based relative transformations and Jacobians.
I was wondering how to address that.

[diegoferigo]

It is not mandatory to bind a class to python in order to get an object returned by a function and pass it to another function. The only thing is that this object will have no methods nor properties that can be called from Python.

This being said, who should be responsible to create the KinDynComputations object?

[prashanthr05]

This being said, who should be responsible to create the KinDynComputations object?

The user is expected to create the KinDynComputations object, load it with a valid model and then pass it to the FloatingBaseEstimator as a shared pointer. A shared pointer is used here so that the same object can be used by the FloatingBaseEstimator and ContactDetector blocks (at the moment assumed to be run in a single-threaded process at the C++ level).

[traversaro]

Probably we can do something similar to how the YARP devices are created in https:/dic-iit/bipedal-locomotion-framework/blob/7bdbed8154499a644094bacca0a5313f074c5bbb/bindings/python/RobotInterface.cpp#L36 and #200 ? In that cases, a YARP-only class PolyDriverDescriptor is created using blf-specific classes/methods (that use blf's parameter system), and then it is passed around, wrapped "lightly" in Python. Could you try to do something similar also in blf? Do we have something to create a KinDynComputations in blf from a blf ParametersHandlers, so that the user do not need to have wrappers for iDynTree::ModelLoader and all its functionalities?

[traversaro]

Probably we can do something similar to how the YARP devices are created in

https:/dic-iit/bipedal-locomotion-framework/blob/7bdbed8154499a644094bacca0a5313f074c5bbb/bindings/python/RobotInterface.cpp#L36
and #200 ? In that cases, a YARP-only class PolyDriverDescriptor is created using blf-specific classes/methods (that use blf's parameter system), and then it is passed around, wrapped "lightly" in Python. Could you try to do something similar also in blf? Do we have something to create a KinDynComputations in blf from a blf ParametersHandlers, so that the user do not need to have wrappers for iDynTree::ModelLoader and all its functionalities?

TL;DR: Similarly to https:/dic-iit/bipedal-locomotion-framework/blob/master/src/RobotInterface/YarpImplementation/include/BipedalLocomotion/RobotInterface/YarpHelper.h , could we have a KinDynComputationsDescription and a:

KinDynComputationsDescription constructKinDynComputations(
    std::weak_ptr<BipedalLocomotion::ParametersHandler::IParametersHandler> handler);

function?

(Doubt: Note that I am not sure if KinDynComputationsDescription actually serve something or we can just have the function return KinDynComputation ?)

[traversaro]

(Doubt: Note that I am not sure if KinDynComputationsDescription actually serve something or we can just have the function return KinDynComputation ?)

If we need to pass around shared_ptr, either the function returns std::shared_ptr<KinDynComputation > or KinDynComputationsDescription contains a std::shared_ptr<KinDynComputation >

[prashanthr05]

(Doubt: Note that I am not sure if KinDynComputationsDescription actually serve something or we can just have the function return KinDynComputation ?)

If we need to pass around shared_ptr, either the function returns std::shared_ptr<KinDynComputation > or KinDynComputationsDescription contains a std::shared_ptr<KinDynComputation >

I think KinDynComputationsDescription could be a viable choice. (Update: maybe, in the future we can also provide a mutex within this description to support multi-threaded usage at a C++ level)

P.S. tagging also @S-Dafarra in this thread.

[prashanthr05]

The KinDynComputationsDescriptor added in #216 is working as expected as seen in tests (test_legged_odometry.py) added by #218.

I will close this issue. Thank you all for your inputs!