search

robotology / whole-body-estimators

YARP devices that implement estimators for humanoid robots.
24 stars 12 forks source link

WholeBodyDynamicsDevice: Remove meaningless check that total number FTs in attached device match the total number of FTs consired by estimation #193

Closed traversaro closed 4 days ago

traversaro commented 4 days ago

This is a bug found by @LoreMoretti , basically he was trying to remote just the upper_leg FTs from ergocub from the estimation, but the wbd launch was failing with:

        yError() << "WholeBodyDynamicsDevice::attachAll Invalid number of MAS FT sensors after remapper";

However, the check does not make a lot of sense. nrMASFTSensors is the total number of FT sensors in all the devices attached to wbd (for ergocub 8), while remappedMASInterfaces.ftMultiSensors->getNrOfSixAxisForceTorqueSensors() and ftMultipleAnalogSensorNames.size() are the actual number of sensors considered (removing the upper_leg fts as @LoreMoretti was doing, 6).

To be honest, I am wondering how we were removing the FT from the estimation with this bug. Perhaps in all other cases were also either avoiding to pass the upper_leg FTs devices to the attach list, or we removed the FT sensors from the devices opened by the yarprobotinterface?

S-Dafarra commented 4 days ago

To be honest, I am wondering how we were removing the FT from the estimation with this bug. Perhaps in all other cases were also either avoiding to pass the upper_leg FTs devices to the attach list, or we removed the FT sensors from the devices opened by the yarprobotinterface?

Both 😅

S-Dafarra commented 4 days ago

I was looking at this piece of code https://github.com/robotology/whole-body-estimators/blob/1fc1d54c9db9183f7a6e4a2f537ea8bef885230c/devices/wholeBodyDynamics/WholeBodyDynamicsDevice.cpp#L1826-L1836

Maybe it should throw an error if the name is not found?

traversaro commented 4 days ago

Maybe it should throw an error if the name is not found?

I am preparing a different PR to drop the support for reading FT sensor data from IAnalogSensor interfaces, I think it is more appropriate to keep that change there.

traversaro commented 4 days ago

Maybe it should throw an error if the name is not found?

I am preparing a different PR to drop the support for reading FT sensor data from IAnalogSensor interfaces, I think it is more appropriate to keep that change there.

Actually that was already removed in https://github.com/robotology/whole-body-estimators/pull/181 . However I indeed preparing a different cleanup PR.