Repository navigation
feat(simulation): add local virtual lab runtime - #17
Merged
Merged
Conversation
审查者指南此 PR 通过 CLI 或环境变量启用一个本地托管的、确定性的虚拟实验室；通过与实体硬件相同的 OpenSDL 设备/传输/适配器/事件路径注册虚拟设备；并提供可注入的后端接口，以支持未来的物理引擎运行时，同时保留可选的 Hub 资产标识作为遥测元数据。 模拟设备命令和遥测的时序图sequenceDiagram
participant User
participant CLI as lab CLI
participant Engine as OsdlEngine
participant Transport as SimulationTransport
participant Backend as SimulationBackend
participant Adapter as SimulationAdapter
participant Client as Agent/UI
User->>CLI: serve --simulation
CLI->>Engine: start_simulation()
Engine->>Transport: start()
Transport->>Client: DeviceStatus telemetry
Client->>Engine: DeviceCommand
Engine->>Adapter: encode_command()
Adapter->>Transport: JSON command bytes
Transport->>Backend: apply_action()
Backend-->>Transport: updated SimulationState
Transport->>Adapter: JSON telemetry envelope
Adapter->>Engine: decode_response()
Engine-->>Client: status and events
文件级变更
提示和命令与 Sourcery 交互
自定义你的使用体验访问你的控制面板以：
获取帮助Original review guide in EnglishReviewer's GuideThis PR adds a locally hosted, deterministic virtual lab activated through the CLI or environment, registers virtual devices through the same OpenSDL device/transport/adapter/event path as physical hardware, and exposes injectable backend interfaces for future physics runtimes while preserving optional Hub asset identity as telemetry metadata. Sequence diagram for simulated device commands and telemetrysequenceDiagram
participant User
participant CLI as lab CLI
participant Engine as OsdlEngine
participant Transport as SimulationTransport
participant Backend as SimulationBackend
participant Adapter as SimulationAdapter
participant Client as Agent/UI
User->>CLI: serve --simulation
CLI->>Engine: start_simulation()
Engine->>Transport: start()
Transport->>Client: DeviceStatus telemetry
Client->>Engine: DeviceCommand
Engine->>Adapter: encode_command()
Adapter->>Transport: JSON command bytes
Transport->>Backend: apply_action()
Backend-->>Transport: updated SimulationState
Transport->>Adapter: JSON telemetry envelope
Adapter->>Engine: decode_response()
Engine-->>Client: status and events
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.
嘿——我发现了 3 个问题
面向 AI Agent 的提示
请处理本次代码审查中的评论：
## 各条评论
### 评论 1
<location path="crates/lab-cli/src/commands/serve.rs" line_range="501-503" />
<code_context>
}
}
+ if args.simulation {
+ cfg.simulation = Some(SimulationConfig::default());
+ }
+
Ok(cfg)
</code_context>
<issue_to_address>
**issue (broader_impact):** 启用 `--simulation` 只会添加 `SimulationConfig`；它不会禁用已配置的 ESP-NOW 加密狗或由 MQTT 支持的物理设备发现。因此，使用包含硬件传输配置的配置文件运行模拟模式服务器时，仍然会发现并能够控制物理设备，这与文档所描述的隔离本地模式不符。
**触发条件：** 提供的配置包含 ESP-NOW 加密狗或其他物理设备配置时。
**建议修复：** 启用模拟模式后，应拒绝物理传输，或明确禁用这些传输，除非用户选择混合硬件/模拟运行模式。
</issue_to_address>
### 评论 2
<location path="crates/osdl-core/src/engine.rs" line_range="780-802" />
<code_context>
+ Ok(())
+ }
+
+ async fn stop_simulation(&self) {
+ let Some(config) = self.handle.config.simulation.as_ref() else {
+ return;
+ };
+ let transport_list = {
+ let transports = self.handle.transports.read().await;
+ config
+ .devices
+ .iter()
+ .filter_map(|device| {
+ let id = simulation_transport_id(&config.world_id, &device.id);
+ transports
+ .get(&id)
+ .cloned()
+ .map(|transport| (id, transport))
+ })
+ .collect::<Vec<_>>()
+ };
+ for (id, transport) in transport_list {
+ if let Err(error) = transport.stop().await {
+ log::warn!("Failed to stop simulation transport {id}: {error}");
+ }
+ }
+ }
</code_context>
<issue_to_address>
**issue (bug_risk):** 停止模拟只会停止各个传输，却不会从引擎注册表中移除模拟传输或设备。因此，`run()` 返回后会留下过时的在线设备和传输条目，并导致后续运行或检查暴露出已停止的设备。
**触发条件：** 引擎停止但其句柄仍被保留，或再次运行同一个引擎时。
**建议修复：** 在关闭过程中从各自的注册表中移除模拟传输和设备，并在返回前将它们标记为离线或发出离线事件。
</issue_to_address>
### 评论 3
<location path="crates/osdl-core/src/transport/simulation.rs" line_range="265" />
<code_context>
+ let mut properties = state.properties;
+ properties.insert("step".into(), json!(state.step));
+ let envelope = json!({
+ "device_id": self.inner.device_id,
+ "world_id": self.inner.world_id,
+ "engine": self.inner.backend.engine_id(),
+ "role": self.inner.role,
+ "position": self.inner.position,
+ "timestamp": now_millis(),
+ "last_action": state.last_action,
+ "properties": properties,
+ });
+ let _ = self.inner.rx_tx.send(TransportRx {
+ transport_id: self.inner.transport_id.clone(),
</code_context>
<issue_to_address>
**issue (bug_risk):** 每个模拟遥测信封都使用墙上时钟的 `now_millis()` 值，因此即使模拟运行完全相同，生成的遥测数据和事件存储输出也会不同，这违反了所宣称的确定性运行特性。
**触发条件：** 客户端或回放/测试比较多次运行中的完整遥测信封时。
**建议修复：** 使用基于 `step` 和 `tick_hz` 计算的确定性模拟时间戳；或者从确定性状态中省略时间戳，并在模拟载荷之外添加基于墙上时钟的接收时间。
```suggestion
"timestamp": state.step.saturating_mul(1000) / self.inner.tick_hz as u64,
```
</issue_to_address>Original comment in English
Hey - I've found 3 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="crates/lab-cli/src/commands/serve.rs" line_range="501-503" />
<code_context>
}
}
+ if args.simulation {
+ cfg.simulation = Some(SimulationConfig::default());
+ }
+
Ok(cfg)
</code_context>
<issue_to_address>
**issue (broader_impact):** Enabling `--simulation` only adds `SimulationConfig`; it does not disable configured ESP-NOW dongles or MQTT-backed physical discovery, so a simulation-mode server using a config with hardware transports still discovers and can control physical devices despite the documented isolated local mode.
**Triggers:** When the supplied config contains ESP-NOW dongles or other physical-device configuration.
**Suggested fix:** When simulation mode is enabled, either reject physical transports or explicitly disable them unless the user opts into mixed hardware/simulation operation.
</issue_to_address>
### Comment 2
<location path="crates/osdl-core/src/engine.rs" line_range="780-802" />
<code_context>
+ Ok(())
+ }
+
+ async fn stop_simulation(&self) {
+ let Some(config) = self.handle.config.simulation.as_ref() else {
+ return;
+ };
+ let transport_list = {
+ let transports = self.handle.transports.read().await;
+ config
+ .devices
+ .iter()
+ .filter_map(|device| {
+ let id = simulation_transport_id(&config.world_id, &device.id);
+ transports
+ .get(&id)
+ .cloned()
+ .map(|transport| (id, transport))
+ })
+ .collect::<Vec<_>>()
+ };
+ for (id, transport) in transport_list {
+ if let Err(error) = transport.stop().await {
+ log::warn!("Failed to stop simulation transport {id}: {error}");
+ }
+ }
+ }
</code_context>
<issue_to_address>
**issue (bug_risk):** Stopping a simulation stops each transport but never removes the simulation transports or devices from the engine registries, leaving stale online devices and transport entries after `run()` returns and causing a subsequent run or inspection to expose stopped devices.
**Triggers:** When the engine is stopped and its handle is retained, or when the same engine is run again.
**Suggested fix:** Remove the simulation transports and devices from their registries during shutdown, and mark or emit them offline before returning.
</issue_to_address>
### Comment 3
<location path="crates/osdl-core/src/transport/simulation.rs" line_range="265" />
<code_context>
+ let mut properties = state.properties;
+ properties.insert("step".into(), json!(state.step));
+ let envelope = json!({
+ "device_id": self.inner.device_id,
+ "world_id": self.inner.world_id,
+ "engine": self.inner.backend.engine_id(),
+ "role": self.inner.role,
+ "position": self.inner.position,
+ "timestamp": now_millis(),
+ "last_action": state.last_action,
+ "properties": properties,
+ });
+ let _ = self.inner.rx_tx.send(TransportRx {
+ transport_id: self.inner.transport_id.clone(),
</code_context>
<issue_to_address>
**issue (bug_risk):** Every simulation telemetry envelope uses the wall-clock `now_millis()` value, so otherwise identical simulation runs produce different telemetry and event-store output, violating the advertised deterministic runtime.
**Triggers:** When clients or replay/tests compare complete telemetry envelopes across runs.
**Suggested fix:** Use a deterministic simulation timestamp derived from `step` and `tick_hz`, or omit timestamps from deterministic state and add wall-clock receipt time outside the simulation payload.
```suggestion
"timestamp": state.step.saturating_mul(1000) / self.inner.tick_hz as u64,
```
</issue_to_address>
Comment on lines
+501
to
+503
| if args.simulation { | ||
| cfg.simulation = Some(SimulationConfig::default()); | ||
| } |
There was a problem hiding this comment.
issue (broader_impact): 启用 --simulation 只会添加 SimulationConfig；它不会禁用已配置的 ESP-NOW 加密狗或由 MQTT 支持的物理设备发现。因此，使用包含硬件传输配置的配置文件运行模拟模式服务器时，仍然会发现并能够控制物理设备，这与文档所描述的隔离本地模式不符。
触发条件： 提供的配置包含 ESP-NOW 加密狗或其他物理设备配置时。
建议修复： 启用模拟模式后，应拒绝物理传输，或明确禁用这些传输，除非用户选择混合硬件/模拟运行模式。
Original comment in English
issue (broader_impact): Enabling --simulation only adds SimulationConfig; it does not disable configured ESP-NOW dongles or MQTT-backed physical discovery, so a simulation-mode server using a config with hardware transports still discovers and can control physical devices despite the documented isolated local mode.
Triggers: When the supplied config contains ESP-NOW dongles or other physical-device configuration.
Suggested fix: When simulation mode is enabled, either reject physical transports or explicitly disable them unless the user opts into mixed hardware/simulation operation.
Comment on lines
+780
to
+802
| async fn stop_simulation(&self) { | ||
| let Some(config) = self.handle.config.simulation.as_ref() else { | ||
| return; | ||
| }; | ||
| let transport_list = { | ||
| let transports = self.handle.transports.read().await; | ||
| config | ||
| .devices | ||
| .iter() | ||
| .filter_map(|device| { | ||
| let id = simulation_transport_id(&config.world_id, &device.id); | ||
| transports | ||
| .get(&id) | ||
| .cloned() | ||
| .map(|transport| (id, transport)) | ||
| }) | ||
| .collect::<Vec<_>>() | ||
| }; | ||
| for (id, transport) in transport_list { | ||
| if let Err(error) = transport.stop().await { | ||
| log::warn!("Failed to stop simulation transport {id}: {error}"); | ||
| } | ||
| } |
There was a problem hiding this comment.
issue (bug_risk): 停止模拟只会停止各个传输，却不会从引擎注册表中移除模拟传输或设备。因此，run() 返回后会留下过时的在线设备和传输条目，并导致后续运行或检查暴露出已停止的设备。
触发条件： 引擎停止但其句柄仍被保留，或再次运行同一个引擎时。
建议修复： 在关闭过程中从各自的注册表中移除模拟传输和设备，并在返回前将它们标记为离线或发出离线事件。
Original comment in English
issue (bug_risk): Stopping a simulation stops each transport but never removes the simulation transports or devices from the engine registries, leaving stale online devices and transport entries after run() returns and causing a subsequent run or inspection to expose stopped devices.
Triggers: When the engine is stopped and its handle is retained, or when the same engine is run again.
Suggested fix: Remove the simulation transports and devices from their registries during shutdown, and mark or emit them offline before returning.
| "engine": self.inner.backend.engine_id(), | ||
| "role": self.inner.role, | ||
| "position": self.inner.position, | ||
| "timestamp": now_millis(), |
There was a problem hiding this comment.
issue (bug_risk): 每个模拟遥测信封都使用墙上时钟的 now_millis() 值，因此即使模拟运行完全相同，生成的遥测数据和事件存储输出也会不同，这违反了所宣称的确定性运行特性。
触发条件： 客户端或回放/测试比较多次运行中的完整遥测信封时。
建议修复： 使用基于 step 和 tick_hz 计算的确定性模拟时间戳；或者从确定性状态中省略时间戳，并在模拟载荷之外添加基于墙上时钟的接收时间。
Suggested change
| "timestamp": now_millis(), | |
| "timestamp": state.step.saturating_mul(1000) / self.inner.tick_hz as u64, |
Original comment in English
issue (bug_risk): Every simulation telemetry envelope uses the wall-clock now_millis() value, so otherwise identical simulation runs produce different telemetry and event-store output, violating the advertised deterministic runtime.
Triggers: When clients or replay/tests compare complete telemetry envelopes across runs.
Suggested fix: Use a deterministic simulation timestamp derived from step and tick_hz, or omit timestamps from deterministic state and add wall-clock receipt time outside the simulation payload.
Suggested change
| "timestamp": now_millis(), | |
| "timestamp": state.step.saturating_mul(1000) / self.inner.tick_hz as u64, |
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
OpenSDL can now boot a deterministic local virtual lab with
lab serve --simulationorOSDL_SIMULATION=true. Virtual heater, stirrer, valve, and probe devices use the existing Device/DeviceCommand/DeviceStatus/event/gRPC path, so Runner, Agent, and UI development does not require physical hardware.The simulation runtime has a public
SimulationBackendandSimulationBackendFactoryseam. Kinematic behavior is included; native Rapier, Bullet, MuJoCo, Isaac Sim, and Gazebo integrations can be injected without changing the OpenSDL device contract. Devices can also carry a pinned Hubasset_ref; telemetry exposes that identity without copying model bytes into OpenSDL.Validation
cargo fmt --allcargo test -p osdl-core --features espnowcargo test -p lab-cli --features espnow --no-runSourcery 总结
支持无需实体硬件即可进行本地确定性虚拟实验室开发。
新功能：
增强功能：
文档：
测试：
Original summary in English
Summary by Sourcery
Enable local deterministic virtual lab development without requiring physical hardware.
New Features:
Enhancements:
Documentation:
Tests: