Repository navigation
feat(simulation): bind Hub models at runtime - #18
Merged
Merged
Conversation
审查者指南此 PR 引入了一个可选择启用的确定性模拟运行时，通过 OpenSDL 现有的设备和事件契约公开虚拟设备，并支持可扩展的后端以及 CLI/配置接入。同时，它还通过 运行时 Hub 资产绑定时序图sequenceDiagram
participant Client
participant Engine
participant Adapter as SimulationAdapter
participant Transport as SimulationTransport
participant Backend as KinematicBackend
Client->>Engine: send DeviceCommand
Engine->>Adapter: encode_command
Adapter->>Transport: send
Transport->>Backend: apply_action
Backend-->>Transport: update simulation_asset
Transport-->>Engine: telemetry envelope
Engine->>Adapter: decode_response
Adapter-->>Client: simulation_asset telemetry
模拟资产同步流程图flowchart TD
Start[Virtual device starts] --> Snapshot[Telemetry snapshot emitted]
Snapshot --> Action{set_asset or clear_asset}
Action --> Set[set_asset validates namespace and name]
Action --> Clear[clear_asset removes simulation_asset]
Set --> Telemetry[Next telemetry includes simulation_asset]
Clear --> Telemetry
Telemetry --> Client[UI resolves or removes Hub model]
文件级变更
提示和命令与 Sourcery 交互
自定义你的体验访问你的控制面板以：
获取帮助Original review guide in EnglishReviewer's GuideThe PR introduces an opt-in deterministic simulation runtime with virtual devices exposed through OpenSDL’s existing device and event contracts, including extensible backend support and CLI/configuration wiring. It also enables runtime Hub model binding via set_asset and clear_asset, propagating the selected identity as simulation_asset telemetry for synchronized clients, with tests and local-development documentation. Sequence diagram for runtime Hub asset bindingsequenceDiagram
participant Client
participant Engine
participant Adapter as SimulationAdapter
participant Transport as SimulationTransport
participant Backend as KinematicBackend
Client->>Engine: send DeviceCommand
Engine->>Adapter: encode_command
Adapter->>Transport: send
Transport->>Backend: apply_action
Backend-->>Transport: update simulation_asset
Transport-->>Engine: telemetry envelope
Engine->>Adapter: decode_response
Adapter-->>Client: simulation_asset telemetry
Flow diagram for simulation asset synchronizationflowchart TD
Start[Virtual device starts] --> Snapshot[Telemetry snapshot emitted]
Snapshot --> Action{set_asset or clear_asset}
Action --> Set[set_asset validates namespace and name]
Action --> Clear[clear_asset removes simulation_asset]
Set --> Telemetry[Next telemetry includes simulation_asset]
Clear --> Telemetry
Telemetry --> Client[UI resolves or removes Hub model]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
嘿——我发现了 2 个问题
面向 AI Agent 的提示
请处理这次代码审查中的评论：
## 单独评论
### 评论 1
<location path="crates/osdl-core/src/transport/simulation.rs" line_range="332-348" />
<code_context>
+ .and_then(Value::as_str)
+ .ok_or("simulation: command missing action")?;
+ let params = command.get("params").cloned().unwrap_or_else(|| json!({}));
+ if action == "set_asset" {
+ let namespace = params
+ .get("namespace")
+ .and_then(Value::as_str)
+ .unwrap_or("");
+ let name = params.get("name").and_then(Value::as_str).unwrap_or("");
+ if namespace.trim().is_empty() || name.trim().is_empty() {
+ return Err("simulation: set_asset requires namespace and name".into());
+ }
+ if params
+ .get("version")
</code_context>
<issue_to_address>
**问题 (bug_risk)：** `set_asset` 接受任意非空的命名空间、名称和可选版本，并在未查询或验证 Hub 模型的情况下将其作为 `simulation_asset` 发出，因此任意或不存在的身份都会被客户端当作已验证的 Hub 绑定。
**触发条件：** 客户端发送了语法有效但不存在、未发布或未经验证的 Hub 身份时。
**建议修复：** 在修改状态之前，通过 Hub 验证/目录边界解析该引用；或者将此操作重命名并记录为接受未经验证的身份。
</issue_to_address>
### 评论 2
<location path="crates/osdl-core/src/transport/simulation.rs" line_range="442-482" />
<code_context>
+ let update = rx.recv().await.expect("action telemetry");
</code_context>
<issue_to_address>
**问题 (testing)：** 测试在发送 `set_asset` 后读取下一条遥测消息，并假定它是操作响应；但周期性 tick 任务可能会先发出未更改的遥测快照，导致断言出现非确定性失败。
**触发条件：** 10 Hz 的 tick 在 `send` 和测试的 `rx.recv()` 之间运行时。
**建议修复：** 持续读取遥测消息，直到消息包含预期的资产绑定；或者在断言操作响应时停止 tick 任务或与其协调。
</issue_to_address>Original comment in English
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="crates/osdl-core/src/transport/simulation.rs" line_range="332-348" />
<code_context>
+ .and_then(Value::as_str)
+ .ok_or("simulation: command missing action")?;
+ let params = command.get("params").cloned().unwrap_or_else(|| json!({}));
+ if action == "set_asset" {
+ let namespace = params
+ .get("namespace")
+ .and_then(Value::as_str)
+ .unwrap_or("");
+ let name = params.get("name").and_then(Value::as_str).unwrap_or("");
+ if namespace.trim().is_empty() || name.trim().is_empty() {
+ return Err("simulation: set_asset requires namespace and name".into());
+ }
+ if params
+ .get("version")
</code_context>
<issue_to_address>
**issue (bug_risk):** `set_asset` accepts any non-empty namespace, name, and optional version and emits it as `simulation_asset` without consulting or verifying a Hub model, so arbitrary or nonexistent identities are presented to clients as verified Hub bindings.
**Triggers:** When a client sends a syntactically valid but nonexistent, unpublished, or unverified Hub identity.
**Suggested fix:** Resolve the reference through the Hub verification/catalog boundary before mutating state, or rename/document the action as accepting an unverified identity.
</issue_to_address>
### Comment 2
<location path="crates/osdl-core/src/transport/simulation.rs" line_range="442-482" />
<code_context>
+ let update = rx.recv().await.expect("action telemetry");
</code_context>
<issue_to_address>
**issue (testing):** The test reads the next telemetry message after sending `set_asset` and assumes it is the action response, but the periodic tick task can emit an unchanged telemetry snapshot first, causing the assertion to fail nondeterministically.
**Triggers:** When the 10 Hz tick runs between `send` and the test's `rx.recv()`.
**Suggested fix:** Read telemetry until the message contains the expected asset binding, or stop/coordinate the tick task while asserting the action response.
</issue_to_address>
Comment on lines
+332
to
+348
| if action == "set_asset" { | ||
| let namespace = params | ||
| .get("namespace") | ||
| .and_then(Value::as_str) | ||
| .unwrap_or(""); | ||
| let name = params.get("name").and_then(Value::as_str).unwrap_or(""); | ||
| if namespace.trim().is_empty() || name.trim().is_empty() { | ||
| return Err("simulation: set_asset requires namespace and name".into()); | ||
| } | ||
| if params | ||
| .get("version") | ||
| .and_then(Value::as_str) | ||
| .is_some_and(|version| version.trim().is_empty()) | ||
| { | ||
| return Err("simulation: set_asset version must not be empty".into()); | ||
| } | ||
| } |
There was a problem hiding this comment.
问题 (bug_risk)： set_asset 接受任意非空的命名空间、名称和可选版本，并在未查询或验证 Hub 模型的情况下将其作为 simulation_asset 发出，因此任意或不存在的身份都会被客户端当作已验证的 Hub 绑定。
触发条件： 客户端发送了语法有效但不存在、未发布或未经验证的 Hub 身份时。
建议修复： 在修改状态之前，通过 Hub 验证/目录边界解析该引用；或者将此操作重命名并记录为接受未经验证的身份。
Original comment in English
issue (bug_risk): set_asset accepts any non-empty namespace, name, and optional version and emits it as simulation_asset without consulting or verifying a Hub model, so arbitrary or nonexistent identities are presented to clients as verified Hub bindings.
Triggers: When a client sends a syntactically valid but nonexistent, unpublished, or unverified Hub identity.
Suggested fix: Resolve the reference through the Hub verification/catalog boundary before mutating state, or rename/document the action as accepting an unverified identity.
Comment on lines
+442
to
+482
| let update = rx.recv().await.expect("action telemetry"); | ||
| let payload: Value = serde_json::from_slice(&update.data).expect("json"); | ||
| assert_eq!(payload["properties"]["target_temperature"], json!(80.0)); | ||
| transport.stop().await.expect("stop"); | ||
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn binds_and_clears_a_hub_asset_at_runtime() { | ||
| let config = SimulationConfig::default(); | ||
| let device = config.devices.first().expect("default heater"); | ||
| let (tx, mut rx) = mpsc::unbounded_channel(); | ||
| let transport = | ||
| SimulationTransport::new(config.world_id, config.engine, device, config.tick_hz, tx) | ||
| .expect("kinematic backend"); | ||
| transport.start().await.expect("start"); | ||
| let _ = rx.recv().await.expect("initial telemetry"); | ||
|
|
||
| let command = DeviceCommand { | ||
| command_id: "asset-command".into(), | ||
| device_id: device_id_for("lab-sim", &device.id), | ||
| action: "set_asset".into(), | ||
| params: json!({ | ||
| "namespace": "scienceol", | ||
| "name": "heater-dalong", | ||
| "version": "1.0.0" | ||
| }), | ||
| }; | ||
| transport | ||
| .send(&serde_json::to_vec(&command).expect("encode")) | ||
| .await | ||
| .expect("bind asset"); | ||
| let bound = rx.recv().await.expect("asset telemetry"); | ||
| let payload: Value = serde_json::from_slice(&bound.data).expect("json"); | ||
| assert_eq!( | ||
| payload["properties"]["simulation_asset"], | ||
| json!({ | ||
| "namespace": "scienceol", | ||
| "name": "heater-dalong", | ||
| "version": "1.0.0" | ||
| }) | ||
| ); |
There was a problem hiding this comment.
问题 (testing)： 测试在发送 set_asset 后读取下一条遥测消息，并假定它是操作响应；但周期性 tick 任务可能会先发出未更改的遥测快照，导致断言出现非确定性失败。
触发条件： 10 Hz 的 tick 在 send 和测试的 rx.recv() 之间运行时。
建议修复： 持续读取遥测消息，直到消息包含预期的资产绑定；或者在断言操作响应时停止 tick 任务或与其协调。
Original comment in English
issue (testing): The test reads the next telemetry message after sending set_asset and assumes it is the action response, but the periodic tick task can emit an unchanged telemetry snapshot first, causing the assertion to fail nondeterministically.
Triggers: When the 10 Hz tick runs between send and the test's rx.recv().
Suggested fix: Read telemetry until the message contains the expected asset binding, or stop/coordinate the tick task while asserting the action response.
Mile-Away
force-pushed
the
codex/simulation-hub-model-binding
branch
from
September 26, 2026 16:21
534396f to
dffda52
Compare
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.
What changed
Virtual devices can now bind a verified Hub model at runtime through the canonical
set_assetandclear_assetactions. The selected binding is emitted in telemetry assimulation_asset, so clients can keep the rendered model synchronized with the simulation process.Validation
cargo test -p osdl-core --features espnowSourcery 摘要
启用确定性的本地虚拟实验室，并通过运行时资产绑定同步其可视化 Hub 模型。
新功能：
set_asset和clear_asset支持虚拟设备的运行时 Hub 模型绑定与清除，并将选定的身份以simulation_asset遥测数据的形式公开。增强功能：
文档：
测试：
Original summary in English
Sourcery 摘要
支持模拟设备在运行时绑定 Hub 模型，并通过遥测同步所选的视觉标识。
新功能：
增强功能：
文档：
测试：
Original summary in English
Summary by Sourcery
Support runtime Hub model binding for simulated devices and synchronize the selected visual identity through telemetry.
New Features:
Enhancements:
Documentation:
Tests: