ros2 / ros1_bridge

ROS 2 package that provides bidirectional communication between ROS 1 and ROS 2
Apache License 2.0
421 stars 274 forks source link

[Humble] Building Failed #393

Open Ogunniran opened 1 year ago

Ogunniran commented 1 year ago

Bug report

Note: It built properly after c2fa9ed11d1be6cb36441ad317d0797deb4714d9 was removed.

Required Info:

Building the ros1_bridge with colcon build --symlink-install --packages-select ros1_bridge --cmake-force-configure failed with the error:

//home/x/ros1_bridge/ros_bridge/build/ros1_bridge/generated/...__factories.cpp: In instantiation of ‘void ros1_bridge::streamPrimitiveVector(ros::serialization::OStream&, const VEC_PRIMITIVE_T&) [with VEC_PRIMITIVE_T = std::vector<bool, std::allocator<bool> >]’:
/home/x/ros1_bridge/ros_bridge/build/ros1_bridge/generated/...__factories.cpp:830:24:   required from here
/home/x/ros1_bridge/ros_bridge/build/ros1_bridge/generated/...__factories.cpp:694:46: error: lvalue required as unary ‘&’ operand
  694 |   memcpy(stream.advance(data_len), &vec.front(), data_len);
      |                                     ~~~~~~~~~^~
/home/x/ros1_bridge/ros_bridge/build/ros1_bridge/generated/...__factories.cpp: In instantiation of ‘void ros1_bridge::streamPrimitiveVector(ros::serialization::IStream&, VEC_PRIMITIVE_T&) [with VEC_PRIMITIVE_T = std::vector<bool, std::allocator<bool> >]’:
/home/x/ros1_bridge/ros_bridge/build/ros1_bridge/generated/...__factories.cpp:1063:24:   required from here
/home/x/ros1_bridge/ros_bridge/build/ros1_bridge/generated/...__factories.cpp:711:20: error: taking address of rvalue [-fpermissive]
  711 |   memcpy(&vec.front(), stream.advance(data_len), data_len);
      |           ~~~~~~~~~^~
/home/x/ros1_bridge/ros_bridge/build/ros1_bridge/generated/...__factories.cpp:711:9: warning: ‘void* memcpy(void*, const void*, size_t)’ writing to an object of type ‘std::vector<bool, std::allocator<bool> >::reference’ with no trivial copy-assignment; use copy-assignment or copy-initialization instead [-Wclass-memaccess]
  711 |   memcpy(&vec.front(), stream.advance(data_len), data_len);

Additional information


One of the ROS2 msgs which triggered the error has a similar format:

std_msgs/Header header
# foo
string[] foo_1
float64[] foo_2
float64[] foo_3
float64[] foo_4
int32[] foo_5
float64[] foo_6
int32[] foo_7
bool[] foo_8
bool[] foo_9

int32[] foo_x
int32[] foo_y
clalancette commented 1 year ago

How are you using Noetic on Ubuntu 22.04? Did you build from source.

While this should probably work, this isn't currently a supported configuration, so we may not be able to help here.

Ogunniran commented 1 year ago

Ok, thanks for the reply. Yes it was built from source.

Ogunniran commented 1 year ago

The issue still occurs while building with rolling and noetic, using the docker image.

alexg-k commented 1 year ago

Hi, I have the same setup and the same error message as described above. Just want to add that this error seems to be introduced with the latest commits. When I checkout the commit ID b9f1739fd84fc877a8ec6e5c416b65aa2d782f89 for example and build the otherwise untouched project again, it runs fine without the error from above.

Timple commented 1 year ago

This is great!

This commit also 'fixes' #391

oKermorgant commented 1 year ago

Hi,

I have just gone into the same issue. My guess is that vector<bool> is a bad class and is not a vector of bool.

As a consequence, taking the address of the first element does not work as it should, because there are no bools inside a vector<bool>. Returning by value is fine but a reference is not possible.

I have written a quick fix, need to test it before submitting a PR.