Skip to content

Populate type hashes in zenoh graph cache (#356) - #1004

Open
tisomi wants to merge 1 commit into
ros2:rollingfrom
tisomi:rolling
Open

Populate type hashes in zenoh graph cache (#356)#1004
tisomi wants to merge 1 commit into
ros2:rollingfrom
tisomi:rolling

Conversation

@tisomi

@tisomi tisomi commented Jul 2, 2026

Copy link
Copy Markdown

Description

This PR populates the new type_hashes field on rmw_names_and_types_t from the zenoh
graph cache, so rmw_zenoh reports the type hash of each name/type pair through the graph
query APIs. It is the rmw_zenoh counterpart to the interface change in ros2/rmw#356, and
mirrors the conflict-detection behavior of the rmw_dds_common companion PR.

Key changes:

  • Adds two private helpers in graph_cache.cpp:
    • _type_hashes_equal compares two rosidl_type_hash_t values (version plus a
      byte-by-byte std::memcmp of the value).
    • _resolve_type_hash_for_type walks the per-type QoS map and reconciles the hashes.
      The zenoh cache stores each endpoint's hash as a serialized string
      (TopicData::info_.type_hash_) and keeps one TopicData per QoS/endpoint, so the hash
      is resolved at read time: each string is parsed with rosidl_parse_type_hash_string,
      unparseable entries are skipped, and if all successfully-parsed hashes agree that hash
      is returned. If they disagree, a WARN is logged and a zero-initialized hash is returned
      so the caller sees an unambiguous "no consensus" value. See Additional Information
      for why diverging hashes can occur.
  • fill_names_and_types allocates the per-name type_hashes array (or nullptr when a
    name has no types) and fills each entry via _resolve_type_hash_for_type, alongside the
    corresponding type name. Its existing rcpputils::make_scope_exit cleanup (cancelled on
    success) already finalizes the partially-built struct via rmw_names_and_types_fini on
    any early return.

Adds gtests covering pub/sub topic hashes, per-node publisher and subscriber hashes, and
topic endpoint info hashes, plus the equivalent set for services (server/client and service
endpoint info).

Related: ros2/rmw#356

Is this user-facing behavior change?

Yes.

  • The graph query APIs (rmw_get_topic_names_and_types, rmw_get_service_names_and_types,
    and the per-node variants) now report the type hash of each name/type pair via the new
    type_hashes array.
  • When endpoints disagree on the hash for the same (topic, type) pair, the reported hash is
    a zero hash and a WARN is logged.
  • No public signature changes in this package; it populates the existing output field. It
    requires the ABI-breaking struct change from Extend names_and_types to contain type hashes rmw#356.

Did you use Generative AI?

Yes. Claude (claude-sonnet-4-6) via GitHub Copilot was used to explore the codebase and
create an initial prototype.

Additional Information

  • Depends on the interface PR ros2/rmw#421, which adds the type_hashes field and hardens
    rmw_names_and_types_init / rmw_names_and_types_fini. The scope-exit cleanup here relies
    on fini cleaning up partially-populated structs without leaking.
  • Companion PR: rmw_dds_common #89 "Populate type hashes in graph cache names_and_types". The
    conflict handling (zero hash + WARN) is intentionally identical across the two.

On the hash-conflict case. A single topic name can appear with more than one type in
the ROS graph, which is why rmw_names_and_types_t stores an array of types per name.
Beyond that, two entries can share the same type name while the type definitions behind
them differ structurally — different versions of the same interface — and therefore produce
different hashes. This "version drift" arises when nodes on the graph are built against
different versions of the same type definition. When it happens, the graph query reports a
zero hash for that type and logs a WARN. In a version-consistent system this should not
occur, but the graph layer has to surface it rather than hand the caller one endpoint's
hash as if it were authoritative.

Testing

  • Ran the package unit tests (including the new session-level gtests in test_rmw_zenoh_cpp)
    in a Docker container; all pass.
  • Implemented and successfully ran an integration test exercising the end-to-end type-hash
    path through the graph query APIs.

This is a SIEMENS activity and was initialized by @gramss

Populates the new type_hashes field on rmw_names_and_types_t from the
zenoh graph cache. Adds two private helpers:

  - _type_hashes_equal compares two rosidl_type_hash_t values
    (version + byte-by-byte via std::memcmp).
  - _resolve_type_hash_for_type walks the per-type QoS map; if all
    successfully-parsed hashes agree it returns that hash, otherwise it
    logs a WARN and returns a zero-initialized hash so the caller sees
    an unambiguous "no consensus" value. Matches the rmw_dds_common
    conflict-detection behavior.

fill_names_and_types now allocates the per-name type_hashes array (or
sets it to nullptr when there are no types) and fills each entry via
_resolve_type_hash_for_type so callers of the rmw graph query APIs see
type hashes alongside names and types.

Adds gtests covering pub/sub topic hashes, per-node publisher and
subscriber hashes, topic endpoint info hashes, plus the equivalent set
for services (server/client + service endpoint info).

Related: ros2/rmw#356

Signed-off-by: tisomi <titus090404@gmail.com>
@mergify

mergify Bot commented Jul 2, 2026

Copy link
Copy Markdown

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

@wjwwood wjwwood left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I requested changes to the rmw interface pr (intrinsic-ai/sdk-ros#138)

@gramss

gramss commented Jul 26, 2026

Copy link
Copy Markdown

Thanks for the feedback @wjwwood !
We'll look into it next week.
I think that your ref links here and in the companion PR (ros2/rmw_dds_common#89) should be linking to ros2/rmw#421 instead of intrinsic-ai/sdk-ros#138 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants