Add Graph description messages to rosgraph_msgs#188
Add Graph description messages to rosgraph_msgs#188emersonknapp wants to merge 3 commits intorollingfrom
Conversation
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
dc66d05 to
7fea0f1
Compare
skyegalaxy
left a comment
There was a problem hiding this comment.
seems like pretty much exactly what was discussed at this week's ROSgraph WG
skyegalaxy
left a comment
There was a problem hiding this comment.
need to add DCO for the last commit
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
31aa0e1 to
0892e87
Compare
|
Note for reviewers - I'm updating the docstrings with more information, and calling out the change here. I was trying to use these messages to represent both the "abstract descriptor" of these entities, as well as the "observed/observable instantiation" of them. However, those two concepts have some key disjunctions. Therefore, I am focusing in on the latter - observed/observable instantiation. We will use NoDL for the "abstract descriptor" and may or may not choose to be able to represent it as messages, which will be a separate exercise. Thanks @fujitatomoya for taking the close look that forced this decision |
…ns specifically to observed/actual state, rather than trying to fold in an abstract descriptor meaning Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
There was a problem hiding this comment.
I wonder if instead of a structure here we should just allow this to be the stringified representation of the type hash that we show in the ros2 cli, etc. Theoretically a RIHS v2 wouldn't have to have the same length, so the [32] fixed size below wouldn't necessarily provide the flexibility to communicate that.
(hightly doubt we ever define RIHS02, but hey, we allowed for it explicitly)
Could just do non-fixed-size array, which would be half the size of sending stringified hexadecimal.
There was a problem hiding this comment.
I personally would err on the side of the fixed size array for the type hash and just increase the size when we actually need to, and feel less inclined to allow for a variable length string. Part of that is because of the known issues with rosidl / zero copy and variable length data, but maybe zero-copy isn't going to matter anyway if there are other string fields inside of the message. It could also be the case someday that as a result of these discussions, strings in ROS messages will end up with a max character length before being dropped as the implicit upper bound of a RIHSv2 length.
There was a problem hiding this comment.
there are many string fields in here 😅
| Service[] service_servers | ||
|
|
||
| Action[] action_clients | ||
| Action[] action_servers |
There was a problem hiding this comment.
Would these lists allow for duplicates, say for example if a node spins up multiple (action) clients to the same (action) server for whatever reason?
There was a problem hiding this comment.
nothing prevents it. based on the current representation you wouldn't be able to tell them apart though.
I'm now wondering if I need to basically have the leaf message actually be an encoding of rmw_topic_endpoint_info (and service_endpoint_info), and include the GID as well.
That would directly represent the "observable state" as it's defined most fundamentally in the API, if we try to match those definitions.
There was a problem hiding this comment.
I personally would err on the side of the fixed size array for the type hash and just increase the size when we actually need to, and feel less inclined to allow for a variable length string. Part of that is because of the known issues with rosidl / zero copy and variable length data, but maybe zero-copy isn't going to matter anyway if there are other string fields inside of the message. It could also be the case someday that as a result of these discussions, strings in ROS messages will end up with a max character length before being dropped as the implicit upper bound of a RIHSv2 length.
Description
Fixes #182
Add new message definitions to
rosgraph_msgsthat can be used to describe a running or potential ROS Graph.Graph: a list of nodesNode: describe a single node's parameters and interface endpointsTopic: describe a single topic endpoint, either Publisher or SubscriptionService: describe a single service endpoint, either Server or ClientAction: describe a single action endpoint, either Server or ClientQoSProfile: describe a QoS Profile, either offered or requestedInterfaceType: identify arosidltypeTypeHash: identify a REP-2016 RIHS type hash (rosidl_runtime_cdefined type)This is intended to supplant the definitions in use at https://github.com/ros-tooling/graph-monitor/tree/main/rosgraph_monitor_msgs
Is this user-facing behavior change?
Yes
Did you use Generative AI?
No
Additional Information
Part of the NoDL and rosgraph_monitor work being done within the ROSGraph Working Group.
We would like to get these messages into the core for Lyrical, so that we can build tools around them for release into that LTS distro.
These messages act as a counterpart to the NoDL schema, being used to output either a desired or observed state of a ROS application as a message between nodes. The NoDL schema is not a strict duplicate of the information that these messages will contain, because: