Ros2 migration behavior/arm

[ana-GT]

Conversion was straightforward, with the exception of a few issues that I marked TODO:

1. arm/tools/arm_tool.cc, line 168: The arm_tool is initialized using the AnonymousName option, which is not yet available in ROS2, although there seems to be active work on it (as of this week).
2. arm/src/arm_nodelet.cc, lines 491-493: Some functions were not ported, as they no longer seem applicable in ROS2, such as GetPrivateHandle(), Listen function for reconfiguring parameters and its corresponding reconfigure_callback function.
3. arm/src/arm_nodelet.cc: Line 546: A subscriber for TOPIC_JOINT_GOALS is created, originally with latch=true, however this topic is not in the LatchedTopic list, so if we use the FF_CREATE_SUBSCRIBER, the topic will not be latched.

[trey0]

1. arm/tools/arm_tool.cc, line 168: The arm_tool is initialized using the AnonymousName option, which is not yet available in ROS2, although there seems to be active work on it (as of this week).

As far as I understand, the AnonymousName option appends a random number to the end of the node name at initialization time, which we could do manually at [1]. I see you also changed the node name from "control" to "arm_tool", which seems like an improvement.

[1]

astrobee/behaviors/arm/tools/arm_tool.cc

Line 195 in 939adfe

auto nh = std::make_shared<rclcpp::Node>("arm_tool");

[trey0]

3. arm/src/arm_nodelet.cc: Line 546: A subscriber for TOPIC_JOINT_GOALS is created, originally with latch=true, however this topic is not in the LatchedTopic list, so if we use the FF_CREATE_SUBSCRIBER, the topic will not be latched.

@marinagmoreira and @kbrowne15 recently merged a3ae212 on develop, which makes this publisher non-latching.

I'm not sure why it was originally marked latching. The latching option would have the effect of automatically resending the joint goals if somehow the arm nodelet joined late after the goals were originally published, but I don't think that's really desired (could lead to an unintended "zombie command").

[marinagmoreira]

So I tested the arm behavior in simulation and it works. But let's hold on merging this until we make a decision on the services since it affects it.
I commented out the services test because as a gtest, it's really hard to test it since the server and client are started from the same main, and that opens a can of worms for the sync service problem. That's why the service test passed in the past, even if we couldn't use it for components.
I'll keep on tinkering with the service test to see if I can make it work with a multithreaded executor since we might want to have the server and client under the same container and if this doesn't work, that's just problems down the line.. just don't want to sink even more time into it :(..

[marinagmoreira]

According to the analysis in #628, this does not need to be latched

[trey0]

Yes! See my comment above. This topic is no longer latching in develop, although maybe it was latching on whatever baseline version @ana-GT started from. Probably the correct action is to remove the comment.

[bcoltin]

Thanks Ana and Marina!

[bcoltin]

Now that we aren't using tol (I'm pretty sure) except to set it but never look at it, I would just delete it from JointInfo to make things more clear.

[trey0]

That sounds like a change to apply/test on develop first?

[bcoltin]

I thought it only applies here due to changes in how the parameter server works, but correct me if I'm wrong Marina.

[marinagmoreira]

In order to maintain functionality with the previous version the tol with GetTolerance() still only gets updated in certain state modes, which maintains the need for the tol variable :)