ros2
Fix the GoalUUID to_string representation
Changes the string representation of rclcpp_action::GoalUUIDs to match the canonical format described in RFC 4122. (The current representation doesn't contain hyphens and omits the leading zero in the hex of a byte.)
I'm not necessarily opposed to changing it, but I would like to know the motivation (other than just conforming to the RFC). There are possible backwards-compatibility things to consider here.
@fujitatomoya I agree, we should use a consistent format across client libraries. Unfortunately, rclpy currently doesn't format goal IDs well either. For example, the rclpy action server prints them as arrays of integers:
[DEBUG] [1661282923.515954250] [minimal_action_server]: New goal request with ID: [231 15 208 8 188 196 68 43 129 63 56 102 30 126 48 197]
So it would be good to start standardizing this output.
@clalancette The issues with the current string representation aren't just aesthetic or pedantic. Omitting the leading zero in a byte is actually ambiguous. For example, the two UUIDs 01100000-0000-0000-0000-000000000000 and 11000000-0000-0000-0000-000000000000 will both be converted to the string 11000000000000000 in the current representation. If users are comparing the string values of these UUIDs or storing them in a hash table, this would cause collisions.
@fujitatomoya @clalancette Are there any further concerns about this or action required to get this merged?
Just FYI, I see https://github.com/ros2/geometry2 uses this method, but all issued for debug print.
I've rebased this onto the latest, and I've also done a bit more cleanup in here. In particular, I made the whole thing a little more efficient by using .resize instead of .reserve on the string, and by adding in the dashes in the main loop (rather than adding them in separately).
I also did a quick look around the code base and came to the same conclusion that Tomoya did; the only uses seem to be in the tests here and in debug messages in tf2_ros. So I think this is good to go, I'm going to go ahead and approve this and run CI.
That said, I did some work on this, so I'd appreciate another look from @fujitatomoya before I go ahead and merge.
CI is green with this, so going ahead and merging. Thanks for the contribution!