policy_client: config schema + SdkConfig wiring#249
Open
shantanuparab-tr wants to merge 5 commits into
Open
Conversation
Collaborator
Author
This was referenced Jun 15, 2026
shantanuparab-tr
force-pushed
the
06-15-policy-client-01-config-schema
branch
from
June 15, 2026 18:45
cfd49b9 to
e435235
Compare
shantanuparab-tr
added
the
enhancement
Graphite App
shantanuparab-tr
force-pushed
the
06-15-policy-client-01-config-schema
branch
from
June 16, 2026 21:51
e435235 to
edb331d
Compare
shantanuparab-tr
changed the base branch from
06-09-hardware-episode-lifecycle
to
graphite-base/249
June 16, 2026 21:56
shantanuparab-tr
force-pushed
the
graphite-base/249
branch
from
June 16, 2026 21:57
7bb1883 to
6e2eadd
Compare
shantanuparab-tr
force-pushed
the
06-15-policy-client-01-config-schema
branch
from
June 16, 2026 21:57
edb331d to
1aac9de
Compare
shantanuparab-tr
force-pushed
the
06-15-policy-client-01-config-schema
branch
from
June 16, 2026 21:57
1aac9de to
9ae754e
Compare
lukeschmitt-tr
requested changes
Jun 22, 2026
PolicyClientConfig (transport name, transport_config, joint_layout, subscriptions, drain_threshold, inference_hz, output filters) and its HardwareConfig.policy_clients wiring + duplicate-id validation. Header-only schema, so the parse test builds without the policy-client build gate.
Two subscriptions with distinct record_id but the same obs_key parsed cleanly, then silently collided at pack time (the second overwrote the first in the observation dict, dropping one arm's state with no error). Mirror the existing record_id dedup with an obs_key uniqueness check. Addresses review finding PR249-F1.
…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
force-pushed
the
06-15-policy-client-01-config-schema
branch
from
July 16, 2026 15:12
9ae754e to
a2311ea
Compare
shantanuparab-tr
added a commit
that referenced
this pull request
Jul 16, 2026
…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.
There was a problem hiding this comment.
Pull request overview
Adds the initial configuration schema and SDK config wiring for the upcoming PolicyClient hardware component stack, enabling validated config parsing (and tests) without introducing runtime policy-client dependencies yet.
Changes:
- Introduces
PolicyClientConfigschema (transport selection, subscriptions, joint layout, timing/playback knobs) with validation. - Wires
HardwareConfig.policy_clientsthroughHardwareConfig::from_json()including duplicate-idvalidation and indexed error context. - Adds
test_policy_client_configand CMake integration to cover parsing/validation.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
include/trossen_sdk/configuration/types/hardware/policy_client_config.hpp |
New header-only PolicyClientConfig schema + validation logic. |
include/trossen_sdk/configuration/sdk_config.hpp |
Adds HardwareConfig::policy_clients and includes the new schema header. |
src/configuration/sdk_config.cpp |
Parses hardware.policy_clients, adds per-index error context, validates duplicate ids. |
tests/test_policy_client_config.cpp |
New unit tests for PolicyClientConfig and HardwareConfig::policy_clients parsing/validation. |
tests/CMakeLists.txt |
Adds test_policy_client_config target and registers it with CTest via gtest_discover_tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Address Copilot's post-review comments on #249: - Replace the non-ASCII drain-threshold symbol (theta) and an en-dash range separator with ASCII, for source-encoding portability. - Note that the JSON 'type' field selects this schema at the hardware-registry level and is not parsed by from_json(), so readers don't expect it here.
shantanuparab-tr
added a commit
that referenced
this pull request
Jul 16, 2026
…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
…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.
…example Replace non-ASCII characters in the PolicyClientConfig comments and tests with ASCII, and remove the 'type' field from the JSON doc example since from_json() does not parse it.
shantanuparab-tr
added a commit
that referenced
this pull request
Jul 17, 2026
…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.
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
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
First PR in the PolicyClient stack — the SDK bridge that drives Trossen arms from a remote inference server (openpi or LeRobot). This PR adds only the configuration schema so the rest of the stack has a validated config type to build against; no runtime code or dependencies yet.
Changes
include/trossen_sdk/configuration/types/hardware/policy_client_config.hpp: newPolicyClientConfigschema — transport selection (transportname + opaquetransport_config),joint_layout(per-arm groups withjoint_names), recordsubscriptions, and playback/timing knobs (drain_threshold,inference_hz, output EMA alphas). Defaults preserve the openpi flow (transportdefaults toopenpi_ws).include/trossen_sdk/configuration/sdk_config.hpp+src/configuration/sdk_config.cpp: carryHardwareConfig.policy_clients(zero or more) throughfrom_json, with duplicate-idvalidation and per-entry error context.tests/test_policy_client_config.cpp+tests/CMakeLists.txt: parse/validation coverage. The schema is header-only, so this test builds without the policy-client build gate.Test Plan
make build)test_policy_client_configpre-commit run --all-files)Breaking Changes
None.
policy_clientsis a new optionalHardwareConfigarray; existing configs load unchanged.Related
Based on #248 (auto-retargets to
mainwhen it merges). Followed by #250–#255. No TDS ticket — experimental subsystem; design lives in the externalpolicy_client_docs.