From 1a0472c5495df63cb2f4ade35504199b8339f70a Mon Sep 17 00:00:00 2001 From: Dylan Knutson Date: Wed, 15 Apr 2026 11:35:46 -0700 Subject: [PATCH 1/8] fix: use explicit host target for cargo test in pre-commit hook cargo test without --target fails locally when rust-toolchain.toml installs aarch64-unknown-none-softfloat, because no_std-incompatible transitive deps (futures-timer, slab) are resolved for that target. Using the host target from rustc -vV ensures portability across Linux, macOS, and Windows. --- .husky/pre-commit | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/.husky/pre-commit b/.husky/pre-commit index c7ca1af..c8f1100 100755 --- a/.husky/pre-commit +++ b/.husky/pre-commit @@ -43,11 +43,17 @@ check_rustup_target "aarch64-unknown-none" set -x # cargo deny --log-level=warn check -cargo clippy --target=aarch64-unknown-none -p ihv1-ec-sp -p qemu-ec-sp -cargo clippy +cargo clippy --target=aarch64-unknown-none -p ihv1-ec-sp -p qemu-ec-sp -- -D warnings +cargo clippy -- -D warnings cargo fmt --check cargo hack check --feature-powerset --exclude=ihv1-ec-sp --exclude=qemu-ec-sp cargo hack check --feature-powerset --target=aarch64-unknown-none -cargo test -q +HOST_TARGET=$(rustc -vV | sed -n 's/host: //p') +if [ -z "$HOST_TARGET" ]; then + echo "Failed to determine Rust host target from \`rustc -vV\` output." + echo "Please verify that \`rustc -vV\` prints a valid \`host:\` line." + exit 1 +fi +cargo test --target "$HOST_TARGET" -q cd "$PROJECT_DIR/platform/qemu-sp" && cargo build --target=aarch64-unknown-none cd "$PROJECT_DIR/platform/ihv1-sp" && cargo build --target=aarch64-unknown-none From 19b08071acce648fddc18638744a430085ddb213 Mon Sep 17 00:00:00 2001 From: Dylan Knutson Date: Tue, 14 Apr 2026 18:54:21 -0700 Subject: [PATCH 2/8] feat(02-01): add Battery state struct and response types - Battery struct with all IHV1 ec_memory fields and realistic defaults - BixRsp (11-field), PsrRsp, PifRsp, StaRsp, ValueRsp response structs - Bidirectional DirectMessagePayload conversion for all response types - Manual Default impl delegating to new() (thermal pattern) - BST now reads from struct state fields instead of hardcoded literals - Existing BstRsp and GenericRsp unchanged --- platform/qemu-sp/src/battery.rs | 220 +++++++++++++++++++++++++++++++- 1 file changed, 213 insertions(+), 7 deletions(-) diff --git a/platform/qemu-sp/src/battery.rs b/platform/qemu-sp/src/battery.rs index cf3aeb0..dad2f78 100644 --- a/platform/qemu-sp/src/battery.rs +++ b/platform/qemu-sp/src/battery.rs @@ -57,20 +57,226 @@ impl From<&DirectMessagePayload> for BstRsp { } } -#[derive(Default)] -pub struct Battery {} +struct BixRsp { + events: u32, + status: u32, + last_full_charge: u32, + cycle_count: u32, + state: u32, + present_rate: u32, + remain_cap: u32, + present_volt: u32, + psr_state: u32, + psr_max_out: u32, + psr_max_in: u32, +} + +impl From for DirectMessagePayload { + fn from(value: BixRsp) -> Self { + let regs = [ + value.events, + value.status, + value.last_full_charge, + value.cycle_count, + value.state, + value.present_rate, + value.remain_cap, + value.present_volt, + value.psr_state, + value.psr_max_out, + value.psr_max_in, + ]; + DirectMessagePayload::from_iter(regs.iter().flat_map(|®| u32::to_le_bytes(reg))) + } +} + +impl From<&DirectMessagePayload> for BixRsp { + fn from(payload: &DirectMessagePayload) -> Self { + BixRsp { + events: payload.u32_at(0), + status: payload.u32_at(4), + last_full_charge: payload.u32_at(8), + cycle_count: payload.u32_at(12), + state: payload.u32_at(16), + present_rate: payload.u32_at(20), + remain_cap: payload.u32_at(24), + present_volt: payload.u32_at(28), + psr_state: payload.u32_at(32), + psr_max_out: payload.u32_at(36), + psr_max_in: payload.u32_at(40), + } + } +} + +struct PsrRsp { + psr_state: u32, +} + +impl From for DirectMessagePayload { + fn from(value: PsrRsp) -> Self { + DirectMessagePayload::from_iter(value.psr_state.to_le_bytes()) + } +} + +impl From<&DirectMessagePayload> for PsrRsp { + fn from(payload: &DirectMessagePayload) -> Self { + PsrRsp { + psr_state: payload.u32_at(0), + } + } +} + +struct PifRsp { + max_power: u32, +} + +impl From for DirectMessagePayload { + fn from(value: PifRsp) -> Self { + DirectMessagePayload::from_iter(value.max_power.to_le_bytes()) + } +} + +impl From<&DirectMessagePayload> for PifRsp { + fn from(payload: &DirectMessagePayload) -> Self { + PifRsp { + max_power: payload.u32_at(0), + } + } +} + +struct StaRsp { + sta_status: u32, +} + +impl From for DirectMessagePayload { + fn from(value: StaRsp) -> Self { + DirectMessagePayload::from_iter(value.sta_status.to_le_bytes()) + } +} + +impl From<&DirectMessagePayload> for StaRsp { + fn from(payload: &DirectMessagePayload) -> Self { + StaRsp { + sta_status: payload.u32_at(0), + } + } +} + +struct ValueRsp { + value: u32, +} + +impl From for DirectMessagePayload { + fn from(value: ValueRsp) -> Self { + DirectMessagePayload::from_iter(value.value.to_le_bytes()) + } +} + +impl From<&DirectMessagePayload> for ValueRsp { + fn from(payload: &DirectMessagePayload) -> Self { + ValueRsp { + value: payload.u32_at(0), + } + } +} + +#[allow(dead_code)] +pub struct Battery { + // BIX fields + events: u32, + status: u32, + last_full_charge: u32, + cycle_count: u32, + // BST fields (used by get_bst) + state: u32, + present_rate: u32, + remain_cap: u32, + present_volt: u32, + // PSR fields + psr_state: u32, + psr_max_out: u32, + psr_max_in: u32, + // BPT/BPC fields + peak_level: u32, + peak_power: u32, + sus_level: u32, + sus_power: u32, + peak_thres: u32, + sus_thres: u32, + // BTP field (settable) + trip_thres: u32, + // BMC/BMD fields + bmc_data: u32, + bmd_data: u32, + bmd_flags: u32, + bmd_count: u32, + // BCT/BTM/BMS/BMA fields + charge_time: u32, + run_time: u32, + sample_time: u32, + // STA field + sta_status: u32, + // PIF fields + pif_max_power: u32, + // BPS field + bps_status: u32, + // BMA field + bma_data: u32, + // BMS field + bms_data: u32, + // BTM field + btm_temp: u32, +} + +impl Default for Battery { + fn default() -> Self { + Self::new() + } +} impl Battery { pub fn new() -> Self { - Self::default() + Battery { + events: 0, + status: 0, // no pending events + last_full_charge: 4500, // mWh + cycle_count: 42, + state: 0x1, // discharging + present_rate: 500, // mW draw + remain_cap: 5000, // mWh remaining + present_volt: 12000, // 12V in mV + psr_state: 0x1, // AC adapter present + psr_max_out: 65000, // 65W max output (mW) + psr_max_in: 0, + peak_level: 100, // percentage + peak_power: 45000, // mW + sus_level: 30, + sus_power: 15000, + peak_thres: 80, + sus_thres: 20, + trip_thres: 10, // default trip point at 10% + bmc_data: 0, + bmd_data: 0, + bmd_flags: 0, + bmd_count: 0, + charge_time: 120, // minutes to full + run_time: 300, // minutes remaining + sample_time: 1000, // ms between samples + sta_status: 0x1F, // present + enabled + functional + show in UI + pif_max_power: 65000, // mW + bps_status: 0x1, // battery physically present + bma_data: 0, + bms_data: 0x1, // managed + btm_temp: 2980, // 298.0K (25°C in tenths of Kelvin) + } } fn get_bst(&self, _msg: &MsgSendDirectReq2) -> BstRsp { BstRsp { - state: 0x1, // Battery discharging - present_rate: 500, // Power being supplied to battery - remaining_cap: 5000, // Remaining capacity of battery - present_volt: 12000, // 12V or 12000mV + state: self.state, + present_rate: self.present_rate, + remaining_cap: self.remain_cap, + present_volt: self.present_volt, } } From 6df0a5289f0da0a289b0bf0d398168bba60be8bc Mon Sep 17 00:00:00 2001 From: Dylan Knutson Date: Tue, 14 Apr 2026 18:56:13 -0700 Subject: [PATCH 3/8] feat(02-01): implement all 15 battery opcode methods and wire dispatch - All 15 match arms dispatch to real methods (zero generic_test refs) - get_bix returns BixRsp with 11 fields from Battery state - get_bst reads from struct fields (not hardcoded literals) - get_psr/get_pif/get_sta use dedicated response structs - handle_btp/handle_bmc support set-then-get via payload u32_at(4) - 10 get-only opcodes use ValueRsp reading from Battery state - Removed GenericRsp and generic_test (dead code) - cargo clippy -D warnings clean, cargo fmt clean - Existing battery_get_bst_works test passes --- platform/qemu-sp/src/battery.rs | 123 +++++++++++++++++++++++++------- 1 file changed, 96 insertions(+), 27 deletions(-) diff --git a/platform/qemu-sp/src/battery.rs b/platform/qemu-sp/src/battery.rs index dad2f78..fbc0a10 100644 --- a/platform/qemu-sp/src/battery.rs +++ b/platform/qemu-sp/src/battery.rs @@ -20,17 +20,6 @@ const EC_BAT_GET_BMS: u8 = 0xd; const EC_BAT_GET_BMA: u8 = 0xe; const EC_BAT_GET_STA: u8 = 0xf; -#[derive(Default)] -struct GenericRsp { - status: i64, -} - -impl From for DirectMessagePayload { - fn from(value: GenericRsp) -> Self { - DirectMessagePayload::from_iter(value.status.to_le_bytes()) - } -} - #[derive(Default)] struct BstRsp { state: u32, @@ -271,6 +260,22 @@ impl Battery { } } + fn get_bix(&self, _msg: &MsgSendDirectReq2) -> BixRsp { + BixRsp { + events: self.events, + status: self.status, + last_full_charge: self.last_full_charge, + cycle_count: self.cycle_count, + state: self.state, + present_rate: self.present_rate, + remain_cap: self.remain_cap, + present_volt: self.present_volt, + psr_state: self.psr_state, + psr_max_out: self.psr_max_out, + psr_max_in: self.psr_max_in, + } + } + fn get_bst(&self, _msg: &MsgSendDirectReq2) -> BstRsp { BstRsp { state: self.state, @@ -280,8 +285,72 @@ impl Battery { } } - fn generic_test(&self, _msg: &MsgSendDirectReq2) -> GenericRsp { - GenericRsp { status: 0x0 } + fn get_psr(&self, _msg: &MsgSendDirectReq2) -> PsrRsp { + PsrRsp { + psr_state: self.psr_state, + } + } + + fn get_pif(&self, _msg: &MsgSendDirectReq2) -> PifRsp { + PifRsp { + max_power: self.pif_max_power, + } + } + + fn get_bps(&self, _msg: &MsgSendDirectReq2) -> ValueRsp { + ValueRsp { value: self.bps_status } + } + + fn handle_btp(&mut self, msg: &MsgSendDirectReq2) -> ValueRsp { + let set_value = msg.payload().u32_at(4); + if set_value != 0 { + self.trip_thres = set_value; + } + ValueRsp { value: self.trip_thres } + } + + fn get_bpt(&self, _msg: &MsgSendDirectReq2) -> ValueRsp { + ValueRsp { value: self.peak_thres } + } + + fn get_bpc(&self, _msg: &MsgSendDirectReq2) -> ValueRsp { + ValueRsp { value: self.peak_level } + } + + fn handle_bmc(&mut self, msg: &MsgSendDirectReq2) -> ValueRsp { + let set_value = msg.payload().u32_at(4); + if set_value != 0 { + self.bmc_data = set_value; + } + ValueRsp { value: self.bmc_data } + } + + fn get_bmd(&self, _msg: &MsgSendDirectReq2) -> ValueRsp { + ValueRsp { value: self.bmd_data } + } + + fn get_bct(&self, _msg: &MsgSendDirectReq2) -> ValueRsp { + ValueRsp { + value: self.charge_time, + } + } + + fn get_btm(&self, _msg: &MsgSendDirectReq2) -> ValueRsp { + ValueRsp { value: self.btm_temp } + } + + fn get_bms(&self, _msg: &MsgSendDirectReq2) -> ValueRsp { + ValueRsp { value: self.bms_data } + } + + fn get_bma(&self, _msg: &MsgSendDirectReq2) -> ValueRsp { + ValueRsp { value: self.bma_data } + } + + fn get_sta(&self, _msg: &MsgSendDirectReq2) -> StaRsp { + StaRsp { + sta_status: self.sta_status, + } } } @@ -294,21 +363,21 @@ impl Service for Battery { debug!("Received Battery command 0x{:x}", cmd); let payload = match cmd { - EC_BAT_GET_BIX => DirectMessagePayload::from(self.generic_test(&msg)), + EC_BAT_GET_BIX => DirectMessagePayload::from(self.get_bix(&msg)), EC_BAT_GET_BST => DirectMessagePayload::from(self.get_bst(&msg)), - EC_BAT_GET_PSR => DirectMessagePayload::from(self.generic_test(&msg)), - EC_BAT_GET_PIF => DirectMessagePayload::from(self.generic_test(&msg)), - EC_BAT_GET_BPS => DirectMessagePayload::from(self.generic_test(&msg)), - EC_BAT_GET_BTP => DirectMessagePayload::from(self.generic_test(&msg)), - EC_BAT_GET_BPT => DirectMessagePayload::from(self.generic_test(&msg)), - EC_BAT_GET_BPC => DirectMessagePayload::from(self.generic_test(&msg)), - EC_BAT_GET_BMC => DirectMessagePayload::from(self.generic_test(&msg)), - EC_BAT_GET_BMD => DirectMessagePayload::from(self.generic_test(&msg)), - EC_BAT_GET_BCT => DirectMessagePayload::from(self.generic_test(&msg)), - EC_BAT_GET_BTM => DirectMessagePayload::from(self.generic_test(&msg)), - EC_BAT_GET_BMS => DirectMessagePayload::from(self.generic_test(&msg)), - EC_BAT_GET_BMA => DirectMessagePayload::from(self.generic_test(&msg)), - EC_BAT_GET_STA => DirectMessagePayload::from(self.generic_test(&msg)), + EC_BAT_GET_PSR => DirectMessagePayload::from(self.get_psr(&msg)), + EC_BAT_GET_PIF => DirectMessagePayload::from(self.get_pif(&msg)), + EC_BAT_GET_BPS => DirectMessagePayload::from(self.get_bps(&msg)), + EC_BAT_GET_BTP => DirectMessagePayload::from(self.handle_btp(&msg)), + EC_BAT_GET_BPT => DirectMessagePayload::from(self.get_bpt(&msg)), + EC_BAT_GET_BPC => DirectMessagePayload::from(self.get_bpc(&msg)), + EC_BAT_GET_BMC => DirectMessagePayload::from(self.handle_bmc(&msg)), + EC_BAT_GET_BMD => DirectMessagePayload::from(self.get_bmd(&msg)), + EC_BAT_GET_BCT => DirectMessagePayload::from(self.get_bct(&msg)), + EC_BAT_GET_BTM => DirectMessagePayload::from(self.get_btm(&msg)), + EC_BAT_GET_BMS => DirectMessagePayload::from(self.get_bms(&msg)), + EC_BAT_GET_BMA => DirectMessagePayload::from(self.get_bma(&msg)), + EC_BAT_GET_STA => DirectMessagePayload::from(self.get_sta(&msg)), _ => { error!("Unknown Battery Command: {}", cmd); return Err(odp_ffa::Error::Other("Unknown Battery Command")); From d98e68277e995f4b85e355f6c450eeb0f13178c9 Mon Sep 17 00:00:00 2001 From: Dylan Knutson Date: Tue, 14 Apr 2026 19:04:36 -0700 Subject: [PATCH 4/8] test(02-02): add 15 core battery opcode unit tests - Add bat_req/bat_req_with_value test helper functions - Add test_get_bix: verify all 11 BIX response fields from defaults - Add test_get_bst_from_state: verify BST reads struct state - Add tests for PSR, PIF, BPS, BTP default, BPT, BPC, BMC default - Add tests for BMD, BCT, BTM, BMS, BMA, STA - 16 tests total (1 existing + 15 new), all passing --- platform/qemu-sp/src/battery.rs | 146 ++++++++++++++++++++++++++++++++ 1 file changed, 146 insertions(+) diff --git a/platform/qemu-sp/src/battery.rs b/platform/qemu-sp/src/battery.rs index fbc0a10..742571e 100644 --- a/platform/qemu-sp/src/battery.rs +++ b/platform/qemu-sp/src/battery.rs @@ -393,6 +393,19 @@ mod tests { use super::*; use odp_ffa::HasRegisterPayload; + /// Build a battery request with just the opcode command byte. + fn bat_req(cmd: u8) -> MsgSendDirectReq2 { + MsgSendDirectReq2::new(0, 0, Battery::UUID, DirectMessagePayload::from_iter(vec![cmd])) + } + + /// Build a battery request with opcode + u32 value at offset 4 (for BTP/BMC set operations). + fn bat_req_with_value(cmd: u8, value: u32) -> MsgSendDirectReq2 { + let mut bytes = [0u8; 14 * 8]; + bytes[0] = cmd; + bytes[4..8].copy_from_slice(&value.to_le_bytes()); + MsgSendDirectReq2::new(0, 0, Battery::UUID, DirectMessagePayload::from_iter(bytes)) + } + #[test] fn battery_get_bst_works() { let mut bat = Battery::new(); @@ -410,4 +423,137 @@ mod tests { assert_eq!(bst.remaining_cap, 5000); assert_eq!(bst.present_volt, 12000); } + + #[test] + fn test_get_bix() { + let mut bat = Battery::new(); + let resp = bat.ffa_msg_send_direct_req2(bat_req(EC_BAT_GET_BIX)).unwrap(); + let bix = BixRsp::from(resp.payload()); + assert_eq!(bix.events, 0); + assert_eq!(bix.status, 0); + assert_eq!(bix.last_full_charge, 4500); + assert_eq!(bix.cycle_count, 42); + assert_eq!(bix.state, 0x1); + assert_eq!(bix.present_rate, 500); + assert_eq!(bix.remain_cap, 5000); + assert_eq!(bix.present_volt, 12000); + assert_eq!(bix.psr_state, 0x1); + assert_eq!(bix.psr_max_out, 65000); + assert_eq!(bix.psr_max_in, 0); + } + + #[test] + fn test_get_bst_from_state() { + let mut bat = Battery::new(); + let resp = bat.ffa_msg_send_direct_req2(bat_req(EC_BAT_GET_BST)).unwrap(); + let bst = BstRsp::from(resp.payload()); + assert_eq!(bst.state, 0x1); + assert_eq!(bst.present_rate, 500); + assert_eq!(bst.remaining_cap, 5000); + assert_eq!(bst.present_volt, 12000); + } + + #[test] + fn test_get_psr() { + let mut bat = Battery::new(); + let resp = bat.ffa_msg_send_direct_req2(bat_req(EC_BAT_GET_PSR)).unwrap(); + let psr = PsrRsp::from(resp.payload()); + assert_eq!(psr.psr_state, 0x1); + } + + #[test] + fn test_get_pif() { + let mut bat = Battery::new(); + let resp = bat.ffa_msg_send_direct_req2(bat_req(EC_BAT_GET_PIF)).unwrap(); + let pif = PifRsp::from(resp.payload()); + assert_eq!(pif.max_power, 65000); + } + + #[test] + fn test_get_bps() { + let mut bat = Battery::new(); + let resp = bat.ffa_msg_send_direct_req2(bat_req(EC_BAT_GET_BPS)).unwrap(); + let val = ValueRsp::from(resp.payload()); + assert_eq!(val.value, 0x1); + } + + #[test] + fn test_get_btp_default() { + let mut bat = Battery::new(); + let resp = bat.ffa_msg_send_direct_req2(bat_req(EC_BAT_GET_BTP)).unwrap(); + let val = ValueRsp::from(resp.payload()); + assert_eq!(val.value, 10); + } + + #[test] + fn test_get_bpt() { + let mut bat = Battery::new(); + let resp = bat.ffa_msg_send_direct_req2(bat_req(EC_BAT_GET_BPT)).unwrap(); + let val = ValueRsp::from(resp.payload()); + assert_eq!(val.value, 80); + } + + #[test] + fn test_get_bpc() { + let mut bat = Battery::new(); + let resp = bat.ffa_msg_send_direct_req2(bat_req(EC_BAT_GET_BPC)).unwrap(); + let val = ValueRsp::from(resp.payload()); + assert_eq!(val.value, 100); + } + + #[test] + fn test_get_bmc_default() { + let mut bat = Battery::new(); + let resp = bat.ffa_msg_send_direct_req2(bat_req(EC_BAT_GET_BMC)).unwrap(); + let val = ValueRsp::from(resp.payload()); + assert_eq!(val.value, 0); + } + + #[test] + fn test_get_bmd() { + let mut bat = Battery::new(); + let resp = bat.ffa_msg_send_direct_req2(bat_req(EC_BAT_GET_BMD)).unwrap(); + let val = ValueRsp::from(resp.payload()); + assert_eq!(val.value, 0); + } + + #[test] + fn test_get_bct() { + let mut bat = Battery::new(); + let resp = bat.ffa_msg_send_direct_req2(bat_req(EC_BAT_GET_BCT)).unwrap(); + let val = ValueRsp::from(resp.payload()); + assert_eq!(val.value, 120); + } + + #[test] + fn test_get_btm() { + let mut bat = Battery::new(); + let resp = bat.ffa_msg_send_direct_req2(bat_req(EC_BAT_GET_BTM)).unwrap(); + let val = ValueRsp::from(resp.payload()); + assert_eq!(val.value, 2980); + } + + #[test] + fn test_get_bms() { + let mut bat = Battery::new(); + let resp = bat.ffa_msg_send_direct_req2(bat_req(EC_BAT_GET_BMS)).unwrap(); + let val = ValueRsp::from(resp.payload()); + assert_eq!(val.value, 0x1); + } + + #[test] + fn test_get_bma() { + let mut bat = Battery::new(); + let resp = bat.ffa_msg_send_direct_req2(bat_req(EC_BAT_GET_BMA)).unwrap(); + let val = ValueRsp::from(resp.payload()); + assert_eq!(val.value, 0); + } + + #[test] + fn test_get_sta() { + let mut bat = Battery::new(); + let resp = bat.ffa_msg_send_direct_req2(bat_req(EC_BAT_GET_STA)).unwrap(); + let sta = StaRsp::from(resp.payload()); + assert_eq!(sta.sta_status, 0x1F); + } } From 81e8976edbd276b3ae056b353db691e99619f5cf Mon Sep 17 00:00:00 2001 From: Dylan Knutson Date: Tue, 14 Apr 2026 19:05:33 -0700 Subject: [PATCH 5/8] test(02-02): add BTP/BMC round-trip edge cases and unknown command test - Add test_btp_set_get_round_trip: set value then get verifies persistence - Add test_bmc_set_get_round_trip: set 0x42 then get verifies persistence - Add test_btp_set_overwrites_previous: last set wins (50 then 75) - Add test_unknown_command_returns_error: 0xFF returns Err - 20 total battery tests passing, clippy and fmt clean --- platform/qemu-sp/src/battery.rs | 54 +++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/platform/qemu-sp/src/battery.rs b/platform/qemu-sp/src/battery.rs index 742571e..c2eb7f9 100644 --- a/platform/qemu-sp/src/battery.rs +++ b/platform/qemu-sp/src/battery.rs @@ -556,4 +556,58 @@ mod tests { let sta = StaRsp::from(resp.payload()); assert_eq!(sta.sta_status, 0x1F); } + + #[test] + fn test_btp_set_get_round_trip() { + let mut bat = Battery::new(); + // Set trip_thres to 25 via payload value at offset 4 + let resp = bat + .ffa_msg_send_direct_req2(bat_req_with_value(EC_BAT_GET_BTP, 25)) + .unwrap(); + let val = ValueRsp::from(resp.payload()); + assert_eq!(val.value, 25); + // Get without set value — should return the persisted value + let resp = bat.ffa_msg_send_direct_req2(bat_req(EC_BAT_GET_BTP)).unwrap(); + let val = ValueRsp::from(resp.payload()); + assert_eq!(val.value, 25); + } + + #[test] + fn test_bmc_set_get_round_trip() { + let mut bat = Battery::new(); + // Set bmc_data to 0x42 + let resp = bat + .ffa_msg_send_direct_req2(bat_req_with_value(EC_BAT_GET_BMC, 0x42)) + .unwrap(); + let val = ValueRsp::from(resp.payload()); + assert_eq!(val.value, 0x42); + // Get without set — verify persistence + let resp = bat.ffa_msg_send_direct_req2(bat_req(EC_BAT_GET_BMC)).unwrap(); + let val = ValueRsp::from(resp.payload()); + assert_eq!(val.value, 0x42); + } + + #[test] + fn test_btp_set_overwrites_previous() { + let mut bat = Battery::new(); + // Set to 50 + let _ = bat + .ffa_msg_send_direct_req2(bat_req_with_value(EC_BAT_GET_BTP, 50)) + .unwrap(); + // Overwrite to 75 + let _ = bat + .ffa_msg_send_direct_req2(bat_req_with_value(EC_BAT_GET_BTP, 75)) + .unwrap(); + // Verify latest value + let resp = bat.ffa_msg_send_direct_req2(bat_req(EC_BAT_GET_BTP)).unwrap(); + let val = ValueRsp::from(resp.payload()); + assert_eq!(val.value, 75); + } + + #[test] + fn test_unknown_command_returns_error() { + let mut bat = Battery::new(); + let result = bat.ffa_msg_send_direct_req2(bat_req(0xFF)); + assert!(result.is_err()); + } } From e32619e0ceb0c5a6c34f225fa25a880729a1b9e9 Mon Sep 17 00:00:00 2001 From: Dylan Knutson Date: Tue, 14 Apr 2026 19:17:59 -0700 Subject: [PATCH 6/8] fix(02): use set_flag byte for BTP/BMC to allow zero-value writes Addresses code review WR-01: BTP and BMC handlers used != 0 guard on the set value, making it impossible to reset trip_thres or bmc_data to zero. Now uses a separate flag byte at offset 8 to indicate a set operation, allowing zero as a valid value (e.g., ACPI _BTP=0 disables trip point). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- platform/qemu-sp/src/battery.rs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/platform/qemu-sp/src/battery.rs b/platform/qemu-sp/src/battery.rs index c2eb7f9..638826b 100644 --- a/platform/qemu-sp/src/battery.rs +++ b/platform/qemu-sp/src/battery.rs @@ -302,9 +302,11 @@ impl Battery { } fn handle_btp(&mut self, msg: &MsgSendDirectReq2) -> ValueRsp { - let set_value = msg.payload().u32_at(4); - if set_value != 0 { - self.trip_thres = set_value; + // Byte 8 is a set flag: non-zero means "store the value at offset 4" + // This allows setting trip_thres to 0 (disable trip point per ACPI) + let set_flag = msg.payload().u8_at(8); + if set_flag != 0 { + self.trip_thres = msg.payload().u32_at(4); } ValueRsp { value: self.trip_thres } } @@ -318,9 +320,9 @@ impl Battery { } fn handle_bmc(&mut self, msg: &MsgSendDirectReq2) -> ValueRsp { - let set_value = msg.payload().u32_at(4); - if set_value != 0 { - self.bmc_data = set_value; + let set_flag = msg.payload().u8_at(8); + if set_flag != 0 { + self.bmc_data = msg.payload().u32_at(4); } ValueRsp { value: self.bmc_data } } @@ -399,10 +401,12 @@ mod tests { } /// Build a battery request with opcode + u32 value at offset 4 (for BTP/BMC set operations). + /// Sets a flag byte at offset 8 to indicate "store this value". fn bat_req_with_value(cmd: u8, value: u32) -> MsgSendDirectReq2 { let mut bytes = [0u8; 14 * 8]; bytes[0] = cmd; bytes[4..8].copy_from_slice(&value.to_le_bytes()); + bytes[8] = 1; // set flag MsgSendDirectReq2::new(0, 0, Battery::UUID, DirectMessagePayload::from_iter(bytes)) } From 4d32ecaebb413f30bb04e2b09b831ffc47e8bf6f Mon Sep 17 00:00:00 2001 From: Dylan Knutson Date: Wed, 15 Apr 2026 12:29:38 -0700 Subject: [PATCH 7/8] =?UTF-8?q?fix:=20standardize=20remain=5Fcap=20?= =?UTF-8?q?=E2=86=92=20remaining=5Fcap=20naming?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- platform/qemu-sp/src/battery.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/platform/qemu-sp/src/battery.rs b/platform/qemu-sp/src/battery.rs index 638826b..cca6615 100644 --- a/platform/qemu-sp/src/battery.rs +++ b/platform/qemu-sp/src/battery.rs @@ -53,7 +53,7 @@ struct BixRsp { cycle_count: u32, state: u32, present_rate: u32, - remain_cap: u32, + remaining_cap: u32, present_volt: u32, psr_state: u32, psr_max_out: u32, @@ -69,7 +69,7 @@ impl From for DirectMessagePayload { value.cycle_count, value.state, value.present_rate, - value.remain_cap, + value.remaining_cap, value.present_volt, value.psr_state, value.psr_max_out, @@ -88,7 +88,7 @@ impl From<&DirectMessagePayload> for BixRsp { cycle_count: payload.u32_at(12), state: payload.u32_at(16), present_rate: payload.u32_at(20), - remain_cap: payload.u32_at(24), + remaining_cap: payload.u32_at(24), present_volt: payload.u32_at(28), psr_state: payload.u32_at(32), psr_max_out: payload.u32_at(36), @@ -179,7 +179,7 @@ pub struct Battery { // BST fields (used by get_bst) state: u32, present_rate: u32, - remain_cap: u32, + remaining_cap: u32, present_volt: u32, // PSR fields psr_state: u32, @@ -232,7 +232,7 @@ impl Battery { cycle_count: 42, state: 0x1, // discharging present_rate: 500, // mW draw - remain_cap: 5000, // mWh remaining + remaining_cap: 5000, // mWh remaining present_volt: 12000, // 12V in mV psr_state: 0x1, // AC adapter present psr_max_out: 65000, // 65W max output (mW) @@ -268,7 +268,7 @@ impl Battery { cycle_count: self.cycle_count, state: self.state, present_rate: self.present_rate, - remain_cap: self.remain_cap, + remaining_cap: self.remaining_cap, present_volt: self.present_volt, psr_state: self.psr_state, psr_max_out: self.psr_max_out, @@ -280,7 +280,7 @@ impl Battery { BstRsp { state: self.state, present_rate: self.present_rate, - remaining_cap: self.remain_cap, + remaining_cap: self.remaining_cap, present_volt: self.present_volt, } } @@ -439,7 +439,7 @@ mod tests { assert_eq!(bix.cycle_count, 42); assert_eq!(bix.state, 0x1); assert_eq!(bix.present_rate, 500); - assert_eq!(bix.remain_cap, 5000); + assert_eq!(bix.remaining_cap, 5000); assert_eq!(bix.present_volt, 12000); assert_eq!(bix.psr_state, 0x1); assert_eq!(bix.psr_max_out, 65000); From 8dd18e4fc712b43b8bf5f185bd0e4057f944d13c Mon Sep 17 00:00:00 2001 From: Dylan Knutson Date: Wed, 15 Apr 2026 12:29:53 -0700 Subject: [PATCH 8/8] test: add BTP and BMC zero-value write round-trip tests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- platform/qemu-sp/src/battery.rs | 38 +++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/platform/qemu-sp/src/battery.rs b/platform/qemu-sp/src/battery.rs index cca6615..e37b70c 100644 --- a/platform/qemu-sp/src/battery.rs +++ b/platform/qemu-sp/src/battery.rs @@ -608,6 +608,44 @@ mod tests { assert_eq!(val.value, 75); } + #[test] + fn test_btp_zero_value_write() { + let mut bat = Battery::new(); + // Set BTP to non-zero first + let _ = bat + .ffa_msg_send_direct_req2(bat_req_with_value(EC_BAT_GET_BTP, 50)) + .unwrap(); + // Set BTP to 0 with set_flag byte set — should persist zero + let resp = bat + .ffa_msg_send_direct_req2(bat_req_with_value(EC_BAT_GET_BTP, 0)) + .unwrap(); + let val = ValueRsp::from(resp.payload()); + assert_eq!(val.value, 0); + // Read back to verify persistence + let resp = bat.ffa_msg_send_direct_req2(bat_req(EC_BAT_GET_BTP)).unwrap(); + let val = ValueRsp::from(resp.payload()); + assert_eq!(val.value, 0); + } + + #[test] + fn test_bmc_zero_value_write() { + let mut bat = Battery::new(); + // Set BMC to non-zero first + let _ = bat + .ffa_msg_send_direct_req2(bat_req_with_value(EC_BAT_GET_BMC, 0x42)) + .unwrap(); + // Set BMC to 0 with set_flag byte set — should persist zero + let resp = bat + .ffa_msg_send_direct_req2(bat_req_with_value(EC_BAT_GET_BMC, 0)) + .unwrap(); + let val = ValueRsp::from(resp.payload()); + assert_eq!(val.value, 0); + // Read back to verify persistence + let resp = bat.ffa_msg_send_direct_req2(bat_req(EC_BAT_GET_BMC)).unwrap(); + let val = ValueRsp::from(resp.payload()); + assert_eq!(val.value, 0); + } + #[test] fn test_unknown_command_returns_error() { let mut bat = Battery::new();