ros-drivers / rosserial

A ROS client library for small, embedded devices, such as Arduino. See: http://wiki.ros.org/rosserial
508 stars 527 forks source link

Fix/time #497

Closed rikba closed 4 years ago

rikba commented 4 years ago

In https://github.com/ros-drivers/rosserial/issues/169 @PaulBouchier mentions that the implementation of subtraction and addition of time and duration may be erroneous. We actually observed that

ros::Time time = {1, 0};
time -= ros::Duration(0, 1000000);

indeed lead to a wrong result on our system.

This PR implements the fix provided by @chuck-h and additional unit tests.

Since I cannot run the unit tests on my machine I'm creating this draft PR first.

rikba commented 4 years ago

Ok, I was hoping Travis has a different workspace setup than I do, but I get the same redefinition error of ros::Duration here.

Can someone help me? How do I #include "ros_lib/ros/duration.h" in the unit test without redefining ros::Duration?

In file included from /home/travis/catkin_ws/src/rosserial/rosserial_client/test/time_test.cpp:2:

In file included from /home/travis/catkin_ws/src/rosserial/rosserial_client/src/ros_lib/ros/time.h:38:

/opt/ros/melodic/include/ros/duration.h:108:20: error: redefinition of

      'Duration'

class ROSTIME_DECL Duration : public DurationBase<Duration>

                   ^

/home/travis/catkin_ws/src/rosserial/rosserial_client/src/ros_lib/ros/duration.h:46:7: note: 

      previous definition is here

class Duration

      ^

[ 96%] Built target rosserial_test_publish_subscribe

1 error generated.

rosserial/rosserial_client/CMakeFiles/time_test.dir/build.make:62: recipe for target 'rosserial/rosserial_client/CMakeFiles/time_test.dir/test/time_test.cpp.o' failed

make[3]: *** [rosserial/rosserial_client/CMakeFiles/time_test.dir/test/time_test.cpp.o] Error 1

CMakeFiles/Makefile2:2431: recipe for target 'rosserial/rosserial_client/CMakeFiles/time_test.dir/all' failed

make[2]: *** [rosserial/rosserial_client/CMakeFiles/time_test.dir/all] Error 2

CMakeFiles/Makefile2:77: recipe for target 'CMakeFiles/tests.dir/rule' failed

make[1]: *** [CMakeFiles/tests.dir/rule] Error 2

Makefile:175: recipe for target 'tests' failed

make: *** [tests] Error 2

Invoking "make tests -j2 -l2" failed

The command "catkin_make tests" exited with 1.
mikepurvis commented 4 years ago

How do I #include "ros_lib/ros/duration.h" in the unit test without redefining ros::Duration?

You figured out one way. Another is to wrap the include in a namespace to isolate it from its surroundings; we do this for the rosserial_test node, as it is a single binary that is simultaneously a rosserial client and a roscpp node:

https://github.com/ros-drivers/rosserial/blob/noetic-devel/rosserial_test/src/publish_subscribe.cpp#L4-L7