Add RobotInterface library and a SensorBridge interface class #87

prashanthr05 · 2020-08-27T13:10:10Z

This PR,

Adds RobotInterface library to the Framework
Adds ISensorBridge interface class to RobotInterface library
~~- Adds OpenCV as a dependency for handling cv::Mat within the ISensorBridge class~~
Adds a trivial DummySensorBridge test implementation to depict the use of the ISensorBridge interface

S-Dafarra · 2020-08-28T10:09:56Z

Adds OpenCV as a dependency for handling cv::Mat within the ISensorBridge class

To be honest, I think that OpenCV may be a too strong dependency for a single type. It should be possible to convert a cv::Mat type from/to an Eigen type, https://github.com/opencv/opencv/blob/e3d502310f6ae0cad79f3474b9ea2b25705985f9/modules/core/include/opencv2/core/eigen.hpp

What about using Eigen types in the interface then?

I would be totally fine in having OpenCV as a dependency in case we develop some modules which use it extensively, but right now it seems a bit of an overkill.

prashanthr05 · 2020-08-28T10:13:39Z

To be honest, I think that OpenCV may be a too strong dependency for a single type. It should be possible to convert a cv::Mat type from/to an Eigen type, https://github.com/opencv/opencv/blob/e3d502310f6ae0cad79f3474b9ea2b25705985f9/modules/core/include/opencv2/core/eigen.hpp

What about using Eigen types in the interface then?

I thought about it too. If we can depend on the user to make relevant conversions from Eigen to relevant types, then we can keep the design that way.

In that case, would it also make sense to change GenericVectorContainer to Eigen?

prashanthr05 · 2020-08-28T10:14:46Z

But then, if we also want point clouds to be handled by this Bridge, then there comes a problem of adding PCL which is quite a heavy library that depends on many other libraries as a dependency.

So we might have to think about this a bit.

If we find suitable conversions between Eigen and PCL and these conversions are lightweight, then it's reasonable.

S-Dafarra · 2020-08-28T10:25:06Z

In that case, would it also make sense to change GenericVectorContainer to Eigen?

The GenericContainer::Vector was meant to have a unique way to deal with multiple vector implementations that could have also been resized. This was coming in handy when dealing with parameters. Depending on the implementation and on the needs, we may have needed a Yarp::Sig::Vector, or iDynTree::VectorDynSize for example. This was before moving to Eigen in the main API. So, right now I would suggest using GenericContainer::Vector in case you need to deal with different vector implementation at the same time, especially in case you need to resize them (something that is not possible with an Eigen::map).

But then, if we also want point clouds to be handled by this Bridge, then there comes a problem of adding PCL which is quite a heavy library that depends on many other libraries as a dependency.

Is there a specific type/class we may need to use from this library?

prashanthr05 · 2020-08-28T10:31:55Z

The GenericContainer::Vector was meant to have a unique way to deal with multiple vector implementations that could have also been resized. This was coming in handy when dealing with parameters. Depending on the implementation and on the needs, we may have needed a Yarp::Sig::Vector, or iDynTree::VectorDynSize for example. This was before moving to Eigen in the main API. So, right now I would suggest using GenericContainer::Vector in case you need to deal with different vector implementation at the same time, especially in case you need to resize them (something that is not possible with an Eigen::map).

Agreed.

Is there a specific type/class we may need to use from this library?

I am just looking forward into our tasks. For example, for the staircase climbing task, if we need perception in the loop, we could begin to use PCL library extensively for plane segmentation. But from within the class, we would need pcl::PoincloudXYZRGB.

S-Dafarra · 2020-08-28T10:39:03Z

I am just looking forward into our tasks. For example, for the staircase climbing task, if we need perception in the loop, we could begin to use PCL library extensively for plane segmentation. But from within the class, we would need pcl::PoincloudXYZRGB.

I think there might be some conversions available. https://github.com/PointCloudLibrary/pcl/blob/2c8d58a0e6497d91cc50f61460d75b9ebf44429a/common/include/pcl/point_cloud.h#L351

Anyway, the take-home message would be to avoid heavy dependencies only for a type in an interface class. For sure, if we need some additional feature of these libraries, it makes total sense to add them. In principle, I may want to avoid having a certain dependency, paying the price of losing some features. But I am expecting the feature to be related to the dependency I am missing. In this specific case, it would be a pity not to be able to compile the RobotInterface part if OpenCV is missing. This may complicate things if we would need this to run on the robot head maybe.

