search

MRPT / mrpt

:zap: The Mobile Robot Programming Toolkit (MRPT)
https://docs.mrpt.org/reference/latest/
BSD 3-Clause "New" or "Revised" License
1.94k stars 629 forks source link

New variance field for the Range message #1270

Closed ejalaa12 closed 11 months ago

ejalaa12 commented 1 year ago

The Range message will get an additional field for the variance. I don't know if this is relevant for you guys, but this issue is simply a notification, feel free to close it if it's not a change that you could benefit from.

You can see details about the PR here

jlblancoc commented 11 months ago

@ejalaa12 This has been finally addressed here: https://github.com/MRPT/mrpt/commit/67358e90876f6af1b1021ec03d60740c68521157

so MRPT >= v2.11.2 will use the new variance field.

ejalaa12 commented 11 months ago

@jlblancoc awesome! I saw the commit, this a clever way of making sure the member exist. I guess this makes it retro-compatibke with old version of the Range message as well.

jlblancoc commented 11 months ago

Exactly 👍

jlblancoc commented 11 months ago

However, for some reason, the if constexpr trick fails on Humble... hmm....

https://build.ros2.org/job/Hdev__mrpt2__ubuntu_jammy_amd64/349/console

03:46:44 [ 92%] Building CXX object libs/ros2bridge/CMakeFiles/ros2bridge.dir/src/range.cpp.o
03:46:44 /tmp/ws/src/mrpt2/libs/ros2bridge/src/range.cpp: In function ‘bool mrpt::ros2bridge::fromROS(const Range&, mrpt::obs::CObservationRange&)’:
03:46:44 /tmp/ws/src/mrpt2/libs/ros2bridge/src/range.cpp:43:78: error: ‘const Range’ {aka ‘const struct sensor_msgs::msg::Range_<std::allocator<void> >’} has no member named ‘variance’
03:46:44    43 |                 obj.sensedData.at(0).sensorNoiseStdDeviation = std::sqrt(msg.variance);
03:46:44       |                                                                              ^~~~~~~~

I'll investigate it.

jolting commented 11 months ago

This was added on March 15. It didn't make Humble. https://github.com/ros2/common_interfaces/commit/05d7e1942acffefbeb40f7dd9d69ce4298119dcf

jolting commented 11 months ago

Maybe try it with declval?

template <class T>
struct has_variance<T, std::void_t<decltype(std::declval<T>().variance)>> : std::true_type
{
};
jlblancoc commented 11 months ago

This was added on March 15. It didn't make Humble. ros2/common_interfaces@05d7e19

Yeap, exactly! :-)

jlblancoc commented 11 months ago

@jolting It was something else... I just found (the hard way..) that everything inside an if constexpr() is actually parsed by the compiler (even if the condition is false)... except if it's inside a template. Solution -> move that part to a template, and it's now working (ignoring) on humble too! https://github.com/MRPT/mrpt/commit/4b6e10895848a1c66e12d26e63dad4e3ebb52bc8

PS: before that, I also tried the declval trick without luck... :-( Thanks!!