Repository navigation
feat(simulation): add deterministic device reset #20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
+65
−1
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
问题 (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
resetreleases the state mutex, the ticker can advance the state and enqueue telemetry beforeemit_statusruns, 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.