Use builtin_interfaces/Time for TransitionEvent stamp

[SuperJappie08]

Description

Use builtin_interfaces/Time for the timestamp in lifecycle_msgs/TransitionEvent

Fixes #184

Is this user-facing behavior change?

Yes, the field name has been changed to be more in line with other uses (stamp instead of timestamp).
Furthermore, the type has been changed from uint64 to builtin_interfaces/Time to allow it to be more easily used by preexisting tooling.

The impact will be minimal, since the timestamp field is currently not being filled in rcl_lifecycle (ros2/rcl#1019).

Did you use Generative AI?

No.

Additional Information

[fujitatomoya]

as @SuperJappie08 mentions in issue header, this is a breaking change because it changes the topic message type. but until now, timestamp has never been used. that said, we can assume that there are no application that actually relies on this timestamp field.

i see that there could be almost no affection for the user application. but i would like to confirm with other PMC members on this.

[wjwwood]

The only thing I would consider is whether or not we need a timestamp in this message at all? There is the sent and received timestamps for all messages, and usually we only include a timestamp in a message if it is representing data that may have happened in the past and there is an impactful delay in sending the message, or if we think the message will be relayed (and therefore have the sent timestamp overwritten by the second send).

[tfoote]

Changing the type to be consistent makes sense to me. I think that this field has a legacy that predates builtin_interfaces/Time when we started with the uint64 nanoseconds representation more broadly. Especially since this field has not been being populated this seems like the right time to update the type and then fill in the implementation with the new type.

I agree with William though that I'd bias towards keeping the name so that it's a smaller change. We have the shorthand in Header but I think that the full variable name is valuable without the header context. stamp is somewhat overloaded for a standalone field.

[fujitatomoya]

@wjwwood @tfoote thanks for the comments.

i now also think having timestamp field is redundant, at least i do not find it useful for the application.

@SuperJappie08 what do you think? probably we could just remove this field instead?

[gavanderhoorn]

The only thing I would consider is whether or not we need a timestamp in this message at all?

Might be interesting to compare this to some sw-defined fieldbuses, such as OPC-UA.

In those systems, the approach is often to have different stamps for different 'aspects' of what happened to a message. So a timestamp for when an event occurred, one for when the message was sent (which is not necessarily always when the event happened, nor would it necessarily be sent by the source of the event) and sometimes even a stamp to record when it was received (which again might not always be when the consumer gets its hands on it).

With that in mind I could see a use for this additional timestamp field here.

[SuperJappie08]

i now also think having timestamp field is redundant, at least i do not find it useful for the application.

@SuperJappie08 what do you think? probably we could just remove this field instead?

An application I have/had is to get the exact moment of a transition as a time reference for a measurement that is controlled by a lifecycle node.
I need exact timing since it is embedded hardware and control related, but since it isn't a real-time system, I'm dependent on accurate timestamps.

[wjwwood]

Those sound like reasonable use cases for a timestamp in the message (since there's no plan at this time to have a "created" timestamp built-in to the middleware), so leaving it is fine. I just wanted to check if it was needed (since it seems to have gone so long being unused).

Also, as I said I'm not too worried about the field name. stamp is more consistent with other parts of the stack, but changing as little as possible is best in general when changing messages. Either should be ok.

[fujitatomoya]

a timestamp for when an event occurred, one for when the message was sent (which is not necessarily always when the event happened, nor would it necessarily be sent by the source of the event) and sometimes even a stamp to record when it was received (which again might not always be when the consumer gets its hands on it).

i think this makes sense to me. i was thinking that rmw_message_info as following can cover this requirement, that is the major reason i do not find the use of this timestamp field in this message.

...<snip>
publication_sequence_number: 4
publisher_gid:
  data: !!binary |
    AQ8E9kv3XYQAAAAAAAATAw==
  implementation_identifier: rmw_fastrtps_cpp
received_timestamp: 1761111785570276567
reception_sequence_number: null
source_timestamp: 1761111785570084585
---
timestamp: 0
transition:
  id: 0
  label: ''
start_state:
  id: 13
  label: activating
goal_state:
  id: 3
  label: active
---
...<snip>

but as @gavanderhoorn mentioned, source and receive timestamp is different when the event takes place.

and @SuperJappie08 thanks for iterating with us. i will go ahead to run CI.

[fujitatomoya]

Pulls: #185, ros2/rcl#1269, ros2/rclcpp#2967, ros2/rclpy#1528
Gist: https://gist.githubusercontent.com/fujitatomoya/43f97e6f8303a55ef1fdd1c645c51dd1/raw/feab0bba51e7c64b9f07536c61c03781c6dcd1e2/ros2.repos
BUILD args: --packages-above-and-dependencies lifecycle_msgs rcl_lifecycle rclcpp_lifecycle rclpy
TEST args: --packages-above lifecycle_msgs rcl_lifecycle rclcpp_lifecycle rclpy
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/17335

• Linux
• Linux-aarch64
• Linux-rhel
• Windows