Add timestamp to rmw_publish tracepoint #454
Conversation
cwecht
force-pushed
the
extend_rmw_publish_tracepoint
branch
from
45ee72d
to
fb1393c
Compare
rmw_cyclonedds_cpp/src/rmw_node.cpp
Outdated
| // the following wouldn't work since dds_writecdr resets the timestamp anyways. | ||
| // const dds_time_t tstamp = dds_time(); | ||
| // d->timestamp.v = tstamp; | ||
| TRACETOOLS_TRACEPOINT(rmw_publish, (const void *)publisher, ros_message, 0); |
There was a problem hiding this comment.
This is a bit unfortunate. As of now dds_writecdr will always overwrite the timestamp.
Will the serdata object outlive the dds_writecdr function call? If so, we could maybe also take the timestamp from it after the fact.
There was a problem hiding this comment.
Ok, I had a closer look into cyclonedds and it looks like the serdata object does not outlive dds_writecdr if everything is fine.
@christophebedard I'd suggest to leave it like this for the time beeing. The only way I could image that this could be fixed is add the respective API to cyclonedds.
There was a problem hiding this comment.
After having a closer look at cyclones API, it turns out that there is an APi which seems to fit this usecase quite well: dds_forwardcdr. @eboasson and/or @ivanpauno, it would be great if you could have a look at these changes (again).
There was a problem hiding this comment.
@cwecht I'm catching up with whatever happened during my break ... I'll review as request but wanted to quickly note that this weird behaviour of dds_writecdr is the reason I needed to fix something ... and added dds_forwardcdr.
cwecht
force-pushed
the
extend_rmw_publish_tracepoint
branch
from
fb1393c
to
40cad3f
Compare
|
For the record: the linter warning revealed in the builds of ros2/rmw_connextdds#120 should be fixed now. |
Signed-off-by: Christopher Wecht <cwecht@mailbox.org>
cwecht
force-pushed
the
extend_rmw_publish_tracepoint
branch
from
40cad3f
to
fe934b5
Compare
Depends on ros2/ros2_tracing#74
For some background: ros2/ros2_tracing#73
Question: Has
dds_write_ts+dds_time()any drawbacks if compared with using plaindds_write?