[WIP] uORB::Publication automatically set timestamp #13970
Conversation
dagar
force-pushed
the
pr-uorb_timestamp
branch
from
c545256 to
df1a945
Compare
|
@dagar what are we missing here to bring it in? |
| * @param timestamp | ||
| * @param data The uORB message struct we are updating. | ||
| */ | ||
| bool publish(const hrt_abstime ×tamp, T &data) |
There was a problem hiding this comment.
Why not instead
| bool publish(const hrt_abstime ×tamp, T &data) | |
| bool publish(T &data, hrt_abstime ×tamp = hrt_absolute_time()) |
| * Publish the struct | ||
| * @param data The uORB message struct we are updating. | ||
| */ | ||
| bool publish(const hrt_abstime timestamp, T &data) |
Finding out why the tests are broken and carefully reviewing special cases where setting the timestamp manually has mattered historically.
Note, this isn't the complete solution, but I see it as an incremental step that brings us closer. I ultimately want the timestamp to be set in the same critical section where the published data is copied into the buffer. I want the publication timestamp to be as close as possible to the actual time it is available to subscribers in the system. EDIT: If we can guarantee that |
What do we gain from pushing to |
We get a trustworthy timestamp that is actual publication metadata and can't be screwed up.
I don't think so. We might have some side mechanism for very special cases like replay that might need to set it manually, but for everything else we can rely on uORB to do it perfectly. |
|
This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions. |
WORK IN PROGRESS
The eventual goal is the
timestampfield of a uORB messsage to be metadata that corresponds (atomically) with the message being published and available to the rest of the system. This is an incremental step in that direction that removes a lot of the current potential misuse. Things like setting the timestamp to an inappropriate value (like a much older sample timestamp passed through), setting it much earlier than actual publication, or even just forgetting to set it at all.Ideally this would be pushed into the uORB::DeviceNode internals and the timestamp set in the same critical section as the message copy, however we're currently in an odd position with the timestamp not strictly required in every message. This needs to be revisited with ROS2 taken into account, including the possibility of directly into a
std_msgs/Headerequivalent.For example in #13955 the accel and gyro sensor message are updated to have a dedicated
timestamp_samplefield that we set as closely as possible to time corresponding to the contained data.