Fix the GoalUUID to_string representation

[nwn]

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.)

[fujitatomoya]

I think this PR is reasonable to me, but format should be consistent in language frontend libraries for debugging use case? (probably goal id is likely to be used with grep)

[clalancette]

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.

[nwn]

@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.

[nwn]

@fujitatomoya @clalancette Are there any further concerns about this or action required to get this merged?

[fujitatomoya]

Just FYI, I see https://github.com/ros2/geometry2 uses this method, but all issued for debug print.

[clalancette]

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.

[clalancette]

CI:

• Linux
• Linux-aarch64
• Windows

[clalancette]

CI is green with this, so going ahead and merging. Thanks for the contribution!