Repository navigation
fix(simulation): isolate CLI from physical defaults - #19
Merged
Merged
Conversation
审查者指南该 PR 将 运行时模拟资产绑定时序图sequenceDiagram
participant Client
participant SimulationTransport
participant KinematicBackend
participant Telemetry
Client->>SimulationTransport: send(set_asset, namespace, name, version)
SimulationTransport->>SimulationTransport: validate set_asset parameters
SimulationTransport->>KinematicBackend: apply_action(set_asset, params)
KinematicBackend-->>SimulationTransport: update simulation_asset
SimulationTransport->>Telemetry: emit_status()
Telemetry-->>Client: telemetry with simulation_asset
Client->>SimulationTransport: send(clear_asset, params)
SimulationTransport->>KinematicBackend: apply_action(clear_asset, params)
KinematicBackend-->>SimulationTransport: remove simulation_asset
SimulationTransport->>Telemetry: emit_status()
Telemetry-->>Client: telemetry without simulation_asset
模拟配置隔离流程图flowchart TD
A[build_config] --> B{--simulation?}
B -->|是，无配方| C[禁用 MQTT]
C --> D[不使用物理适配器]
D --> E[创建默认 SimulationConfig]
B -->|是，有配方| F[保留配方中的模拟世界]
F --> G[跳过注册表回退]
G --> H[清除 ESP-NOW 加密狗]
B -->|否| I[使用物理默认配置]
I --> J[启用 MQTT 和 UniLabOS 适配器]
I --> K[允许注册表和 ESP-NOW 配置]
文件级变更
提示和命令与 Sourcery 交互
自定义你的使用体验访问你的控制面板以：
获取帮助Original review guide in EnglishReviewer's GuideThe PR isolates --simulation startup from MQTT, registry, and ESP-NOW physical defaults while preserving explicit recipe worlds, and adds validated runtime asset binding for virtual devices with telemetry and documentation support. Sequence diagram for runtime simulation asset bindingsequenceDiagram
participant Client
participant SimulationTransport
participant KinematicBackend
participant Telemetry
Client->>SimulationTransport: send(set_asset, namespace, name, version)
SimulationTransport->>SimulationTransport: validate set_asset parameters
SimulationTransport->>KinematicBackend: apply_action(set_asset, params)
KinematicBackend-->>SimulationTransport: update simulation_asset
SimulationTransport->>Telemetry: emit_status()
Telemetry-->>Client: telemetry with simulation_asset
Client->>SimulationTransport: send(clear_asset, params)
SimulationTransport->>KinematicBackend: apply_action(clear_asset, params)
KinematicBackend-->>SimulationTransport: remove simulation_asset
SimulationTransport->>Telemetry: emit_status()
Telemetry-->>Client: telemetry without simulation_asset
Flow diagram for simulation configuration isolationflowchart TD
A[build_config] --> B{--simulation?}
B -->|yes, no recipe| C[Disable MQTT]
C --> D[Use no physical adapters]
D --> E[Create default SimulationConfig]
B -->|yes, recipe| F[Preserve recipe simulation world]
F --> G[Skip registry fallback]
G --> H[Clear ESP-NOW dongles]
B -->|no| I[Use physical defaults]
I --> J[Enable MQTT and UniLabOS adapter]
I --> K[Allow registry and ESP-NOW configuration]
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.
您好——我发现了 1 个问题
面向 AI Agent 的提示
请处理本次代码审查中的评论：
## 个别评论
### 评论 1
<location path="crates/lab-cli/src/commands/serve.rs" line_range="481-484" />
<code_context>
a.registry_path = Some(reg_str.clone());
}
}
- } else {
+ } else if !args.simulation {
// Final fallback for adapters with no registry configured — pick
// the registry that ships next to the config file. With a config
</code_context>
<issue_to_address>
**issue (broader_impact)：** `--simulation --registry PATH` 仍会添加一个 `unilabos` 适配器，导致引擎加载物理注册表，尽管模拟模式的设计目标是将 CLI 与物理默认设置隔离。模拟模式会明确忽略 dongle 覆盖设置，但等效的注册表覆盖设置却不会被忽略。
**触发条件：** 将 `--simulation` 与 `--registry` 结合使用时。
**建议修复：** 在模拟模式下忽略 `--registry`，或者像处理 dongle 覆盖设置一样，在两者组合使用时给出明确警告并拒绝该组合。
</issue_to_address>Original comment in English
Hey - I've found 1 issue
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="481-484" />
<code_context>
a.registry_path = Some(reg_str.clone());
}
}
- } else {
+ } else if !args.simulation {
// Final fallback for adapters with no registry configured — pick
// the registry that ships next to the config file. With a config
</code_context>
<issue_to_address>
**issue (broader_impact):** `--simulation --registry PATH` still adds a `unilabos` adapter, causing the engine to load a physical registry even though simulation mode is intended to isolate the CLI from physical defaults. The dongle override is explicitly ignored in simulation, but the equivalent registry override is not.
**Triggers:** When `--simulation` is combined with `--registry`.
**Suggested fix:** Ignore `--registry` in simulation mode, or reject the combination with a clear warning, just as the dongle override is handled.
</issue_to_address>
Comment on lines
481
to
+484
| a.registry_path = Some(reg_str.clone()); | ||
| } | ||
| } | ||
| } else { | ||
| } else if !args.simulation { |
There was a problem hiding this comment.
issue (broader_impact)： --simulation --registry PATH 仍会添加一个 unilabos 适配器，导致引擎加载物理注册表，尽管模拟模式的设计目标是将 CLI 与物理默认设置隔离。模拟模式会明确忽略 dongle 覆盖设置，但等效的注册表覆盖设置却不会被忽略。
触发条件： 将 --simulation 与 --registry 结合使用时。
建议修复： 在模拟模式下忽略 --registry，或者像处理 dongle 覆盖设置一样，在两者组合使用时给出明确警告并拒绝该组合。
Original comment in English
issue (broader_impact): --simulation --registry PATH still adds a unilabos adapter, causing the engine to load a physical registry even though simulation mode is intended to isolate the CLI from physical defaults. The dongle override is explicitly ignored in simulation, but the equivalent registry override is not.
Triggers: When --simulation is combined with --registry.
Suggested fix: Ignore --registry in simulation mode, or reject the combination with a clear warning, just as the dongle override is handled.
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
lab serve --simulationnow starts a pure virtual lab when no recipe is provided. It no longer enables the MQTT broker, loads the default UniLabOS registry, or opens ESP-NOW hardware by accident. A recipe's explicit simulation world is preserved when the flag is used, while physical dongles are ignored with a clear warning.Validation
cargo test --workspacelab serve --simulationfrom a directory without a registry; the server stayed up without registry-load errors.Sourcery 摘要
将模拟模式与实体实验室默认设置隔离，并支持在运行时为虚拟设备绑定视觉资源。
新功能：
错误修复：
文档：
测试：
Original summary in English
Sourcery 总结
将模拟模式与物理实验室默认设置隔离，并支持为虚拟设备运行时绑定视觉资产。
新功能：
错误修复：
文档：
测试：
Original summary in English
Summary by Sourcery
Isolate simulation mode from physical lab defaults and support runtime visual asset bindings for virtual devices.
New Features:
Bug Fixes:
Documentation:
Tests: