Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add diagnostic_msgs/Heartbeat message #179

Draft
wants to merge 2 commits into
base: noetic-devel
Choose a base branch
from
Draft

Add diagnostic_msgs/Heartbeat message #179

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ce5bed1
Added diagnostic_msgs/Heartbeat message
peci1 Apr 28, 2022
e697a14
Added diagnostic_msgs/Heartbeat message.
peci1 Apr 28, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions diagnostic_msgs/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ add_message_files(
FILES
DiagnosticArray.msg
DiagnosticStatus.msg
Heartbeat.msg
KeyValue.msg)

add_service_files(
Expand Down
4 changes: 4 additions & 0 deletions diagnostic_msgs/msg/Heartbeat.msg
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# This message can be for example sent along with a large message (like point cloud)
# to allow another node to measure publishing frequency and delay without subscribing
# to the large message itself.
Header header
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that a header is appropriate. The frame_id is irrelevant here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also what is the policy for the timestamp data? It's presumably important to set that on publish?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's the thing. If you put just stamp there, you have to write all the analytics tools from scratch. If you put there Header, you can rostopic hz, you can add TimeStampStatus diagnostics etc. right away.

And I wouldn't say frame_id is irrelevant. Consider e.g. a topic where you mix messages from multiple sensors and differentiate between them via frame_id (not saying this is optimal, but it is a supported case IMO). Yes, rostopic hz would lose sense on such a topic, but it would still be super-easy to write some kind of demux node which would then provide several streams of diagnostic data, one for each frame_id.

Yes, maybe it should be explicitly specified that stamp should be just whatever stamp was put in the parallel data stream (if there is any), or the publish time (if used "standalone").