Skip to content

policy_client: transport interface, registry, and both transports#252

Open
shantanuparab-tr wants to merge 7 commits into
06-15-policy-client-03-codecsfrom
06-15-policy-client-04-transports
Open

policy_client: transport interface, registry, and both transports#252
shantanuparab-tr wants to merge 7 commits into
06-15-policy-client-03-codecsfrom
06-15-policy-client-04-transports

Conversation

@shantanuparab-tr

@shantanuparab-tr shantanuparab-tr commented Jun 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Summary

Adds the transport plugin layer: an abstract PolicyTransport, a name→factory TransportRegistry, and the two built-in transports — openpi (WebSocket + msgpack) and LeRobot (async_inference gRPC + pickle). This is the seam that lets the client speak to either server family without naming a concrete transport.

Stack: PolicyClient stack, based on #248. Review bottom-up (#251 → here).

Changes

  • In include/trossen_sdk/hw/policy/policy_transport.hpp: the abstract PolicyTransport — an async-first, hot-path-safe contract (connect / push_observation latest-wins / try_poll_chunk / status), all noexcept on the hot path. Each transport owns its wire format end-to-end.
  • In include/trossen_sdk/hw/policy/transport_registry.{hpp} + src/hw/policy/transport_registry.cpp: process-wide name → factory map (function-local statics under a mutex, factory invoked outside the lock, errors annotated with the name). Built-ins self-register; user plugins call register_factory().
  • In include/trossen_sdk/hw/policy/openpi_websocket_transport.hpp + src/hw/policy/openpi_websocket_transport.cpp: openpi over a single WebSocket (request/reply mapped onto the async contract via an internal worker + one-deep slots); self-registers as openpi_ws.
  • In src/hw/policy/lerobot_grpc_transport.{hpp,cpp} + src/hw/policy/proto/lerobot_transport_services.proto: LeRobot async_inference over gRPC; self-registers as lerobot_grpc. The proto is vendored byte-identical from the pinned LeRobot release and compiled at build time.
  • In cmake/policy_client.cmake: add FetchContent of IXWebSocket and build-time protoc/grpc generation for the LeRobot service; link them PRIVATE.
  • In tests/test_openpi_websocket_transport.cpp, tests/test_lerobot_grpc_transport.cpp, tests/CMakeLists.txt: openpi codec/round-trip coverage and a LerobotGrpcTransport driven against an in-process AsyncInference server.

Test Plan

  • Builds cleanly with -DTROSSEN_SDK_ENABLE_POLICY_CLIENT=ON
  • Tests pass — test_openpi_websocket_transport, test_lerobot_grpc_transport
  • Lint passes (pre-commit run --all-files)
  • Hardware — exercised end-to-end against the live pi05 server by policy_client: stationary examples (openpi + lerobot) #255

Breaking Changes

None. New gated sources behind TROSSEN_SDK_ENABLE_POLICY_CLIENT (default OFF).

Related

Based on #248. Stacked on #251; followed by #253–#255.

This was referenced Jun 15, 2026
Merged
Open
Open
@shantanuparab-tr Graphite App

shantanuparab-tr commented Jun 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

This was referenced Jun 15, 2026
Open
Open
Open
Open
@shantanuparab-tr
shantanuparab-tr force-pushed the 06-15-policy-client-03-codecs branch from 64b5509 to 8960bf7 Compare June 15, 2026 16:56
@shantanuparab-tr
shantanuparab-tr force-pushed the 06-15-policy-client-04-transports branch from 227e418 to 26b0e67 Compare June 15, 2026 16:56
@shantanuparab-tr
shantanuparab-tr force-pushed the 06-15-policy-client-03-codecs branch from 8960bf7 to ea1db70 Compare June 15, 2026 18:45
@shantanuparab-tr
shantanuparab-tr force-pushed the 06-15-policy-client-04-transports branch from 26b0e67 to 1485f2f Compare June 15, 2026 18:45
@shantanuparab-tr shantanuparab-tr added the enhancement New feature or request label Jun 15, 2026 — with Graphite App
@shantanuparab-tr shantanuparab-tr self-assigned this Jun 15, 2026
@shantanuparab-tr
shantanuparab-tr force-pushed the 06-15-policy-client-04-transports branch from 1485f2f to f781714 Compare June 16, 2026 21:51
@shantanuparab-tr
shantanuparab-tr force-pushed the 06-15-policy-client-03-codecs branch 2 times, most recently from b334671 to 9758bde Compare June 16, 2026 21:58
@shantanuparab-tr
shantanuparab-tr force-pushed the 06-15-policy-client-04-transports branch from f781714 to b456b0e Compare June 16, 2026 21:58
shantanuparab-tr added a commit that referenced this pull request Jul 16, 2026
@shantanuparab-tr
docs(policy-client): drop dev-slice/forward-looking refs in config sc…
a2311ea 
…hema

Address Luke's review comments on #249:
- server_url: give openpi/lerobot_grpc endpoints as plain examples instead
  of describing lerobot_grpc as 'planned' (it ships in #252 — the comment
  had gone stale).
- drain_threshold: remove the internal 'slice L5' development artifact,
  keeping the inline fact (consumed by the drain-threshold firing logic).
@shantanuparab-tr
shantanuparab-tr force-pushed the 06-15-policy-client-03-codecs branch from 9758bde to 346766b Compare July 16, 2026 15:12
shantanuparab-tr added a commit that referenced this pull request Jul 16, 2026
@shantanuparab-tr
docs(policy-client): remove stale L3/L4 dev-slice refs in lerobot tra…
cc2b35e 
…nsport

Same class of artifact Luke flagged on #249, found in #252 during a sweep —
and gone stale: the file header called this an 'L3 skeleton' and the connect
path claimed 'L3 ships a placeholder payload; L4 ships the pickled
RemotePolicyConfig'. policy_setup_bytes_() returns the real pickled
RemotePolicyConfig, so the comment misdescribed the shipped code. Describe the
actual handshake and payload path; drop the internal slice references.
@shantanuparab-tr
shantanuparab-tr force-pushed the 06-15-policy-client-04-transports branch from b456b0e to cc2b35e Compare July 16, 2026 15:12
@shantanuparab-tr
shantanuparab-tr force-pushed the 06-15-policy-client-03-codecs branch from 346766b to 909b956 Compare July 16, 2026 15:41
shantanuparab-tr added a commit that referenced this pull request Jul 16, 2026
@shantanuparab-tr
docs(policy-client): remove stale L3/L4 dev-slice refs in lerobot tra…
b167d44 
…nsport

