feat(graph): add EndpointKind::ActionServer and typed wait_for_action_server#194
Open
YuanYuYuan wants to merge 3 commits into
Open
feat(graph): add EndpointKind::ActionServer and typed wait_for_action_server#194YuanYuYuan wants to merge 3 commits into
YuanYuYuan wants to merge 3 commits into
Conversation
…_server Previously `wait_for_action_server` matched action server readiness by constructing five hard-coded `_action/send_goal` / `_action/feedback` / etc. strings and querying the graph individually. This breaks silently if the sub-topic naming convention ever changes, and it conflates "a service named X exists" with "an action server for X is ready". Changes: - Add `ActionServer` and `ActionClient` synthetic variants to `EndpointKind` in hiroz-protocol; add centralised `ACTION_SERVER_*_SUFFIXES` constants and `action_name_from_topic()` helper - Extend `GraphData` with a `by_action` index; during `parse()`, action sub-endpoints (send_goal/get_result/cancel_goal services + feedback/status publishers) are additionally keyed by action name - Add `Graph::has_action_server()` that checks all five sub-endpoints are present using the index, and `count(ActionServer, …)` for consistency - Replace the five-string heuristic in `wait_for_action_server` with a single call to `has_action_server()` - Update exhaustive match arms in event.rs, bridge.rs, and ros2dds.rs - Add negative test: `wait_for_server` returns false when no server exists
…ulate by_action in add_local_entity Two correctness issues found in code review: 1. EndpointKind::FromStr accepted "AS"/"AC" despite ActionServer/ActionClient being synthetic graph-level kinds that never appear on the wire. A peer emitting these codes would produce a phantom entity stored in `parsed` but indexed nowhere, invisible to all graph queries. Removed the "AS"/"AC" arms so the parser returns Err, consistent with the doc comment. 2. add_local_entity did not populate by_action, so a same-process action server was invisible to has_action_server until the Zenoh liveliness echo arrived. The old wait_for_action_server queried by_service/by_topic directly (both populated by add_local_entity), so this was a silent regression. Mirrored the is_action_server_endpoint indexing block from parse() into add_local_entity to make same-process detection immediate.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
EndpointKind::ActionServerandActionClientsynthetic variants tohiroz-protocol, with centralised action sub-topic suffix constants and anaction_name_from_topic()helperGraphDatawith aby_actionindex populated during liveliness parsing — action server sub-endpoints (three services + two publishers) are keyed by action nameGraph::has_action_server()that checks all five sub-endpoints are present, and exposescount(EndpointKind::ActionServer, …)for consistency_action/send_goal/_action/feedbacketc. string checks inwait_for_action_serverwith a single typedhas_action_server()callevent.rs,bridge.rs, andros2dds.rswait_for_serverreturnsfalsewhen no server is presentKey Changes
crates/hiroz-protocol/src/entity.rs— newActionServer/ActionClientvariants, suffix constants,action_name_from_topic()crates/hiroz/src/graph.rs—by_actionindex,visit_by_action(),has_action_server(),count(ActionServer, …)crates/hiroz/src/graph.rs—wait_for_action_serversimplified to one callcrates/hiroz/tests/action/client.rs— newtest_action_client_wait_for_server_no_serverBreaking Changes
None.
EndpointKindgrows two new variants; all existing match arms that were_/ wildcard-terminated are unaffected. The two new arms added to exhaustive matches (bridge.rs,event.rs,ros2dds.rs) preserve existing behaviour.