prashanthr05 · 2020-08-28T20:37:21Z

Anyway, the take-home message would be to avoid heavy dependencies only for a type in an interface class. For sure, if we need some additional feature of these libraries, it makes total sense to add them. In principle, I may want to avoid having a certain dependency, paying the price of losing some features. But I am expecting the feature to be related to the dependency I am missing. In this specific case, it would be a pity not to be able to compile the RobotInterface part if OpenCV is missing. This may complicate things if we would need this to run on the robot head maybe

I understand. In that case, we can avoid having OpenCV and PCL as a dependency for this interface class and rely on relevant conversions at a higher level. I will replace the image related methods to Eigen types.
For the PCL, we shall deal with it in the near future.

src/RobotInterface/include/BipedalLocomotion/RobotInterface/ISensorBridge.h

src/RobotInterface/tests/DummyImplementation/DummySensorBridgeTest.cpp

GiulioRomualdi · 2020-08-31T17:51:37Z

src/RobotInterface/include/BipedalLocomotion/RobotInterface/ISensorBridge.h

+     * @return true/false in case of success/failure
+     */
+    virtual bool getLinearAccelerometerMeasurement(const std::string& accName,
+                                                   GenericContainer::Vector<double>& accMeasurement,


Is it a good idea to use the GenericContainer::Vector here? In all the other parts of the code, we use Eigen vectors as intefrace of the class. I would avoid having mixing different kinds of types

For instance here you can use an Eigen::Ref<Eigen::Vector3d>
And the you can pass every different container (std vector iDynTree or yarp) through an Eigen::Map

We had a discussion about this at #87 (comment).

The important point that was raised was,

So, right now I would suggest using GenericContainer::Vector in case you need to deal with different vector implementation at the same time, especially in case you need to resize them (something that is not possible with an Eigen::map).

@S-Dafarra ,

I would avoid having mixing different kinds of types

would you have an opinion on this?

@S-Dafarra ,

I would avoid having mixing different kinds of types

would you have an opinion on this?

I would say that it depends on whether accMeasurement has a predefined size or not. If so, I agree on using Eigen types for consistency.

At the moment, we assume for joint states and images, that the size is decided at the configuration and remains fixed, and internal checks can be done at the implementation level. This means that the user must pass a resized argument to these getter methods. Correct me if I am wrong, @GiulioRomualdi!
I shall add a description in the documentation regarding this.

However, we need to understand how to handle this for the elements with variable size, for example, point clouds.

Ok for the documentation. For point clouds, are they matrices or vector of points?

They are usually handled as a struct asking for maintaining cloud width and height. Then they store a vector of XYZ points or XYZRGB points.

I hope we will not be causing any overheads by doing multiple conversions by passing these kinds of images or point cloud measurements through this bridge.

I think we may open one or more issue to discuss on this. In particular, it is not clear:

when to use Eigen vectors vs GenericContainer vs templates

how to deal with matrices

how to deal with particular input classes, like point clouds

I have added documentation for the joint states and image methods and relevant configuration parameters in the SensorBridgeOptions if the implementation might want to use it. 979733e

src/RobotInterface/include/BipedalLocomotion/RobotInterface/ISensorBridge.h

GiulioRomualdi · 2020-09-04T18:41:29Z

@prashanthr05 let me know if you want to reorganize the commits otherwise I will merge the PR

[RobotInterface] ISensorBridge make receiveTimeInSeconds optional Add optional helper methods for use of SensorBridgeOptions and SensorLists in the ISensorBridge interface

prashanthr05 · 2020-09-07T17:14:38Z

@prashanthr05 let me know if you want to reorganize the commits otherwise I will merge the PR

Ciao @GiulioRomualdi .. I have squashed and rebased all changes into a single commit. Thanks for waiting!

prashanthr05 requested review from GiulioRomualdi, HosameldinMohamed and S-Dafarra August 27, 2020 13:15

prashanthr05 force-pushed the feature/robotInterface branch from 1a45c8d to 444c13e Compare August 27, 2020 13:19

prashanthr05 force-pushed the feature/robotInterface branch 4 times, most recently from e4741d2 to 4838ba0 Compare August 28, 2020 21:05

S-Dafarra reviewed Aug 31, 2020

View reviewed changes

src/RobotInterface/include/BipedalLocomotion/RobotInterface/ISensorBridge.h Outdated Show resolved Hide resolved

src/RobotInterface/include/BipedalLocomotion/RobotInterface/ISensorBridge.h Outdated Show resolved Hide resolved

S-Dafarra reviewed Aug 31, 2020

View reviewed changes

GiulioRomualdi reviewed Aug 31, 2020

View reviewed changes

GiulioRomualdi assigned prashanthr05 Sep 3, 2020

GiulioRomualdi added this to the Iteration 50 - 50!! milestone Sep 3, 2020

prashanthr05 force-pushed the feature/robotInterface branch from 4a4b818 to 8d44405 Compare September 3, 2020 19:48

GiulioRomualdi approved these changes Sep 3, 2020

View reviewed changes

S-Dafarra reviewed Sep 4, 2020

View reviewed changes

src/RobotInterface/include/BipedalLocomotion/RobotInterface/ISensorBridge.h Outdated Show resolved Hide resolved

src/RobotInterface/include/BipedalLocomotion/RobotInterface/ISensorBridge.h Outdated Show resolved Hide resolved

S-Dafarra approved these changes Sep 4, 2020

View reviewed changes

prashanthr05 mentioned this pull request Sep 4, 2020

Discuss the usage of proper types to handle heterogeneous elements through the RobotInterface #94

Closed

GiulioRomualdi reviewed Sep 4, 2020

View reviewed changes

src/RobotInterface/include/BipedalLocomotion/RobotInterface/ISensorBridge.h Outdated Show resolved Hide resolved

Add SensorBridge interface

9f9a8b6

[RobotInterface] ISensorBridge make receiveTimeInSeconds optional Add optional helper methods for use of SensorBridgeOptions and SensorLists in the ISensorBridge interface

prashanthr05 force-pushed the feature/robotInterface branch from 699ea3f to 9f9a8b6 Compare September 7, 2020 17:13

GiulioRomualdi merged commit 26f45d7 into ami-iit:master Sep 7, 2020

prashanthr05 deleted the feature/robotInterface branch September 7, 2020 17:47

prashanthr05 mentioned this pull request Sep 18, 2020

Understand how to handle Eigen::Ref conversions of yarp Images #110

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add RobotInterface library and a SensorBridge interface class #87

Add RobotInterface library and a SensorBridge interface class #87

prashanthr05 commented Aug 27, 2020 •

edited

S-Dafarra commented Aug 28, 2020 •

edited

prashanthr05 commented Aug 28, 2020

prashanthr05 commented Aug 28, 2020 •

edited

S-Dafarra commented Aug 28, 2020 •

edited

prashanthr05 commented Aug 28, 2020

S-Dafarra commented Aug 28, 2020

prashanthr05 commented Aug 28, 2020

GiulioRomualdi Aug 31, 2020

GiulioRomualdi Aug 31, 2020

prashanthr05 Aug 31, 2020

prashanthr05 Sep 1, 2020

S-Dafarra Sep 1, 2020

prashanthr05 Sep 4, 2020

S-Dafarra Sep 4, 2020

prashanthr05 Sep 4, 2020

S-Dafarra Sep 4, 2020 •

edited

prashanthr05 Sep 4, 2020

GiulioRomualdi commented Sep 4, 2020

prashanthr05 commented Sep 7, 2020 •

edited

Add RobotInterface library and a SensorBridge interface class #87

Add RobotInterface library and a SensorBridge interface class #87

Conversation

prashanthr05 commented Aug 27, 2020 • edited

S-Dafarra commented Aug 28, 2020 • edited

prashanthr05 commented Aug 28, 2020

prashanthr05 commented Aug 28, 2020 • edited

S-Dafarra commented Aug 28, 2020 • edited

prashanthr05 commented Aug 28, 2020

S-Dafarra commented Aug 28, 2020

prashanthr05 commented Aug 28, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

S-Dafarra Sep 4, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GiulioRomualdi commented Sep 4, 2020

prashanthr05 commented Sep 7, 2020 • edited

prashanthr05 commented Aug 27, 2020 •

edited

S-Dafarra commented Aug 28, 2020 •

edited

prashanthr05 commented Aug 28, 2020 •

edited

S-Dafarra commented Aug 28, 2020 •

edited

S-Dafarra Sep 4, 2020 •

edited

prashanthr05 commented Sep 7, 2020 •

edited