Same class of artifact Luke flagged on #249, found in #252 during a sweep —
and gone stale: the file header called this an 'L3 skeleton' and the connect
path claimed 'L3 ships a placeholder payload; L4 ships the pickled
RemotePolicyConfig'. policy_setup_bytes_() returns the real pickled
RemotePolicyConfig, so the comment misdescribed the shipped code. Describe the
actual handshake and payload path; drop the internal slice references.
@shantanuparab-tr
shantanuparab-tr force-pushed the 06-15-policy-client-04-transports branch from cc2b35e to b167d44 Compare July 16, 2026 15:41
shantanuparab-tr added a commit that referenced this pull request Jul 16, 2026
@shantanuparab-tr
test(policy-client): drive lerobot_grpc transport via a mock stub (#252)
e2ac602 
The suite spun up a real in-process AsyncInference server + client channel;
tearing those down at scope exit intermittently aborted inside the system
gRPC's abseil mutex (~35% of runs on gRPC 1.51) — a library teardown race, not
a transport defect.

Inject a gmock MockAsyncInferenceStub instead so no real channel/server is ever
created:
- stub_ is now AsyncInference::StubInterface; add set_stub_for_test(); connect()
  skips channel creation when a stub is injected (production path unchanged).
- proto-gen adds generate_mock_code=true (emits MockAsyncInferenceStub); the
  test links GTest::gmock.
- Tests keep factory/registry/config-validation + unreachable-target coverage,
  and exercise the handshake (Ready + SendPolicyInstructions) and the
  GetActions decode path against the mock. Streaming push isn't mocked; its
  wire format is covered by test_lerobot_codec.

Verified 40/40 runs clean (0 aborts).
shantanuparab-tr added a commit that referenced this pull request Jul 16, 2026
@shantanuparab-tr
test(policy-client): de-flake ServerInitiatedCloseClearsConnectedFlag (…
e160d6b 
…#252)

The fake server sends metadata and closes in the same Open handler, so the
Close frame often lands *during* connect()'s handshake wait — connect() then
throws 'server going away' (~20% of runs) and the test, which called connect()
unguarded, failed.

The invariant under test is 'a server-initiated close ends disconnected',
which holds whether the close races the handshake (connect throws, and its
failure path calls close() -> connected_=false) or arrives just after. Wrap
connect() in try/catch and assert the eventual disconnected state either way.
Test-only change. Verified 0/40 (was ~20%).
shantanuparab-tr added a commit that referenced this pull request Jul 17, 2026
@shantanuparab-tr
docs(policy-client): remove stale L3/L4 dev-slice refs in lerobot tra…
7c6138a 
…nsport

Same class of artifact Luke flagged on #249, found in #252 during a sweep —
and gone stale: the file header called this an 'L3 skeleton' and the connect
path claimed 'L3 ships a placeholder payload; L4 ships the pickled
RemotePolicyConfig'. policy_setup_bytes_() returns the real pickled
RemotePolicyConfig, so the comment misdescribed the shipped code. Describe the
actual handshake and payload path; drop the internal slice references.
shantanuparab-tr added a commit that referenced this pull request Jul 17, 2026
@shantanuparab-tr
test(policy-client): drive lerobot_grpc transport via a mock stub (#252)
fa55512 
The suite spun up a real in-process AsyncInference server + client channel;
tearing those down at scope exit intermittently aborted inside the system
gRPC's abseil mutex (~35% of runs on gRPC 1.51) — a library teardown race, not
a transport defect.

Inject a gmock MockAsyncInferenceStub instead so no real channel/server is ever
created:
- stub_ is now AsyncInference::StubInterface; add set_stub_for_test(); connect()
  skips channel creation when a stub is injected (production path unchanged).
- proto-gen adds generate_mock_code=true (emits MockAsyncInferenceStub); the
  test links GTest::gmock.
- Tests keep factory/registry/config-validation + unreachable-target coverage,
  and exercise the handshake (Ready + SendPolicyInstructions) and the
  GetActions decode path against the mock. Streaming push isn't mocked; its
  wire format is covered by test_lerobot_codec.

Verified 40/40 runs clean (0 aborts).
shantanuparab-tr added a commit that referenced this pull request Jul 17, 2026
@shantanuparab-tr
test(policy-client): de-flake ServerInitiatedCloseClearsConnectedFlag (…
9d3b610 
…#252)

The fake server sends metadata and closes in the same Open handler, so the
Close frame often lands *during* connect()'s handshake wait — connect() then
throws 'server going away' (~20% of runs) and the test, which called connect()
unguarded, failed.

The invariant under test is 'a server-initiated close ends disconnected',
which holds whether the close races the handshake (connect throws, and its
failure path calls close() -> connected_=false) or arrives just after. Wrap
connect() in try/catch and assert the eventual disconnected state either way.
Test-only change. Verified 0/40 (was ~20%).
@shantanuparab-tr
shantanuparab-tr force-pushed the 06-15-policy-client-04-transports branch from e160d6b to cf93349 Compare July 17, 2026 00:21
@shantanuparab-tr
policy_client: transport interface, registry, and both transports
e41015a 
PolicyTransport (async push/poll, hot-path-safe) + TransportRegistry
(config name -> self-registered factory), with the two built-ins:
OpenpiWebsocketTransport (WebSocket + msgpack-numpy) and LerobotGrpcTransport
(async_inference gRPC + pickle via the codec). Adds IXWebSocket FetchContent
and build-time proto generation for the LeRobot service.
@shantanuparab-tr
fix(policy-client): bound transport waits so a wedged server can't st…
22c825b 
…all the arm

- openpi round_trip_ waited on the reply cv with no timeout, so a
  half-open/wedged server parked the worker forever while status() still
  reported kConnected (silent permanent stall on a robot path). Bound it
  with wait_for + a request_timeout_s transport_config field (default
  30s); on timeout, record failure and flip to kDisconnected (PR252-F1).
- gRPC hot-path RPCs (send_observation_bytes_, receive_loop_) set no
  deadline, and a TryCancel publish race let close() miss the cancel and
  hang the destructor (the 'Ctrl+C won't safe the arms' teardown hang).
  Add ctx.set_deadline bounded by rpc_timeout_s (default 5s; safe because
  GetActions is a unary short-poll) and self-cancel right after
  publishing the ctx if shutdown was requested (PR252-F2).
- Clean the stray generate-command fragment glued onto the vendored
  proto license line (PR252-F7).
@shantanuparab-tr
docs(policy-client): remove stale L3/L4 dev-slice refs in lerobot tra…
4dac00e 
…nsport

Same class of artifact Luke flagged on #249, found in #252 during a sweep —
and gone stale: the file header called this an 'L3 skeleton' and the connect
path claimed 'L3 ships a placeholder payload; L4 ships the pickled
RemotePolicyConfig'. policy_setup_bytes_() returns the real pickled
RemotePolicyConfig, so the comment misdescribed the shipped code. Describe the
actual handshake and payload path; drop the internal slice references.
@shantanuparab-tr
test(policy-client): drive lerobot_grpc transport via a mock stub (#252)
119acf9 
The suite spun up a real in-process AsyncInference server + client channel;
tearing those down at scope exit intermittently aborted inside the system
gRPC's abseil mutex (~35% of runs on gRPC 1.51) — a library teardown race, not
a transport defect.

Inject a gmock MockAsyncInferenceStub instead so no real channel/server is ever
created:
- stub_ is now AsyncInference::StubInterface; add set_stub_for_test(); connect()
  skips channel creation when a stub is injected (production path unchanged).
- proto-gen adds generate_mock_code=true (emits MockAsyncInferenceStub); the
  test links GTest::gmock.
- Tests keep factory/registry/config-validation + unreachable-target coverage,
  and exercise the handshake (Ready + SendPolicyInstructions) and the
  GetActions decode path against the mock. Streaming push isn't mocked; its
  wire format is covered by test_lerobot_codec.

Verified 40/40 runs clean (0 aborts).
@shantanuparab-tr
test(policy-client): de-flake ServerInitiatedCloseClearsConnectedFlag (…
8ffe3cc 
…#252)

The fake server sends metadata and closes in the same Open handler, so the
Close frame often lands *during* connect()'s handshake wait — connect() then
throws 'server going away' (~20% of runs) and the test, which called connect()
unguarded, failed.

The invariant under test is 'a server-initiated close ends disconnected',
which holds whether the close races the handshake (connect throws, and its
failure path calls close() -> connected_=false) or arrives just after. Wrap
connect() in try/catch and assert the eventual disconnected state either way.
Test-only change. Verified 0/40 (was ~20%).
@shantanuparab-tr
chore(policy): bump vendored LeRobot proto reference to v0.6.0
0e7703e 
The gRPC proto is vendored byte-identical from LeRobot; update the pinned
reference to v0.6.0 (commit 30da8e68).
@shantanuparab-tr
fix(policy): guard LerobotGrpcTransport::close() against a context us…
d6b2d74 
…e-after-free

close() called TryCancel() on a ClientContext owned by a worker thread's stack,
which the worker could destroy concurrently as its RPC completed. Serialize the
publish, clear, and cancel of the active_*_ctx_ slots under a new cancel_mu_ so a
non-null slot always points at a live context, and destroy the channel normally
on teardown.
@shantanuparab-tr
shantanuparab-tr force-pushed the 06-15-policy-client-03-codecs branch from 79b4728 to 03e39a7 Compare July 17, 2026 14:17
@shantanuparab-tr
shantanuparab-tr force-pushed the 06-15-policy-client-04-transports branch from cf93349 to d6b2d74 Compare July 17, 2026 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lukeschmitt-tr lukeschmitt-tr Awaiting requested review from lukeschmitt-tr
@abhichothani42 abhichothani42 Awaiting requested review from abhichothani42

Assignees

@shantanuparab-tr shantanuparab-tr

Labels

enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@shantanuparab-tr