Fill in message_info timestamps

[iluetkeb]

This fills the timestamps in the message_info.

[wjwwood]

I'm curious why you changed this, to not have the received timestamp? Does it matter so long as it is consistent within a single implementation?

I don't really mind either way, but it seemed like the received timestamp was more useful than wrong.

[iluetkeb]

I'm curious why you changed this, to not have the received timestamp?

The timestamps are intended to see which event came first. However, when you just take system time at the moment of calling rcl_take_request, the timestamps will merely reflect the call order and so carry no value (you could have gotten the same result by calling it yourself just prior to the call).

In contrast, Fast-RTPS since 1.10 takes a real received timestamp when it receives the data and stores it in the SampleInfo, from where the rmw_fastrtps implementation of take_request copies it.

For this reason, I think it is important to communicate to the user whether the middleware really supports a genuine received timestamp or not. 0 is used to signify that it doesn't.

[eboasson]

Thanks @iluetkeb for helping with implementing this also for cyclone dds, it really makes a difference to me. I just wanted to add a suggestion on how to get a receive time stamp without having to rely on Cyclone to provide it.

The RMW layer relies on a custom sample implementation to perform direct ROS2-to-CDR conversion and to get access to the serialized form for rmw_take_serialized_message and any received message will be turned into a sample instance as soon as it is ready to be inserted in the reader history caches. If that’s what you mean by “the receive time stamp” (as I pointed out in ros2/design#259 (comment) there are very many versions), then one way to do it would be to store that time stamp in this custom sample: just add it to

rmw_cyclonedds/rmw_cyclonedds_cpp/src/serdata.hpp

Lines 48 to 61 in 2638f4a

	class serdata_rmw : public ddsi_serdata
	{
	protected:
	size_t m_size {0};
	/* first two bytes of data is CDR encoding
	second two bytes are encoding options */
	std::unique_ptr<byte[]> m_data {nullptr};
	public:
	serdata_rmw(const ddsi_sertopic * topic, ddsi_serdata_kind kind);
	void resize(size_t requested_size);
	size_t size() const {return m_size;}
	void * data() const {return m_data.get();}
	};

It won’t be returned in the sample info because cyclone currently only implements the fields defined in the standard, but that can be worked around by using dds_takecdr instead of dds_take, like when taking a serialized message:

rmw_cyclonedds/rmw_cyclonedds_cpp/src/rmw_node.cpp

Line 2178 in 2638f4a

while (dds_takecdr(sub->enth, &dcmn, 1, &info, DDS_ANY_STATE) == 1) {

That gives you a reference to the sample that you can then deserialize by calling ddsi_serdata_to_sample on it. That way you get everything.

(I don’t know what the rules are for loaned samples, but with this sample implementation, and for those topics where the CDR representation matches the in-memory representation and where there are no endianness issues, the same trick would be feasible.)

[iluetkeb]

Thanks @eboasson, I've now put this into an issue for future reference: #167