Repository navigation
feat(simulation): add deterministic device reset - #20
Merged
Merged
Conversation
审查者指南模拟设备现在会保留其配置的初始属性，并提供确定性的重置操作。重置行为通过可重写的后端钩子实现，以便未来的物理引擎集成可以在重置 OpenSDL 遥测数据的同时重置原生状态；同时，测试覆盖了属性和步数的恢复。 确定性模拟设备重置的时序图sequenceDiagram
participant Client
participant OsdlEngine
participant SimulationTransport
participant KinematicBackend
participant Telemetry
OsdlEngine->>OsdlEngine: register_device
Client->>SimulationTransport: send(action=reset)
SimulationTransport->>KinematicBackend: apply_action(state, reset, params)
KinematicBackend->>KinematicBackend: reset(state)
KinematicBackend->>SimulationTransport: restore initial_properties and step=0
SimulationTransport->>Telemetry: publish reset state
Telemetry-->>Client: properties restored, last_action=reset
文件级变更
提示和命令与 Sourcery 交互
自定义使用体验访问你的控制面板以：
获取帮助Original review guide in EnglishReviewer's GuideSimulation devices now retain their configured initial properties and expose a deterministic reset action. Reset behavior is implemented through an overridable backend hook so future physics integrations can reset native state alongside OpenSDL telemetry, with tests covering property and step restoration. Sequence diagram for deterministic simulation device resetsequenceDiagram
participant Client
participant OsdlEngine
participant SimulationTransport
participant KinematicBackend
participant Telemetry
OsdlEngine->>OsdlEngine: register_device
Client->>SimulationTransport: send(action=reset)
SimulationTransport->>KinematicBackend: apply_action(state, reset, params)
KinematicBackend->>KinematicBackend: reset(state)
KinematicBackend->>SimulationTransport: restore initial_properties and step=0
SimulationTransport->>Telemetry: publish reset state
Telemetry-->>Client: properties restored, last_action=reset
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/osdl-core/src/transport/simulation.rs" line_range="63-66" />
<code_context>
+
+ /// Restore the device's configured state. Physics integrations can
+ /// override this to reset their world state alongside the OpenSDL view.
+ async fn reset(&self, state: &mut SimulationState) {
+ state.properties = state.initial_properties.clone();
+ state.step = 0;
+ state.last_action = Some("reset".into());
+ }
}
</code_context>
<issue_to_address>
**问题 (bug_risk)：** 重置状态的修改与重置遥测的发送不是原子的：`reset` 释放状态互斥锁后，ticker 可能会在 `emit_status` 执行前推进状态并加入遥测消息，因此重置响应报告的可能是非零步数或演化后的属性，而不是重置快照。新测试同样会消费下一条遥测消息，因此可能读到这次介入的 tick，并间歇性失败。
**触发条件：** 模拟 ticker 在动作应用与状态发送之间触发时。
**建议修复：** 在单一的命令/遥测同步机制下，确保重置及其对应的状态发送按顺序执行；同时让测试等待 `last_action == "reset"` 的遥测消息，而不是消费任意一条排队中的消息。
</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/osdl-core/src/transport/simulation.rs" line_range="63-66" />
<code_context>
+
+ /// Restore the device's configured state. Physics integrations can
+ /// override this to reset their world state alongside the OpenSDL view.
+ async fn reset(&self, state: &mut SimulationState) {
+ state.properties = state.initial_properties.clone();
+ state.step = 0;
+ state.last_action = Some("reset".into());
+ }
}
</code_context>
<issue_to_address>
**issue (bug_risk):** Reset state mutation and reset telemetry emission are not atomic: after `reset` releases the state mutex, the ticker can advance the state and enqueue telemetry before `emit_status` runs, so the reset response reports a nonzero step or evolved properties instead of the reset snapshot. The new test also consumes whichever telemetry message is next, so it can read that intervening tick and fail intermittently.
**Triggers:** When the simulation ticker fires between action application and status emission.
**Suggested fix:** Keep reset and its corresponding status emission ordered under a single command/telemetry synchronization mechanism, and make the test wait for telemetry with `last_action == "reset"` rather than consuming an arbitrary queued message.
</issue_to_address>
Comment on lines
+63
to
+66
| async fn reset(&self, state: &mut SimulationState) { | ||
| state.properties = state.initial_properties.clone(); | ||
| state.step = 0; | ||
| state.last_action = Some("reset".into()); |
There was a problem hiding this comment.
问题 (bug_risk)： 重置状态的修改与重置遥测的发送不是原子的：reset 释放状态互斥锁后，ticker 可能会在 emit_status 执行前推进状态并加入遥测消息，因此重置响应报告的可能是非零步数或演化后的属性，而不是重置快照。新测试同样会消费下一条遥测消息，因此可能读到这次介入的 tick，并间歇性失败。
触发条件： 模拟 ticker 在动作应用与状态发送之间触发时。
建议修复： 在单一的命令/遥测同步机制下，确保重置及其对应的状态发送按顺序执行；同时让测试等待 last_action == "reset" 的遥测消息，而不是消费任意一条排队中的消息。
Original comment in English
issue (bug_risk): Reset state mutation and reset telemetry emission are not atomic: after reset releases the state mutex, the ticker can advance the state and enqueue telemetry before emit_status runs, so the reset response reports a nonzero step or evolved properties instead of the reset snapshot. The new test also consumes whichever telemetry message is next, so it can read that intervening tick and fail intermittently.
Triggers: When the simulation ticker fires between action application and status emission.
Suggested fix: Keep reset and its corresponding status emission ordered under a single command/telemetry synchronization mechanism, and make the test wait for telemetry with last_action == "reset" rather than consuming an arbitrary queued message.
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
Simulation transports now retain configured initial properties and expose a deterministic
resetaction. The backend reset hook is overridable so future physics engines can reset native world state together with OpenSDL telemetry.Every configured simulation device advertises the action when it is not already present.
Validation
cargo fmt --allcargo test -p osdl-core --features espnow(123 unit, 6 e2e MQTT, 18 integration)Sourcery 总结
为模拟传输添加确定性的设备重置支持。
新功能：
增强功能：
测试：
Original summary in English
Summary by Sourcery
Add deterministic device reset support to simulation transports.
New Features:
Enhancements:
Tests: