Uh oh!
There was an error while loading. Please reload this page.
fix(ostool): honor board_reset_cmd/board_power_off_cmd from uboot config - #172
Open
yfblock wants to merge 1 commit into
Open
fix(ostool): honor board_reset_cmd/board_power_off_cmd from uboot config#172yfblock wants to merge 1 commit into
yfblock wants to merge 1 commit into
Conversation
UbootConfig and the #[serde(flatten)]-ed LocalUbootConfig both declared board_reset_cmd and board_power_off_cmd. Under serde flatten the outer fields take priority, so LocalBackend (which read the flattened copy) never observed the configured values and the reset/power-off hooks silently never ran. Mark the LocalUbootConfig copies #[serde(skip)] as legacy Rust-API compatibility fields, and pass the commands into LocalBackend from UbootConfig, falling back to the legacy fields for callers that still set them directly.
There was a problem hiding this comment.
审查结论
本 PR 修复了 U-Boot 本地运行时从 UbootConfig 读取 board_reset_cmd/board_power_off_cmd 的路径:将配置示例中的命令保持在顶层,并把顶层值传入 LocalBackend,同时保留旧 Rust API 字段的回退兼容。新增测试覆盖顶层 TOML 解析、序列化去重、后端接线和旧字段回退。
影响范围
变更影响 U-Boot 本地串口启动前的 reset 命令、运行结束后的 power-off 命令,以及 U-Boot 配置的 TOML/schema 表达;中英文 README 保持同步。远程 board backend 和其他运行方式未改变,整体范围看起来是隔离的。
验证与既有意见
- PR 当前没有 GitHub check runs 或 status checks;未发现由本 PR 导致的 CI 失败。
- 已核对 PR head 为
b1946a2dbbf00d1b1d0c97fe43d7ae9a55a83a69,git diff --check通过。 - 本地无法运行
cargo fmt --check、cargo clippy --manifest-path ostool/Cargo.toml --all-features -- -D warnings和cargo test --manifest-path ostool/Cargo.toml --all-features,因为评审环境未安装cargo;PR 描述声称 313 个 lib tests 通过,但本次未能独立复现。 - 没有既有 review 或 issue comments;也未发现相关重复 PR。已考虑的验证缺口仅是缺少本地工具和真实硬件运行,未形成代码层面的未解决问题。
未发现需要阻止合并的问题,批准此次变更。
Powered by gpt-5.6-luna
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Problem
UbootConfigand the#[serde(flatten)]-edLocalUbootConfigboth declaredboard_reset_cmdandboard_power_off_cmd. Under serde's flatten semantics the outer struct's own fields take priority, so TOML top-level values are captured byUbootConfigand the flattenedLocalUbootConfigcopies stayNone.LocalBackend(the local U-Boot runner) read the flattened copies viaself.config.board_reset_cmd/self.config.board_power_off_cmd, so those hooks never observed the configured values and the reset / power-off commands silently never ran.Fix
LocalUbootConfigcopies#[serde(skip)](and#[schemars(skip)]) as legacy Rust-API compatibility fields, so they no longer participate in deserialization and cannot be shadowed.board_reset_cmd/board_power_off_cmdintoLocalBackendthrough its constructor fromUbootConfig, falling back to the legacy fields for callers that still set them directly.after_console_open/after_runnow readself.reset_cmd/self.power_off_cmd.Tests
uboot_config_parses_board_commands_at_top_level— verifies top-level parsing, single serialization of each key, and backend wiring.local_backend_keeps_legacy_command_fields_usable— verifies the legacy fallback path.All 313 lib tests pass.