feat(node-cxx): expose the Service and Action patterns to the C++ binding - #2853
feat(node-cxx): expose the Service and Action patterns to the C++ binding#2853caothu2k1 wants to merge 1 commit into
Conversation
…ding The Rust and Python node APIs have first-class Service (request/reply) and Action (goal/feedback/result) support; the C++ binding had none of it. There was no request-id generator, no response passthrough helper, and no `recv_*` carrying the framework's timeout and server-restart fault tolerance, so a C++-only downstream had to hand-patch the cxx bridge to get any of it. Mirror that surface into `apis/c++/node/src/lib.rs`, so the generated `dora-node-api.h` gains `new_request_id` / `new_goal_id`, `send_service_request` (+ Arrow variant), `send_service_response`, `recv_service_response`, `recv_action_result`, the `goal_status_*` constants, and typed `Metadata` accessors for the reserved correlation keys. Also add `event_as_input_with_metadata`. The server side of a correlated exchange must read the incoming `request_id` to echo it back, and the only metadata-carrying accessor so far (`event_as_arrow_input_with_info`) exports the payload through the Arrow C Data Interface -- which would force an Arrow C++ dependency on byte-payload nodes. The success status is named `Matched` rather than `None` or `Success` because X11's `X.h` defines both of those as macros, which would break any node pulling in X11 directly or via OpenCV / Qt / GTK. Defensive details at the FFI boundary: `timeout_ms` arrives as an unbounded `u64` and is clamped so the internal `Instant::now() + timeout` cannot panic; `server_node_id` is parsed via `FromStr` rather than the panic-on-invalid `From<String>`; and a failed send clears the returned `request_id` so a caller that skips the error check cannot block on a correlation that never reached the wire. Adds `examples/c++-service-action/` covering both patterns end-to-end, wired into the nightly examples job, plus 13 unit tests. Implements requests 1 and 2 of dora-rs#2686. The CUDA/pinned memory-pool transport (request 3) is left for a follow-up: it mirrors ~900 lines of Python shmem/DORADMA logic and its acceptance criterion requires validation on a Jetson without CUDA IPC. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
|
Hi @phil-opp , could you please review this PR?, All CI checks have passed. |
No issues found. I reviewed the cxx bridge surface in Things I specifically checked and that hold up:
The unit tests are meaningful (behavioral assertions, not constant-echo), and the Generated by Claude Code |
|
/trunk merge |
|
An error occurred while submitting your PR to the queue: |
Implements requests 1 (Service) and 2 (Action) of #2686.
What
The Rust and Python node APIs have first-class Service (request/reply) and
Action (goal/feedback/result) support. The C++ binding had none of it — no
request-id generator, no response passthrough helper, and, the part that
actually matters, no
recv_*carrying the framework's timeout andserver-restart fault tolerance. A C++-only downstream had to hand-patch
apis/c++/node/src/lib.rsand rebuild to get any of it.This mirrors that surface into the cxx bridge, so the generated
dora-node-api.hgains:DoraNode::new_request_id/new_goal_idnew_request_id()/new_goal_id()DoraNode::send_service_requestsend_service_request()/send_arrow_service_request()DoraNode::send_service_responsesend_service_response()EventStream::recv_service_responserecv_service_response()EventStream::recv_action_resultrecv_action_result()REQUEST_ID/GOAL_ID/GOAL_STATUSMetadata::set_request_id/set_goal_id/set_goal_status(+ getters)GOAL_STATUS_SUCCEEDED/_ABORTED/_CANCELEDgoal_status_succeeded()/_aborted()/_canceled()PatternErrorDoraPatternStatusScope: request 3 is deliberately not here
#2686 bundles three requests. The CUDA/pinned memory-pool transport (3) is
not in this PR. The Python implementation it would mirror
(
apis/python/node/src/lib.rs) is ~900 lines of shmem/seqlock/DORADMA/CUDAlogic, and its acceptance criterion requires validating the host-pinned path
on a Jetson with no CUDA IPC — hardware I cannot verify against. Bundling
an unverified port of that into an otherwise reviewable Service/Action change
seemed worse than leaving #2686 open for a follow-up. Happy to take it on
separately if the shape here looks right.
A gap this uncovered
event_as_inputreturns{id, data}with no metadata, and the onlymetadata-carrying accessor (
event_as_arrow_input_with_info) exports thepayload through the Arrow C Data Interface. That made the server side of
any correlated exchange impossible from a byte-payload C++ node: it could not
read the incoming
request_idto echo back, short of taking on an Arrow C++dependency. Added
event_as_input_with_metadatato close it.Notes for reviewers
DoraPatternStatus::Matched, notNoneorSuccess. X11'sX.hdefines both of those as macros (
#define None 0L,#define Success 0),so either name would break compilation for any node pulling in X11 —
directly or via OpenCV / Qt / GTK. Verified by compiling the new examples
with
-include X11/X.h.send_service_requestclearsrequest_idwhen the send fails, so acaller that skips the
errorcheck cannot block inrecv_service_responseon a correlation that never reached the wire.
timeout_msis clamped. It crosses the bridge as an unboundedu64while the Rust helpers compute
Instant::now() + timeoutinternally, whichpanics on overflow — the same defensive stance
next_event_timeoutalreadytakes for this reason.
server_node_idis parsed viaFromStr, notFrom<String>, which isdocumented as panicking on invalid characters; a typo in a C++ string
literal returns
InvalidArgumentinstead of aborting the node.DoraEventTypeon the returnedevent (
Timeout/AllInputsClosed/Empty), so callers can branch oneither field rather than being forced onto the new one.
Testing
New example —
examples/c++-service-action/, four C++ nodes covering bothpatterns, wired into
[[example]] cxx-service-actionand the nightlyexamplesjob. Verified end-to-end against a live daemon:5 correlated service exchanges and 3 action goals, all nodes exiting
Success. The feedback lines landing after the result confirm thebuffering guarantee:
recv_action_resultstashes non-matching events for thecaller's own loop instead of swallowing them.
Unit tests — 13 new tests in
apis/c++/node/src/lib.rscovering thecorrelation-key spellings against the framework constants, request-id
overwrite semantics, the id-cleared-on-failure contract, timeout clamping at
u64::MAX, node-id rejection, and eachPatternError→ status mapping.Gates run:
cargo fmt --all -- --check,cargo clippy --all -- -D warnings,cargo test -p dora-node-api-cxx(15/15),cargo check --examples, plusg++ -Wall -Wextra -fsyntax-onlyon all four examplesources both with and without X11 headers.
Not run locally:
cargo test --all. A full workspace debug build reaches~50 GB in
target/, which does not fit on my disk. CI covers it. Also notethe example harness invokes
clang++to match every other C++ example here;clang++isn't installed on my machine, so the end-to-end run above wascompiled with
g++ 9.4— the committed harness is unchanged.Acceptance criteria from #2686
REQUEST_ID-correlated response with timeout + fault tolerancedora-node-api.h; existing C++ examples still build