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

YarpSensorBridge::getJointPositions() returns an empty vector if jointPositions size is zero #185

Closed
GiulioRomualdi opened this issue Feb 3, 2021 · 8 comments
Assignees
Labels
bug Something isn't working

Comments

@GiulioRomualdi
Copy link
Member

If jointPositions size is 0 YarpSensorBridge::getJointPositions() return an empty vector
https://github.com/dic-iit/bipedal-locomotion-framework/blob/e2385fc708e4bbb2d4b2553601f3ffdd735d976d/src/RobotInterface/YarpImplementation/src/YarpSensorBridge.cpp#L235-L242

The code does not crash if compiled in release, if it's compiled in debug the following exception is thrown

blf-cartesian-trajectory-player: /usr/include/eigen3/Eigen/src/Core/DenseBase.h:257: void Eigen::DenseBase<Derived>::resize(Eigen::Index, Eigen::Index) [with Derived = Eigen::Ref<Eigen::Matrix<double, -1, 1> >; Eigen::Index = long int]: Assertion `rows == this->rows() && cols == this->cols() && "DenseBase::resize() does not actually allow one to resize."' failed.

apparently the operator=() do not resize the eigen::ref

@prashanthr05, @S-Dafarra do you have any clue>

@GiulioRomualdi
Copy link
Member Author

@S-Dafarra
Copy link
Member

Ref, like map or span cannot resize the underlying container because it does not own the memory. This is the reason why GenericContainer::Vector was designed in the first place 😁

@GiulioRomualdi
Copy link
Member Author

For the time being, we can add a check into getJointPositions and similar functions

@GiulioRomualdi GiulioRomualdi added the bug Something isn't working label Feb 3, 2021
@prashanthr05
Copy link
Collaborator

prashanthr05 commented Feb 3, 2021

If I remember right, we switched from the usage of GenericContainer to Eigen::Ref and added this warning saying that the user should pass a resized vector.

https://github.com/dic-iit/bipedal-locomotion-framework/blob/e2385fc708e4bbb2d4b2553601f3ffdd735d976d/src/RobotInterface/YarpImplementation/include/BipedalLocomotion/RobotInterface/YarpSensorBridge.h#L209-L211

and looks like the addition of checks were not done in the YarpSensorBridge implementation. So yes, we have to update the code in YarpSensorBridge.

@S-Dafarra
Copy link
Member

I think that this is strongly related to #95. I think it is necessary to clearly define a guideline.

@GiulioRomualdi
Copy link
Member Author

Otherwise we may change

 bool YarpSensorBridge::getJointPositions(Eigen::Ref<Eigen::VectorXd> jointPositions, 
                                          double* receiveTimeInSeconds) 

into

Eigen::Ref<const Eigen::VectorXd> YarpSensorBridge::getJointPositions(double* receiveTimeInSeconds) 

or

GenericVector<const double> YarpSensorBridge::getJointPositions(double* receiveTimeInSeconds) 

@S-Dafarra
Copy link
Member

Otherwise we may change

 bool YarpSensorBridge::getJointPositions(Eigen::Ref<Eigen::VectorXd> jointPositions, 
                                          double* receiveTimeInSeconds) 

into

Eigen::Ref<const Eigen::VectorXd> YarpSensorBridge::getJointPositions(double* receiveTimeInSeconds) 

or

GenericVector<const double> YarpSensorBridge::getJointPositions(double* receiveTimeInSeconds) 

See #124

@GiulioRomualdi
Copy link
Member Author

The checks added in #946 mitigate the problem. Indeed now an error will be raised in case of wrong size of the vector

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants