feat(hiroz-py): rclpy alignment — P1–P8 API improvements by YuanYuYuan · Pull Request #192 · ZettaScaleLabs/hiroz

YuanYuYuan · 2026-05-29T06:56:59Z

Summary

Eight rclpy-alignment changes to hiroz-py. hiroz keeps its reactive, no-spin model.

	Change
P1	`wait_for_service` / `wait_for_server` / `wait_for_subscription` — graph-polling discovery waits, replacing `time.sleep()`. `wait_for_server` polls the full five-endpoint action-server predicate, not the `send_goal` service alone
P2	`create_publisher` / `create_subscriber` detect rclpy's `(msg_type, topic)` argument order and raise a `TypeError` naming the fix
P3	`create_subscription` / `create_service` aliases on `ZNode`
P4	Codegen emits service grouping classes — `AddTwoInts.Request` / `.Response` via `__srvtype__`. A bare Request class is still accepted
P5	Typed exceptions: timeouts raise `hiroz_py.TimeoutError`, other failures `hiroz_py.HirozError`
P6	Optional `create_server(callback=fn)` push mode on a worker thread. Pull mode is unchanged when `callback` is omitted. Errors surface via `last_error` (read-and-clear); shutdown via `close()` or the context manager
P7	Codegen emits action grouping classes — `Fibonacci.Goal` / `.Result` / `.Feedback` via `__actiontype__`. `Result` and `Feedback` are optional in the `.action` format, so only the members that exist are emitted
P8	QoS policy constants (`ReliabilityPolicy.RELIABLE`, …) and the `qos=10` int-depth shorthand

Timeout classification uses the core's structured detector hiroz::error::is_timeout (#221, #225). No path matches on error strings.

Exception hierarchy

RuntimeError
└── HirozError
    ├── TimeoutError          (+ builtins.TimeoutError)
    ├── SerializationError    (declared, not raised)
    └── TypeMismatchError     (declared, not raised)

HirozError inherits RuntimeError, which these paths raised before P5. TimeoutError also inherits the builtin, which rclpy's Client.call raises. Both bases keep ported except clauses working. TimeoutError is therefore also an OSError, the same as rclpy.

P6 callback-server shutdown

The callback-mode server detaches its worker instead of joining it. A join from Drop runs with the GIL held. The worker needs that GIL to call the Python callback.

sequenceDiagram
    autonumber
    participant Py as Python (interpreter shutdown)
    participant Srv as CallbackServerState::drop
    participant W as Worker thread
    W->>W: request arrives, wait for GIL
    activate W
    Py->>Srv: deallocate (GIL held)
    activate Srv
    Srv--xW: join() — waits for worker
    Note over Py,W: worker waits for the GIL, drop holds it → deadlock
    deactivate Srv
    deactivate W

Drop for CallbackServerState only sets the stop flag, then returns. The worker holds an Arc on the server, so the queryable outlives the wrapper until the worker observes the flag. close() and __exit__ are the blocking paths: they hold a Python token, so they release the GIL while joining.

Breaking changes

	What changes	Who is affected	Before → after	Action
BC1	`ActionGoalHandle.get_result(timeout=...)` raises on timeout	Callers that test the result for `None`	returned `None` → raises `hiroz_py.TimeoutError`	Replace `if result is None:` with `except hiroz_py.TimeoutError:`
BC2	Malformed service and action names are rejected at construction	Callers passing empty path components, trailing slashes, or invalid components in a relative name	degraded silently → `ValueError` naming the input	Fix the name, or catch `ValueError`
BC3	Service grouping classes and bare Request classes validate `__hash__`	Callers passing a hand-written service class	degraded silently → `TypeError` when `__hash__` is missing or not a string; `ValueError` when the string is not a RIHS01 hash	Use a codegen-emitted class, or supply a valid `__hash__`
BC4	`timeout=` arguments reject negative, NaN and infinite values	Callers passing such values	Rust panic → `ValueError`	Pass a finite, non-negative float, or `None` to wait forever

BC1 makes get_result match ZClient.call, which already raised on timeout. BC3 is service-side only. On action classes __hash__ stays optional; an absent or malformed value still yields a zero hash.

Note

BC2 does not cover absolute names. qualify_topic_name validates components only on the relative and ~private branches, so /bad name is still accepted — see the absolute-name arm and #264. The Python-side check is resolve_service_name.

What fails without this

The additive API surface has no failing baseline. Its justification is migration cost: a ported rclpy script currently needs time.sleep() for discovery, the flat AddTwoIntsRequest name, and except RuntimeError in place of a typed timeout.

The behaviour changes do have observable failures without them:

Duration::from_secs_f64 panics on a negative, NaN or infinite timeout (checked_timeout rejects these first).
A callback-mode server deadlocks on interpreter shutdown against the GIL its callback needs (see the diagram above).
Fibonacci.Goal does not exist, because actions were never threaded through codegen.
Waiting on the send_goal service alone can return true while result, cancel, feedback and status are still undiscovered, so the next call fails (wait_for_action_server).

Evidence

33 checks green, 0 failed. New crates/hiroz-py/tests/test_rclpy_alignment.py holds 36 test functions, which parametrization expands to 40 collected cases. The regenerated hiroz_msgs_py/types/*.py are part of the diff, together with a new docs/bindings/python-migration.md.

Follow-ups

Known gaps. None blocks this PR; they bound what green CI proves.

	Gap	Tracked
G1	`qualify_topic_name` does not validate absolute names	#264
G2	`ZPayloadView` zero-copy tests have never run, because numpy is absent	#266
G3	`wait_for_service` polls at 50 ms rather than using liveliness	untracked
G4	Action servers have no push mode; `recv_goal` is pull-only	untracked
G5	Parameters, lifecycle and clock exist in the core crate but have no Python surface	untracked

github-actions · 2026-07-28T06:47:56Z

PR Preview Action v1.8.1
🚀 View preview at https://ZettaScaleLabs.github.io/hiroz/pr-preview/pr-192/
Built to branch `gh-pages` at 2026-08-14 19:47 UTC. Preview will be ready when the GitHub Pages deployment is complete.

Copilot

Pull request overview

Adds rclpy-aligned Python APIs while preserving hiroz’s reactive execution model.

Changes:

Adds discovery waits, aliases, callback services, typed errors, and QoS shorthand.
Adds generated service grouping classes and action grouping-class factory support.
Updates generated bindings, examples, tests, and migration documentation.

Reviewed changes

Copilot reviewed 30 out of 30 changed files in this pull request and generated 13 comments.

Show a summary per file

File	Description
`mkdocs.yml`	Adds migration guide navigation.
`docs/bindings/python.md`	Documents aligned APIs.
`docs/bindings/python-migration.md`	Adds rclpy migration guide.
`docs/bindings/python-codegen.md`	Documents grouping classes.
`crates/hiroz-py/tests/test_service.py`	Updates error assertions.
`crates/hiroz-py/tests/test_rclpy_alignment.py`	Adds alignment integration tests.
`crates/hiroz-py/tests/test_action.py`	Updates result-timeout expectations.
`crates/hiroz-py/src/traits.rs`	Extends erased publisher/server interfaces.
`crates/hiroz-py/src/service.rs`	Adds waits and callback servers.
`crates/hiroz-py/src/qos.rs`	Adds integer-depth QoS shorthand.
`crates/hiroz-py/src/pubsub.rs`	Adds subscription discovery waits.
`crates/hiroz-py/src/node.rs`	Adds grouping types and factory enhancements.
`crates/hiroz-py/src/graph.rs`	Adds graph polling helper.
`crates/hiroz-py/src/error.rs`	Adds structured call-error mapping.
`crates/hiroz-py/src/action.rs`	Adds action waits and typed failures.
`crates/hiroz-py/python/hiroz_py/__init__.pyi`	Updates public type stubs.
`crates/hiroz-py/python/hiroz_py/__init__.py`	Adds aliases and QoS constants.
`crates/hiroz-py/examples/topic_demo.py`	Uses subscriber discovery wait.
`crates/hiroz-py/examples/service_demo.py`	Uses grouped services and waits.
`crates/hiroz-py/examples/action_demo.py`	Uses action-server discovery wait.
`crates/hiroz-msgs/python/hiroz_msgs_py/types/type_description_interfaces.py`	Regenerates types and service grouping.
`crates/hiroz-msgs/python/hiroz_msgs_py/types/std_msgs.py`	Regenerates message definitions.
`crates/hiroz-msgs/python/hiroz_msgs_py/types/sensor_msgs.py`	Regenerates types and service grouping.
`crates/hiroz-msgs/python/hiroz_msgs_py/types/rcl_interfaces.py`	Regenerates parameter service groupings.
`crates/hiroz-msgs/python/hiroz_msgs_py/types/nav_msgs.py`	Regenerates navigation service groupings.
`crates/hiroz-msgs/python/hiroz_msgs_py/types/lifecycle_msgs.py`	Regenerates lifecycle service groupings.
`crates/hiroz-msgs/python/hiroz_msgs_py/types/geometry_msgs.py`	Regenerates message definitions.
`crates/hiroz-msgs/python/hiroz_msgs_py/types/example_interfaces.py`	Adds example service groupings.
`crates/hiroz-msgs/python/hiroz_msgs_py/types/action_msgs.py`	Adds cancel-service grouping.
`crates/hiroz-codegen/src/python_msgspec_generator.rs`	Generates service grouping classes.

Comments suppressed due to low confidence (1)

crates/hiroz-py/src/node.rs:492

This validation also occurs after the core action client has already been built. Invalid action names are rejected at lines 482-484 as PyRuntimeError, so the promised construction-time ValueError is unreachable here. Compute the remapped, qualified action name before invoking the builder and use it both for validation and discovery state.

        // The action server advertises a `<action>/_action/send_goal` service;
        // wait_for_server polls the graph for it.
        let send_goal_service = format!(
            "{}/_action/send_goal",
            self.qualify_service_name(&action_name)?
        );

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Implements all 8 proposals from the rclpy API alignment review: P1 — wait_for_service / wait_for_server / wait_for_subscription Adds graph-polling wait primitives to ZClient, ZActionClient, and ZPublisher. Replaces time.sleep(1.0) anti-pattern in all examples. P2 — Smart error for swapped arguments create_publisher/create_subscriber detect when msg_type and topic args are positionally swapped (rclpy order vs hiroz order) and raise a clear TypeError. P3 — Method aliases ZNode.create_subscription aliased to create_subscriber. P4 — Service grouping type in codegen python_msgspec_generator now emits AddTwoInts.Request / .Response grouping classes with __srvtype__. create_client/create_server accept the grouping class or the bare Request class (back-compat). P5 — Wire custom exception types Timeouts now raise hiroz_py.TimeoutError instead of bare RuntimeError. Tests and examples updated accordingly. P6 — Optional callback-style create_server create_server accepts optional callback= kwarg; spawns a background thread when provided. Pull mode (take_request/send_response) remains the default. ZNode.create_service added as alias. P7 — Action grouping type in codegen Actions emit Fibonacci.Goal / .Result / .Feedback grouping classes with __actiontype__. create_action_client/server accept either form. P8 — QoS enum constants + int-depth shorthand ReliabilityPolicy, DurabilityPolicy, HistoryPolicy, LivelinessPolicy enums added to __init__.py. QoS params accept int (depth shorthand, rclpy-style). Also adds test_rclpy_alignment.py (17 tests, all passing). 65 total tests pass, clippy clean, ruff clean.

Background callback threads were silently swallowing exceptions via eprintln. Add a shared Arc<Mutex<Option<String>>> that records the most recent error; expose it as ZServer.last_error (resets on read). Also fix a pre-existing broken intra-doc link in lifecycle/node.rs (create_publisher → Self::create_publisher) that was failing cargo doc.

Add wait_for_server (action) tests, exercise create_subscription/ create_service aliases end-to-end, assert TimeoutError (not just HirozError) is raised on a matched-but-unresponsive service, exercise the action grouping class through an actual goal send, and pin the get_result(timeout=...) None-on-timeout contract. Drop the unused last_error clone in CallbackServerState that caused a dead_code warning.

The book still described the pre-alignment API (pull-only create_server, flat Request/Response classes) with no mention of wait_for_service/ wait_for_server/wait_for_subscription, swapped-arg detection, method aliases, grouping classes, TimeoutError, the callback-mode server plus last_error, or QoS enums/int shorthand. Add an "rclpy Alignment" section to the Python bindings chapter, extend the codegen chapter with the generated grouping-class shape, and merge the untracked migration guide into a new tracked chapter so it survives branch cleanup.

…g None ActionGoalHandle.get_result(timeout=...) silently returned None on timeout, unlike ZClient.call which raises hiroz_py.TimeoutError for the same condition. Align the two so timeout handling is consistent across services and actions.

send_goal and get_result hand-rolled timeout classification and mapped non-timeout failures to PyRuntimeError, while ZClient.call used the shared map_call_error helper and raised HirozError. Same operation shape, two different exception hierarchies. Route both action paths through map_call_error so the documented P5 contract (timeout -> TimeoutError, else -> HirozError) holds uniformly.

Two silent-degradation paths added alongside the P1/P4 work: qualify_service_name reimplemented the core helper and swallowed qualification errors with a fallback to the raw name. A malformed name then made wait_for_service/wait_for_server poll the graph for a name that can never appear, which presents as a hang. Delegate to hiroz::topic_name::qualify_service_name and propagate as ValueError. The service grouping-class path fell back to TypeHash::zero() when __hash__ was missing or unparseable, silently building a client that cannot match a typed server. The legacy Request-class path already raises here; match it.

…t stub The service path yields anyhow::Error but the action path yields zenoh::Error, so routing actions through map_call_error did not compile. Take anything convertible into a boxed error and render the source chain by hand, since boxing loses anyhow's {:#} chain formatting. Also correct the get_result stub: it raises TimeoutError now rather than returning None, so the return type is no longer Optional.

Converting anyhow::Error into Box<dyn Error> wraps the value, so is_timeout's downcast no longer sees the real error and every service timeout was misreported as a plain HirozError -- caught by test_p5_call_timeout_raises_timeout_error. Split into map_call_error (anyhow, derefs) and map_zenoh_error (the action paths' Box<dyn Error>), sharing the classify/format helpers.

…eoutError P5 introduced a typed hierarchy rooted at Exception, which silently broke two catch styles ported rclpy code relies on: except RuntimeError: # what these paths raised before P5 except TimeoutError: # what rclpy's Client.call actually raises Both kept compiling and running while no longer catching. HirozError now derives from RuntimeError, and TimeoutError from both HirozError and the builtin TimeoutError, so either clause keeps working while the typed hierarchy stays available for code that wants it. create_exception! takes a single base, so TimeoutError is built with type(name, bases, dict) at module init and cached for raising.

- Timeout args are unrestricted floats, so -1/NaN/inf reached Duration::from_secs_f64 and panicked. Route every public timeout through checked_timeout, raising ValueError. - The documented ValueError for malformed service/action names never fired: build() validates the same name first and maps failure to HirozError. Resolve before building in create_client/create_server/ create_action_client. - Discovery names skipped remapping, which core applies before qualification -- with a remap rule the entity was created under the new name while wait_for_* polled the old one and timed out. - Callback-mode servers leaked one pending reply per failed request; only send_response removed entries. Added discard_pending and wired it into every error path. - Docs: last_error is read-and-clear, so the example printed None; the migration example discarded the server it told you to keep; the core-gaps table listed parameters, lifecycle and clock as core gaps when they exist in core and merely lack Python exposure.

P7 advertised Fibonacci.Goal/.Result/.Feedback grouping classes, but the Python generator took no actions parameter and hiroz-msgs/build.rs discarded the discovered actions, so no grouping class was ever emitted -- the P7 test only exercised a hand-written class. Thread resolved actions through generate_python_bindings, emit Goal/Result/Feedback structs with the action type hash plus the __actiontype__ grouping class, and cover packages that contribute only actions. Result and Feedback are optional in the .action format, so only existing members are emitted. Callback-mode servers joined their worker from Drop, which runs with the GIL held while the worker takes the GIL to call user code: a slow or blocking callback froze 'del server' and interpreter shutdown, with a deadlock window besides. Drop now only signals; close()/__exit__ do the join with the GIL released.

…ures recv_feedback's closure returns Option, so the ? on checked_timeout did not compile there. Validate before entering the closure in all three action paths, which also raises the ValueError before the GIL is released rather than part-way through the blocking call.

qualify_topic_name skips empty components instead of rejecting them, so '//bad//name' passed ROS validation and failed later inside Zenoh's key-expression parser -- an error that cites a cargo registry path and never names the offending service. Check for empty chunks and trailing slashes in the binding so the documented ValueError actually covers them, and parametrise the test over all three rejection paths.

… rejects qualify_topic_name validates components only for relative and ~private names; absolute names are passed through unchecked, so '/bad name' is accepted. Swap that case for a relative one that core does validate, and record the absolute-name gap in the docstring rather than asserting behaviour that does not exist.

wait_for_server returned True as soon as send_goal was advertised, but a usable action server needs all five endpoints and discovery can surface them one at a time -- so the very next call could fail. Poll the core's has_action_server predicate against the qualified action name instead. Also mark SerializationError/TypeMismatchError as declared-but-unraised in the docs. Wiring them would re-wrap msgspec's TypeError/ValueError and break anyone catching those today, so the honest fix is to stop advertising a contract the code does not implement.

YuanYuYuan force-pushed the dev/review-python-api branch from e6506db to 7fdfcf1 Compare June 11, 2026 06:58

YuanYuYuan force-pushed the dev/review-python-api branch from 7fdfcf1 to c4cbcce Compare July 28, 2026 06:46

YuanYuYuan requested a review from Copilot July 28, 2026 06:47

Copilot started reviewing on behalf of YuanYuYuan July 28, 2026 06:48 View session

Copilot AI reviewed Jul 28, 2026

View reviewed changes

This was referenced Jul 28, 2026

qualify_topic_name does not validate absolute topic/service/action names #264

Open

ZPayloadView zero-copy tests have never run (numpy not installed) #266

Open

YuanYuYuan added 18 commits August 15, 2026 02:26

style(hiroz-py): rustfmt the get_result timeout arm

6e5b144

chore(msgs): regenerate python msgspec stubs after rebase

7c08392

YuanYuYuan force-pushed the dev/review-python-api branch from 02da450 to 7c08392 Compare August 14, 2026 19:46

Navigation Menu

Uh oh!

feat(hiroz-py): rclpy alignment — P1–P8 API improvements - #192

feat(hiroz-py): rclpy alignment — P1–P8 API improvements#192
YuanYuYuan wants to merge 18 commits into
mainfrom
dev/review-python-api

YuanYuYuan commented May 29, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jul 28, 2026 •

edited

Loading

Built to branch `gh-pages` at 2026-08-14 19:47 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

YuanYuYuan commented May 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Exception hierarchy

P6 callback-server shutdown

Breaking changes

What fails without this

Evidence

Follow-ups

Uh oh!

github-actions Bot commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Built to branch gh-pages at 2026-08-14 19:47 UTC. Preview will be ready when the GitHub Pages deployment is complete.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

YuanYuYuan commented May 29, 2026 •

edited

Loading

github-actions Bot commented Jul 28, 2026 •

edited

Loading

Built to branch `gh-pages` at 2026-08-14 19:47 UTC.
Preview will be ready when the GitHub Pages deployment is complete.