From 37845ac1ad83475410c34a9a9eb7860949c494b0 Mon Sep 17 00:00:00 2001 From: Robert Zieba Date: Wed, 24 Jun 2026 11:03:04 -0700 Subject: [PATCH 1/2] Extract power policy mocks into `power-policy-interface-test-mocks` crate Move PSU and charger mock code from power-policy-service/tests/common/mock.rs into a standalone power-policy-interface-test-mocks crate with separate psu and charger modules. Update workspace configuration, CI workflow exclusions, and test imports accordingly. Assisted-by: GitHub Copilot:claude-opus-4.6 --- .github/workflows/check.yml | 2 +- Cargo.lock | 12 + Cargo.toml | 8 +- power-policy-interface-test-mocks/Cargo.toml | 17 + .../src/charger.rs | 83 ++++ power-policy-interface-test-mocks/src/lib.rs | 5 + .../src/psu.rs | 353 +++++++----------- power-policy-service/Cargo.toml | 1 + power-policy-service/tests/common/mod.rs | 8 +- power-policy-service/tests/consumer.rs | 3 +- power-policy-service/tests/provider.rs | 3 +- power-policy-service/tests/unconstrained.rs | 3 +- 12 files changed, 269 insertions(+), 229 deletions(-) create mode 100644 power-policy-interface-test-mocks/Cargo.toml create mode 100644 power-policy-interface-test-mocks/src/charger.rs create mode 100644 power-policy-interface-test-mocks/src/lib.rs rename power-policy-service/tests/common/mock.rs => power-policy-interface-test-mocks/src/psu.rs (61%) diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml index a7c2533b0..dabac2e1b 100644 --- a/.github/workflows/check.yml +++ b/.github/workflows/check.yml @@ -33,7 +33,7 @@ concurrency: name: check env: # Crates that require std and won't build on embedded-targets - STD_EXCLUDED_CRATES: "--exclude fw-update-interface-mocks --exclude type-c-interface-test-mocks" + STD_EXCLUDED_CRATES: "--exclude fw-update-interface-mocks --exclude type-c-interface-test-mocks --exclude power-policy-interface-test-mocks" jobs: fmt: diff --git a/Cargo.lock b/Cargo.lock index 20cee358d..73b041656 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1539,6 +1539,17 @@ dependencies = [ "num_enum", ] +[[package]] +name = "power-policy-interface-test-mocks" +version = "0.1.0" +dependencies = [ + "embassy-sync", + "embedded-batteries-async", + "embedded-services", + "log", + "power-policy-interface", +] + [[package]] name = "power-policy-service" version = "0.1.0" @@ -1554,6 +1565,7 @@ dependencies = [ "heapless 0.9.3", "log", "power-policy-interface", + "power-policy-interface-test-mocks", "tokio", ] diff --git a/Cargo.toml b/Cargo.toml index 000bf05fb..f913cc4ab 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -32,6 +32,7 @@ members = [ "fw-update-interface-mocks", "mctp-rs", "type-c-interface-test-mocks", + "power-policy-interface-test-mocks", ] exclude = ["examples/*"] @@ -82,12 +83,12 @@ embedded-hal-nb = "1.0" embedded-io-async = "0.7.0" embedded-mcu-hal = "0.3.0" embassy-futures = "0.1.2" -embassy-imxrt = { git = "https://github.com/OpenDevicePartnership/embassy-imxrt"} +embassy-imxrt = { git = "https://github.com/OpenDevicePartnership/embassy-imxrt" } embassy-sync = "0.8" embassy-time = "0.5.1" embassy-time-driver = "0.2.2" embedded-batteries-async = "0.3" -embedded-cfu-protocol = { git = "https://github.com/OpenDevicePartnership/embedded-cfu"} +embedded-cfu-protocol = { git = "https://github.com/OpenDevicePartnership/embedded-cfu" } embedded-hal = "1.0" embedded-hal-async = "1.0" embedded-services = { path = "./embedded-service" } @@ -99,6 +100,7 @@ mctp-rs = { path = "./mctp-rs" } num_enum = { version = "0.7.5", default-features = false } portable-atomic = { version = "1.11", default-features = false } power-policy-interface = { path = "./power-policy-interface" } +power-policy-interface-test-mocks = { path = "./power-policy-interface-test-mocks" } paste = "1.0.15" power-policy-service = { path = "./power-policy-service" } fixed = "1.23.1" @@ -117,7 +119,7 @@ time-alarm-service-relay = { path = "./time-alarm-service-relay" } type-c-interface = { path = "./type-c-interface" } type-c-interface-test-mocks = { path = "./type-c-interface-test-mocks" } syn = "2.0" -tps6699x = { git = "https://github.com/OpenDevicePartnership/tps6699x"} +tps6699x = { git = "https://github.com/OpenDevicePartnership/tps6699x" } tokio = { version = "1.42.0" } uuid = { version = "=1.17.0", default-features = false } zerocopy = "0.8" diff --git a/power-policy-interface-test-mocks/Cargo.toml b/power-policy-interface-test-mocks/Cargo.toml new file mode 100644 index 000000000..4efb68075 --- /dev/null +++ b/power-policy-interface-test-mocks/Cargo.toml @@ -0,0 +1,17 @@ +[package] +name = "power-policy-interface-test-mocks" +version.workspace = true +edition.workspace = true +rust-version.workspace = true +license.workspace = true +repository.workspace = true + +[dependencies] +embassy-sync.workspace = true +embedded-batteries-async.workspace = true +embedded-services = { workspace = true } +log = { workspace = true } +power-policy-interface = { workspace = true } + +[lints] +workspace = true diff --git a/power-policy-interface-test-mocks/src/charger.rs b/power-policy-interface-test-mocks/src/charger.rs new file mode 100644 index 000000000..740d712de --- /dev/null +++ b/power-policy-interface-test-mocks/src/charger.rs @@ -0,0 +1,83 @@ +//! Charger mock implementation for testing + +use embassy_sync::{channel, mutex::Mutex}; +use embedded_batteries_async::charger::{MilliAmps, MilliVolts}; +use embedded_services::GlobalRawMutex; +use log::info; +use power_policy_interface::{capability::ConsumerPowerCapability, charger}; + +pub struct ExampleCharger<'a> { + sender: channel::DynamicSender<'a, charger::event::EventData>, + state: charger::State, +} + +impl<'a> ExampleCharger<'a> { + pub fn new(sender: channel::DynamicSender<'a, charger::event::EventData>) -> Self { + Self { + sender, + state: charger::State::default(), + } + } + + pub fn assert_state(&self, internal_state: charger::InternalState, capability: Option) { + assert_eq!(*self.state.internal_state(), internal_state); + assert_eq!(*self.state.capability(), capability); + } + + pub async fn simulate_psu_state_change(&self, psu_state: charger::PsuState) { + self.sender + .try_send(charger::EventData::PsuStateChange(psu_state)) + .unwrap(); + } +} + +impl<'a> embedded_batteries_async::charger::ErrorType for ExampleCharger<'a> { + type Error = core::convert::Infallible; +} + +impl<'a> embedded_batteries_async::charger::Charger for ExampleCharger<'a> { + async fn charging_current(&mut self, current: MilliAmps) -> Result { + Ok(current) + } + + async fn charging_voltage(&mut self, voltage: MilliVolts) -> Result { + Ok(voltage) + } +} + +impl<'a> charger::Charger for ExampleCharger<'a> { + type ChargerError = core::convert::Infallible; + + async fn init_charger(&mut self) -> Result { + info!("Charger init"); + Ok(charger::PsuState::Detached) + } + + fn attach_handler( + &mut self, + capability: ConsumerPowerCapability, + ) -> impl Future> { + info!("Charger attach: {:?}", capability); + async { Ok(()) } + } + + fn detach_handler(&mut self) -> impl Future> { + info!("Charger detach"); + async { Ok(()) } + } + + async fn is_ready(&mut self) -> Result<(), Self::ChargerError> { + info!("Charger check ready"); + Ok(()) + } + + fn state(&self) -> &charger::State { + &self.state + } + + fn state_mut(&mut self) -> &mut charger::State { + &mut self.state + } +} + +pub type ChargerType<'a> = Mutex>; diff --git a/power-policy-interface-test-mocks/src/lib.rs b/power-policy-interface-test-mocks/src/lib.rs new file mode 100644 index 000000000..bc8f565bb --- /dev/null +++ b/power-policy-interface-test-mocks/src/lib.rs @@ -0,0 +1,5 @@ +#![allow(clippy::unwrap_used)] +#![allow(clippy::expect_used)] + +pub mod charger; +pub mod psu; diff --git a/power-policy-service/tests/common/mock.rs b/power-policy-interface-test-mocks/src/psu.rs similarity index 61% rename from power-policy-service/tests/common/mock.rs rename to power-policy-interface-test-mocks/src/psu.rs index 820762604..9b5251a18 100644 --- a/power-policy-service/tests/common/mock.rs +++ b/power-policy-interface-test-mocks/src/psu.rs @@ -1,216 +1,137 @@ -#![allow(clippy::unwrap_used)] -#![allow(dead_code)] -use embassy_sync::{channel, mutex::Mutex, signal::Signal}; -use embedded_batteries_async::charger::{MilliAmps, MilliVolts}; -use embedded_services::{GlobalRawMutex, event::NonBlockingSender, info, named::Named}; -use power_policy_interface::{ - capability::{ - ConsumerDisconnect, ConsumerPowerCapability, PowerCapability, ProviderFlags, ProviderPowerCapability, - }, - charger, - psu::{Error, Psu, State, event::EventData}, -}; - -#[derive(Debug, Clone, PartialEq, Eq)] -#[allow(dead_code)] -pub enum FnCall { - ConnectConsumer(ConsumerPowerCapability), - ConnectProvider(ProviderPowerCapability), - Disconnect, - Reset, -} - -pub struct Mock<'a, S: NonBlockingSender> { - sender: S, - fn_call: &'a Signal, - // Internal state - pub state: State, - name: &'static str, -} - -impl<'a, S: NonBlockingSender> Mock<'a, S> { - pub fn new(name: &'static str, sender: S, fn_call: &'a Signal) -> Self { - Self { - name, - sender, - fn_call, - state: Default::default(), - } - } - - fn record_fn_call(&mut self, fn_call: FnCall) { - let num_fn_calls = self - .fn_call - .try_take() - .map(|(num_fn_calls, _)| num_fn_calls) - .unwrap_or(0); - self.fn_call.signal((num_fn_calls + 1, fn_call)); - } - - pub async fn simulate_consumer_connection(&mut self, capability: ConsumerPowerCapability) { - self.state.attach().unwrap(); - self.sender.try_send(EventData::Attached).unwrap(); - self.state.update_consumer_power_capability(Some(capability)).unwrap(); - self.sender - .try_send(EventData::UpdatedConsumerCapability(Some(capability))) - .unwrap(); - } - - /// Simulate an already-attached consumer renegotiating a new power capability. - pub async fn simulate_update_consumer_power_capability(&mut self, capability: Option) { - self.state.update_consumer_power_capability(capability).unwrap(); - self.sender - .try_send(EventData::UpdatedConsumerCapability(capability)) - .unwrap(); - } - - pub async fn simulate_detach(&mut self) { - self.state.detach(); - self.sender.try_send(EventData::Detached).unwrap(); - } - - pub async fn simulate_provider_connection(&mut self, capability: PowerCapability) { - self.state.attach().unwrap(); - self.sender.try_send(EventData::Attached).unwrap(); - - let capability = Some(ProviderPowerCapability { - capability, - flags: ProviderFlags::none(), - }); - self.state - .update_requested_provider_power_capability(capability) - .unwrap(); - self.sender - .try_send(EventData::RequestedProviderCapability(capability)) - .unwrap(); - } - - pub async fn simulate_disconnect(&mut self) { - self.state.disconnect(true).unwrap(); - self.sender - .try_send(EventData::Disconnected(ConsumerDisconnect::none())) - .unwrap(); - } - - pub async fn simulate_update_requested_provider_power_capability( - &mut self, - capability: Option, - ) { - self.state - .update_requested_provider_power_capability(capability) - .unwrap(); - self.sender - .try_send(power_policy_interface::psu::event::EventData::RequestedProviderCapability(capability)) - .unwrap(); - } -} - -impl<'a, S: NonBlockingSender> Psu for Mock<'a, S> { - async fn connect_consumer(&mut self, capability: ConsumerPowerCapability) -> Result<(), Error> { - info!("Connect consumer {:#?}", capability); - self.record_fn_call(FnCall::ConnectConsumer(capability)); - self.state.connect_consumer(capability) - } - - async fn connect_provider(&mut self, capability: ProviderPowerCapability) -> Result<(), Error> { - info!("Connect provider: {:#?}", capability); - self.record_fn_call(FnCall::ConnectProvider(capability)); - self.state.connect_provider(capability) - } - - async fn disconnect(&mut self) -> Result<(), Error> { - info!("Disconnect"); - self.record_fn_call(FnCall::Disconnect); - self.state.disconnect(false) - } - - fn state(&self) -> &State { - &self.state - } - - fn state_mut(&mut self) -> &mut State { - &mut self.state - } -} - -impl<'a, S: NonBlockingSender> Named for Mock<'a, S> { - fn name(&self) -> &'static str { - self.name - } -} - -pub struct ExampleCharger<'a> { - sender: channel::DynamicSender<'a, power_policy_interface::charger::event::EventData>, - state: charger::State, -} - -impl<'a> ExampleCharger<'a> { - pub fn new(sender: channel::DynamicSender<'a, power_policy_interface::charger::event::EventData>) -> Self { - Self { - sender, - state: charger::State::default(), - } - } - - pub fn assert_state(&self, internal_state: charger::InternalState, capability: Option) { - assert_eq!(*self.state.internal_state(), internal_state); - assert_eq!(*self.state.capability(), capability); - } - - pub async fn simulate_psu_state_change(&self, psu_state: charger::PsuState) { - self.sender - .try_send(charger::EventData::PsuStateChange(psu_state)) - .unwrap(); - } -} - -impl<'a> embedded_batteries_async::charger::ErrorType for ExampleCharger<'a> { - type Error = core::convert::Infallible; -} - -impl<'a> embedded_batteries_async::charger::Charger for ExampleCharger<'a> { - async fn charging_current(&mut self, current: MilliAmps) -> Result { - Ok(current) - } - - async fn charging_voltage(&mut self, voltage: MilliVolts) -> Result { - Ok(voltage) - } -} - -impl<'a> charger::Charger for ExampleCharger<'a> { - type ChargerError = core::convert::Infallible; - - async fn init_charger(&mut self) -> Result { - info!("Charger init"); - Ok(charger::PsuState::Detached) - } - - fn attach_handler( - &mut self, - capability: ConsumerPowerCapability, - ) -> impl Future> { - info!("Charger attach: {:?}", capability); - async { Ok(()) } - } - - fn detach_handler(&mut self) -> impl Future> { - info!("Charger detach"); - async { Ok(()) } - } - - async fn is_ready(&mut self) -> Result<(), Self::ChargerError> { - info!("Charger check ready"); - Ok(()) - } - - fn state(&self) -> &charger::State { - &self.state - } - - fn state_mut(&mut self) -> &mut charger::State { - &mut self.state - } -} - -pub type ChargerType<'a> = Mutex>; +//! PSU mock implementation for testing + +use embassy_sync::signal::Signal; +use embedded_services::{GlobalRawMutex, event::NonBlockingSender, named::Named}; +use log::info; +use power_policy_interface::{ + capability::{ + ConsumerDisconnect, ConsumerPowerCapability, PowerCapability, ProviderFlags, ProviderPowerCapability, + }, + psu::{Error, Psu, State, event::EventData}, +}; + +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum FnCall { + ConnectConsumer(ConsumerPowerCapability), + ConnectProvider(ProviderPowerCapability), + Disconnect, + Reset, +} + +pub struct Mock<'a, S: NonBlockingSender> { + sender: S, + fn_call: &'a Signal, + pub state: State, + name: &'static str, +} + +impl<'a, S: NonBlockingSender> Mock<'a, S> { + pub fn new(name: &'static str, sender: S, fn_call: &'a Signal) -> Self { + Self { + name, + sender, + fn_call, + state: Default::default(), + } + } + + fn record_fn_call(&mut self, fn_call: FnCall) { + let num_fn_calls = self + .fn_call + .try_take() + .map(|(num_fn_calls, _)| num_fn_calls) + .unwrap_or(0); + self.fn_call.signal((num_fn_calls + 1, fn_call)); + } + + pub async fn simulate_consumer_connection(&mut self, capability: ConsumerPowerCapability) { + self.state.attach().unwrap(); + self.sender.try_send(EventData::Attached).unwrap(); + self.state.update_consumer_power_capability(Some(capability)).unwrap(); + self.sender + .try_send(EventData::UpdatedConsumerCapability(Some(capability))) + .unwrap(); + } + + /// Simulate an already-attached consumer renegotiating a new power capability. + pub async fn simulate_update_consumer_power_capability(&mut self, capability: Option) { + self.state.update_consumer_power_capability(capability).unwrap(); + self.sender + .try_send(EventData::UpdatedConsumerCapability(capability)) + .unwrap(); + } + + pub async fn simulate_detach(&mut self) { + self.state.detach(); + self.sender.try_send(EventData::Detached).unwrap(); + } + + pub async fn simulate_provider_connection(&mut self, capability: PowerCapability) { + self.state.attach().unwrap(); + self.sender.try_send(EventData::Attached).unwrap(); + + let capability = Some(ProviderPowerCapability { + capability, + flags: ProviderFlags::none(), + }); + self.state + .update_requested_provider_power_capability(capability) + .unwrap(); + self.sender + .try_send(EventData::RequestedProviderCapability(capability)) + .unwrap(); + } + + pub async fn simulate_disconnect(&mut self) { + self.state.disconnect(true).unwrap(); + self.sender + .try_send(EventData::Disconnected(ConsumerDisconnect::none())) + .unwrap(); + } + + pub async fn simulate_update_requested_provider_power_capability( + &mut self, + capability: Option, + ) { + self.state + .update_requested_provider_power_capability(capability) + .unwrap(); + self.sender + .try_send(EventData::RequestedProviderCapability(capability)) + .unwrap(); + } +} + +impl<'a, S: NonBlockingSender> Psu for Mock<'a, S> { + async fn connect_consumer(&mut self, capability: ConsumerPowerCapability) -> Result<(), Error> { + info!("Connect consumer {:#?}", capability); + self.record_fn_call(FnCall::ConnectConsumer(capability)); + self.state.connect_consumer(capability) + } + + async fn connect_provider(&mut self, capability: ProviderPowerCapability) -> Result<(), Error> { + info!("Connect provider: {:#?}", capability); + self.record_fn_call(FnCall::ConnectProvider(capability)); + self.state.connect_provider(capability) + } + + async fn disconnect(&mut self) -> Result<(), Error> { + info!("Disconnect"); + self.record_fn_call(FnCall::Disconnect); + self.state.disconnect(false) + } + + fn state(&self) -> &State { + &self.state + } + + fn state_mut(&mut self) -> &mut State { + &mut self.state + } +} + +impl<'a, S: NonBlockingSender> Named for Mock<'a, S> { + fn name(&self) -> &'static str { + self.name + } +} diff --git a/power-policy-service/Cargo.toml b/power-policy-service/Cargo.toml index 1be2dd6a3..b8256e7f4 100644 --- a/power-policy-service/Cargo.toml +++ b/power-policy-service/Cargo.toml @@ -30,6 +30,7 @@ tokio = { workspace = true, features = ["rt", "macros", "time"] } env_logger = "0.11.8" log = { workspace = true } embedded-batteries-async = { workspace = true } +power-policy-interface-test-mocks = { workspace = true } # TODO: figure out why enabling the log feature here causes running tests at the workspace level to fail to compile # Uncomment this line to enable log output in tests # power-policy-service = { workspace = true, features = ["log"] } diff --git a/power-policy-service/tests/common/mod.rs b/power-policy-service/tests/common/mod.rs index 4c9de3a7c..2d37e0862 100644 --- a/power-policy-service/tests/common/mod.rs +++ b/power-policy-service/tests/common/mod.rs @@ -20,15 +20,11 @@ use power_policy_interface::{ capability::{ConsumerDisconnect, ConsumerPowerCapability, PowerCapability, ProviderPowerCapability}, service::{UnconstrainedState, event::Event as ServiceEvent}, }; +use power_policy_interface_test_mocks::charger::ChargerType; +use power_policy_interface_test_mocks::psu::{FnCall, Mock}; use power_policy_service::service::{Service, config::Config, customization}; use power_policy_service::{psu::PsuEventReceivers, service::registration::ArrayRegistration}; -pub mod mock; - -use mock::Mock; - -use crate::common::mock::{ChargerType, FnCall}; - pub const MINIMAL_POWER: PowerCapability = PowerCapability { voltage_mv: 5000, current_ma: 500, diff --git a/power-policy-service/tests/consumer.rs b/power-policy-service/tests/consumer.rs index e4638127c..8614c3c88 100644 --- a/power-policy-service/tests/consumer.rs +++ b/power-policy-service/tests/consumer.rs @@ -31,8 +31,9 @@ use crate::common::assert_provider_connected; use crate::common::assert_provider_disconnected; use crate::common::{ DEFAULT_TIMEOUT, HIGH_POWER, assert_consumer_connected, assert_consumer_disconnected, - assert_consumer_disconnected_with_flags, mock::FnCall, run_test, + assert_consumer_disconnected_with_flags, run_test, }; +use power_policy_interface_test_mocks::psu::FnCall; const PER_CALL_TIMEOUT: Duration = Duration::from_millis(1000); diff --git a/power-policy-service/tests/provider.rs b/power-policy-service/tests/provider.rs index 3f3f47eb9..d205c3242 100644 --- a/power-policy-service/tests/provider.rs +++ b/power-policy-service/tests/provider.rs @@ -17,7 +17,8 @@ use crate::common::DeviceType; use crate::common::HIGH_POWER; use crate::common::Test; use crate::common::assert_no_event; -use crate::common::{DEFAULT_TIMEOUT, assert_provider_connected, assert_provider_disconnected, mock::FnCall, run_test}; +use crate::common::{DEFAULT_TIMEOUT, assert_provider_connected, assert_provider_disconnected, run_test}; +use power_policy_interface_test_mocks::psu::FnCall; const PER_CALL_TIMEOUT: Duration = Duration::from_millis(1000); diff --git a/power-policy-service/tests/unconstrained.rs b/power-policy-service/tests/unconstrained.rs index fef0d69a2..a6ca897f8 100644 --- a/power-policy-service/tests/unconstrained.rs +++ b/power-policy-service/tests/unconstrained.rs @@ -17,8 +17,9 @@ use power_policy_service::service::customization::DefaultCustomization; use crate::common::HIGH_POWER; use crate::common::{ DEFAULT_TIMEOUT, assert_consumer_connected, assert_consumer_disconnected, assert_no_event, assert_unconstrained, - mock::FnCall, run_test, + run_test, }; +use power_policy_interface_test_mocks::psu::FnCall; use crate::common::{DeviceType, ServiceMutex, Test}; const PER_CALL_TIMEOUT: Duration = Duration::from_millis(1000); From 4ba05b7f9b0fa373bacb2411cb054352d2de42ad Mon Sep 17 00:00:00 2001 From: Robert Zieba Date: Wed, 24 Jun 2026 12:32:44 -0700 Subject: [PATCH 2/2] Refactor power policy mocks to record calls and set results Follow the type-c-interface-mocks pattern for the PSU and charger mocks. Each mock now records its trait method invocations in a VecDeque of FnCall values and returns results from per-method `next_result_*` queues that tests populate before each action. This replaces the previous Signal-based synchronization and lets tests assert on the exact sequence of calls made by the service. The charger mock is also made generic over NonBlockingSender to match the PSU mock and the service's event-driven architecture. Assisted-by: GitHub Copilot:claude-opus-4.8 --- Cargo.lock | 1 - power-policy-interface-test-mocks/Cargo.toml | 1 - .../src/charger.rs | 96 ++- power-policy-interface-test-mocks/src/psu.rs | 80 ++- power-policy-service/tests/common/mod.rs | 33 +- power-policy-service/tests/consumer.rs | 575 +++++++++--------- power-policy-service/tests/provider.rs | 195 +++--- power-policy-service/tests/unconstrained.rs | 102 ++-- 8 files changed, 555 insertions(+), 528 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 73b041656..210ebebd3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1546,7 +1546,6 @@ dependencies = [ "embassy-sync", "embedded-batteries-async", "embedded-services", - "log", "power-policy-interface", ] diff --git a/power-policy-interface-test-mocks/Cargo.toml b/power-policy-interface-test-mocks/Cargo.toml index 4efb68075..c53e415ef 100644 --- a/power-policy-interface-test-mocks/Cargo.toml +++ b/power-policy-interface-test-mocks/Cargo.toml @@ -10,7 +10,6 @@ repository.workspace = true embassy-sync.workspace = true embedded-batteries-async.workspace = true embedded-services = { workspace = true } -log = { workspace = true } power-policy-interface = { workspace = true } [lints] diff --git a/power-policy-interface-test-mocks/src/charger.rs b/power-policy-interface-test-mocks/src/charger.rs index 740d712de..c1e053e53 100644 --- a/power-policy-interface-test-mocks/src/charger.rs +++ b/power-policy-interface-test-mocks/src/charger.rs @@ -1,21 +1,55 @@ //! Charger mock implementation for testing -use embassy_sync::{channel, mutex::Mutex}; +use std::collections::VecDeque; + +use embassy_sync::mutex::Mutex; use embedded_batteries_async::charger::{MilliAmps, MilliVolts}; -use embedded_services::GlobalRawMutex; -use log::info; +use embedded_services::{GlobalRawMutex, event::NonBlockingSender}; use power_policy_interface::{capability::ConsumerPowerCapability, charger}; -pub struct ExampleCharger<'a> { - sender: channel::DynamicSender<'a, charger::event::EventData>, +/// Contains a charger function call and its arguments +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum FnCall { + InitCharger, + AttachHandler(ConsumerPowerCapability), + DetachHandler, + IsReady, + ChargingCurrent(MilliAmps), + ChargingVoltage(MilliVolts), +} + +/// Mock charger for use in tests +pub struct Mock> { + sender: S, state: charger::State, + /// Recorded function calls + pub fn_calls: VecDeque, + /// Next results to return for [`charger::Charger::init_charger`] + pub next_result_init_charger: VecDeque>, + /// Next results to return for [`charger::Charger::attach_handler`] + pub next_result_attach_handler: VecDeque>, + /// Next results to return for [`charger::Charger::detach_handler`] + pub next_result_detach_handler: VecDeque>, + /// Next results to return for [`charger::Charger::is_ready`] + pub next_result_is_ready: VecDeque>, + /// Next results to return for [`embedded_batteries_async::charger::Charger::charging_current`] + pub next_result_charging_current: VecDeque>, + /// Next results to return for [`embedded_batteries_async::charger::Charger::charging_voltage`] + pub next_result_charging_voltage: VecDeque>, } -impl<'a> ExampleCharger<'a> { - pub fn new(sender: channel::DynamicSender<'a, charger::event::EventData>) -> Self { +impl> Mock { + pub fn new(sender: S) -> Self { Self { sender, state: charger::State::default(), + fn_calls: VecDeque::new(), + next_result_init_charger: VecDeque::new(), + next_result_attach_handler: VecDeque::new(), + next_result_detach_handler: VecDeque::new(), + next_result_is_ready: VecDeque::new(), + next_result_charging_current: VecDeque::new(), + next_result_charging_voltage: VecDeque::new(), } } @@ -24,51 +58,69 @@ impl<'a> ExampleCharger<'a> { assert_eq!(*self.state.capability(), capability); } - pub async fn simulate_psu_state_change(&self, psu_state: charger::PsuState) { + pub async fn simulate_psu_state_change(&mut self, psu_state: charger::PsuState) { self.sender .try_send(charger::EventData::PsuStateChange(psu_state)) .unwrap(); } } -impl<'a> embedded_batteries_async::charger::ErrorType for ExampleCharger<'a> { +impl> embedded_batteries_async::charger::ErrorType for Mock { type Error = core::convert::Infallible; } -impl<'a> embedded_batteries_async::charger::Charger for ExampleCharger<'a> { +impl> embedded_batteries_async::charger::Charger for Mock { async fn charging_current(&mut self, current: MilliAmps) -> Result { - Ok(current) + self.fn_calls.push_back(FnCall::ChargingCurrent(current)); + self.next_result_charging_current + .pop_front() + .expect("next_result_charging_current not set") } async fn charging_voltage(&mut self, voltage: MilliVolts) -> Result { - Ok(voltage) + self.fn_calls.push_back(FnCall::ChargingVoltage(voltage)); + self.next_result_charging_voltage + .pop_front() + .expect("next_result_charging_voltage not set") } } -impl<'a> charger::Charger for ExampleCharger<'a> { +impl> charger::Charger for Mock { type ChargerError = core::convert::Infallible; async fn init_charger(&mut self) -> Result { - info!("Charger init"); - Ok(charger::PsuState::Detached) + self.fn_calls.push_back(FnCall::InitCharger); + self.next_result_init_charger + .pop_front() + .expect("next_result_init_charger not set") } fn attach_handler( &mut self, capability: ConsumerPowerCapability, ) -> impl Future> { - info!("Charger attach: {:?}", capability); - async { Ok(()) } + self.fn_calls.push_back(FnCall::AttachHandler(capability)); + let result = self + .next_result_attach_handler + .pop_front() + .expect("next_result_attach_handler not set"); + async move { result } } fn detach_handler(&mut self) -> impl Future> { - info!("Charger detach"); - async { Ok(()) } + self.fn_calls.push_back(FnCall::DetachHandler); + let result = self + .next_result_detach_handler + .pop_front() + .expect("next_result_detach_handler not set"); + async move { result } } async fn is_ready(&mut self) -> Result<(), Self::ChargerError> { - info!("Charger check ready"); - Ok(()) + self.fn_calls.push_back(FnCall::IsReady); + self.next_result_is_ready + .pop_front() + .expect("next_result_is_ready not set") } fn state(&self) -> &charger::State { @@ -80,4 +132,4 @@ impl<'a> charger::Charger for ExampleCharger<'a> { } } -pub type ChargerType<'a> = Mutex>; +pub type ChargerType = Mutex>; diff --git a/power-policy-interface-test-mocks/src/psu.rs b/power-policy-interface-test-mocks/src/psu.rs index 9b5251a18..5d6c78212 100644 --- a/power-policy-interface-test-mocks/src/psu.rs +++ b/power-policy-interface-test-mocks/src/psu.rs @@ -1,8 +1,8 @@ //! PSU mock implementation for testing -use embassy_sync::signal::Signal; -use embedded_services::{GlobalRawMutex, event::NonBlockingSender, named::Named}; -use log::info; +use std::collections::VecDeque; + +use embedded_services::{event::NonBlockingSender, named::Named}; use power_policy_interface::{ capability::{ ConsumerDisconnect, ConsumerPowerCapability, PowerCapability, ProviderFlags, ProviderPowerCapability, @@ -10,40 +10,42 @@ use power_policy_interface::{ psu::{Error, Psu, State, event::EventData}, }; +/// Contains a PSU function call and its arguments #[derive(Debug, Clone, PartialEq, Eq)] pub enum FnCall { ConnectConsumer(ConsumerPowerCapability), ConnectProvider(ProviderPowerCapability), Disconnect, - Reset, } -pub struct Mock<'a, S: NonBlockingSender> { +/// Mock PSU for use in tests +pub struct Mock> { sender: S, - fn_call: &'a Signal, - pub state: State, name: &'static str, + pub state: State, + /// Recorded function calls + pub fn_calls: VecDeque, + /// Next results to return for [`Psu::connect_consumer`] + pub next_result_connect_consumer: VecDeque>, + /// Next results to return for [`Psu::connect_provider`] + pub next_result_connect_provider: VecDeque>, + /// Next results to return for [`Psu::disconnect`] + pub next_result_disconnect: VecDeque>, } -impl<'a, S: NonBlockingSender> Mock<'a, S> { - pub fn new(name: &'static str, sender: S, fn_call: &'a Signal) -> Self { +impl> Mock { + pub fn new(name: &'static str, sender: S) -> Self { Self { name, sender, - fn_call, state: Default::default(), + fn_calls: VecDeque::new(), + next_result_connect_consumer: VecDeque::new(), + next_result_connect_provider: VecDeque::new(), + next_result_disconnect: VecDeque::new(), } } - fn record_fn_call(&mut self, fn_call: FnCall) { - let num_fn_calls = self - .fn_call - .try_take() - .map(|(num_fn_calls, _)| num_fn_calls) - .unwrap_or(0); - self.fn_call.signal((num_fn_calls + 1, fn_call)); - } - pub async fn simulate_consumer_connection(&mut self, capability: ConsumerPowerCapability) { self.state.attach().unwrap(); self.sender.try_send(EventData::Attached).unwrap(); @@ -102,23 +104,41 @@ impl<'a, S: NonBlockingSender> Mock<'a, S> { } } -impl<'a, S: NonBlockingSender> Psu for Mock<'a, S> { +impl> Psu for Mock { async fn connect_consumer(&mut self, capability: ConsumerPowerCapability) -> Result<(), Error> { - info!("Connect consumer {:#?}", capability); - self.record_fn_call(FnCall::ConnectConsumer(capability)); - self.state.connect_consumer(capability) + self.fn_calls.push_back(FnCall::ConnectConsumer(capability)); + let result = self + .next_result_connect_consumer + .pop_front() + .expect("next_result_connect_consumer not set"); + if result.is_ok() { + self.state.connect_consumer(capability).unwrap(); + } + result } async fn connect_provider(&mut self, capability: ProviderPowerCapability) -> Result<(), Error> { - info!("Connect provider: {:#?}", capability); - self.record_fn_call(FnCall::ConnectProvider(capability)); - self.state.connect_provider(capability) + self.fn_calls.push_back(FnCall::ConnectProvider(capability)); + let result = self + .next_result_connect_provider + .pop_front() + .expect("next_result_connect_provider not set"); + if result.is_ok() { + self.state.connect_provider(capability).unwrap(); + } + result } async fn disconnect(&mut self) -> Result<(), Error> { - info!("Disconnect"); - self.record_fn_call(FnCall::Disconnect); - self.state.disconnect(false) + self.fn_calls.push_back(FnCall::Disconnect); + let result = self + .next_result_disconnect + .pop_front() + .expect("next_result_disconnect not set"); + if result.is_ok() { + self.state.disconnect(false).unwrap(); + } + result } fn state(&self) -> &State { @@ -130,7 +150,7 @@ impl<'a, S: NonBlockingSender> Psu for Mock<'a, S> { } } -impl<'a, S: NonBlockingSender> Named for Mock<'a, S> { +impl> Named for Mock { fn name(&self) -> &'static str { self.name } diff --git a/power-policy-service/tests/common/mod.rs b/power-policy-service/tests/common/mod.rs index 2d37e0862..be201ea5e 100644 --- a/power-policy-service/tests/common/mod.rs +++ b/power-policy-service/tests/common/mod.rs @@ -11,7 +11,6 @@ use embassy_sync::{ channel::{Channel, DynamicReceiver, DynamicSender}, mutex::Mutex, once_lock::OnceLock, - signal::Signal, }; use embassy_time::{Duration, with_timeout}; use embedded_services::GlobalRawMutex; @@ -21,7 +20,7 @@ use power_policy_interface::{ service::{UnconstrainedState, event::Event as ServiceEvent}, }; use power_policy_interface_test_mocks::charger::ChargerType; -use power_policy_interface_test_mocks::psu::{FnCall, Mock}; +use power_policy_interface_test_mocks::psu::Mock; use power_policy_service::service::{Service, config::Config, customization}; use power_policy_service::{psu::PsuEventReceivers, service::registration::ArrayRegistration}; @@ -43,9 +42,13 @@ pub const HIGH_POWER: PowerCapability = PowerCapability { pub const DEFAULT_TIMEOUT: Duration = Duration::from_secs(15); +/// Default timeout for waiting for the power policy task to process events. +/// Used in tests where no service event is expected after an action. +pub const DEFAULT_PER_CALL_TIMEOUT: Duration = Duration::from_millis(100); + const EVENT_CHANNEL_SIZE: usize = 4; -pub type DeviceType<'a> = Mutex>>; +pub type DeviceType<'a> = Mutex>>; pub type ServiceType<'device, 'sender, Customization> = Service< 'device, ArrayRegistration< @@ -54,7 +57,7 @@ pub type ServiceType<'device, 'sender, Customization> = Service< 2, DynamicSender<'sender, ServiceEvent<'device, DeviceType<'device>>>, 1, - ChargerType<'device>, + ChargerType>, 0, >, Customization, @@ -64,7 +67,7 @@ pub type ServiceMutex<'device, 'sender, Customization> = Mutex>; async fn power_policy_task<'device, 'sender, const N: usize, Customization: customization::Customization>( - completion_signal: &'device Signal, + completion_signal: &'device embassy_sync::signal::Signal, power_policy: &ServiceMutex<'device, 'sender, Customization>, mut event_receivers: PsuEventReceivers<'device, N, DeviceType<'device>, DynamicReceiver<'device, EventData>>, ) { @@ -85,9 +88,7 @@ pub trait Test { service: &'a ServiceMutex<'a, 'a, Self::Customization>, service_receiver: DynamicReceiver<'a, ServiceEvent<'a, DeviceType<'a>>>, device0: &'a DeviceType<'a>, - device0_signal: &'a Signal, device1: &'a DeviceType<'a>, - device1_signal: &'a Signal, ) -> impl Future; } @@ -101,19 +102,17 @@ pub async fn run_test(timeout: Duration, mut test: T, config: Config, c let _ = env_logger::builder().filter_level(log::LevelFilter::Info).try_init(); embedded_services::init().await; - let device0_signal = Signal::new(); let device0_event_channel: Channel = Channel::new(); let device0_sender = device0_event_channel.dyn_sender(); let device0_receiver = device0_event_channel.dyn_receiver(); - let device0 = Mutex::new(Mock::new("PSU0", device0_sender, &device0_signal)); + let device0 = Mutex::new(Mock::new("PSU0", device0_sender)); - let device1_signal = Signal::new(); let device1_event_channel: Channel = Channel::new(); let device1_sender = device1_event_channel.dyn_sender(); let device1_receiver = device1_event_channel.dyn_receiver(); - let device1 = Mutex::new(Mock::new("PSU1", device1_sender, &device1_signal)); + let device1 = Mutex::new(Mock::new("PSU1", device1_sender)); - let completion_signal = Signal::new(); + let completion_signal = embassy_sync::signal::Signal::new(); // For simplicity, Test::run is only generic over a single lifetime. But this causes issues with the drop checker because // the device lifetime doesn't outlive the channel lifetime from its perspective. Use ManuallyDrop to work around this. @@ -143,15 +142,7 @@ pub async fn run_test(timeout: Duration, mut test: T, config: Config, c PsuEventReceivers::new([&device0, &device1], [device0_receiver, device1_receiver]), ), async { - test.run( - &power_policy, - service_receiver, - &device0, - &device0_signal, - &device1, - &device1_signal, - ) - .await; + test.run(&power_policy, service_receiver, &device0, &device1).await; completion_signal.signal(()); }, ), diff --git a/power-policy-service/tests/consumer.rs b/power-policy-service/tests/consumer.rs index 8614c3c88..8e4dd7738 100644 --- a/power-policy-service/tests/consumer.rs +++ b/power-policy-service/tests/consumer.rs @@ -1,8 +1,5 @@ #![allow(clippy::unwrap_used)] use embassy_sync::channel::DynamicReceiver; -use embassy_sync::signal::Signal; -use embassy_time::{Duration, TimeoutError, with_timeout}; -use embedded_services::GlobalRawMutex; use embedded_services::info; use embedded_services::sync::Lockable; use power_policy_interface::capability::ProviderFlags; @@ -23,6 +20,7 @@ use power_policy_service::service::customization; use power_policy_service::service::customization::DefaultCustomization; use power_policy_service::service::registration::Registration; +use crate::common::DEFAULT_PER_CALL_TIMEOUT; use crate::common::DeviceType; use crate::common::MINIMAL_POWER; use crate::common::Test; @@ -35,8 +33,6 @@ use crate::common::{ }; use power_policy_interface_test_mocks::psu::FnCall; -const PER_CALL_TIMEOUT: Duration = Duration::from_millis(1000); - const MIN_CONSUMER_THRESHOLD_MW: u32 = 7500; /// Test the basic consumer flow with a single device. @@ -50,31 +46,18 @@ impl Test for TestSingle { service: &ServiceMutex<'a, 'a, Self::Customization>, service_receiver: DynamicReceiver<'a, ServiceEvent<'a, DeviceType<'a>>>, device0: &DeviceType<'a>, - device0_signal: &Signal, _device1: &DeviceType<'a>, - _device1_signal: &Signal, ) { info!("Running test_single"); // Test initial connection { + device0.lock().await.next_result_connect_consumer.push_back(Ok(())); device0 .lock() .await .simulate_consumer_connection(LOW_POWER.into()) .await; - assert_eq!( - with_timeout(PER_CALL_TIMEOUT, device0_signal.wait()).await.unwrap(), - ( - 1, - FnCall::ConnectConsumer(ConsumerPowerCapability { - capability: LOW_POWER, - flags: ConsumerFlags::none(), - }) - ) - ); - device0_signal.reset(); - assert_consumer_connected( service_receiver, device0, @@ -85,6 +68,18 @@ impl Test for TestSingle { ) .await; + { + let mut device = device0.lock().await; + assert_eq!( + device.fn_calls.pop_front().unwrap(), + FnCall::ConnectConsumer(ConsumerPowerCapability { + capability: LOW_POWER, + flags: ConsumerFlags::none(), + }) + ); + assert!(device.fn_calls.is_empty()); + } + // Ensure consumer change doesn't affect provider power computation assert_eq!(service.lock().await.compute_total_provider_power_mw().await, 0); } @@ -92,14 +87,11 @@ impl Test for TestSingle { { device0.lock().await.simulate_detach().await; - // Power policy shouldn't call any functions on detach so we'll timeout - assert_eq!( - with_timeout(PER_CALL_TIMEOUT, device0_signal.wait()).await, - Err(TimeoutError) - ); - device0_signal.reset(); - assert_consumer_disconnected(service_receiver, device0).await; + + // Power policy shouldn't call any functions on detach + assert!(device0.lock().await.fn_calls.is_empty()); + // Ensure consumer change doesn't affect provider power computation assert_eq!(service.lock().await.compute_total_provider_power_mw().await, 0); } @@ -119,31 +111,18 @@ impl Test for TestSwapHigher { service: &ServiceMutex<'a, 'a, Self::Customization>, service_receiver: DynamicReceiver<'a, ServiceEvent<'a, DeviceType<'a>>>, device0: &DeviceType<'a>, - device0_signal: &Signal, device1: &DeviceType<'a>, - device1_signal: &Signal, ) { info!("Running test_swap_higher"); // Device0 connection at low power { + device0.lock().await.next_result_connect_consumer.push_back(Ok(())); device0 .lock() .await .simulate_consumer_connection(LOW_POWER.into()) .await; - assert_eq!( - with_timeout(PER_CALL_TIMEOUT, device0_signal.wait()).await.unwrap(), - ( - 1, - FnCall::ConnectConsumer(ConsumerPowerCapability { - capability: LOW_POWER, - flags: ConsumerFlags::none(), - }) - ) - ); - device0_signal.reset(); - assert_consumer_connected( service_receiver, device0, @@ -154,35 +133,31 @@ impl Test for TestSwapHigher { ) .await; + { + let mut device = device0.lock().await; + assert_eq!( + device.fn_calls.pop_front().unwrap(), + FnCall::ConnectConsumer(ConsumerPowerCapability { + capability: LOW_POWER, + flags: ConsumerFlags::none(), + }) + ); + assert!(device.fn_calls.is_empty()); + } + // Ensure consumer change doesn't affect provider power computation assert_eq!(service.lock().await.compute_total_provider_power_mw().await, 0); } // Device1 connection at high power { + device0.lock().await.next_result_disconnect.push_back(Ok(())); + device1.lock().await.next_result_connect_consumer.push_back(Ok(())); device1 .lock() .await .simulate_consumer_connection(HIGH_POWER.into()) .await; - assert_eq!( - with_timeout(PER_CALL_TIMEOUT, device0_signal.wait()).await.unwrap(), - (1, FnCall::Disconnect) - ); - device0_signal.reset(); - - assert_eq!( - with_timeout(PER_CALL_TIMEOUT, device1_signal.wait()).await.unwrap(), - ( - 1, - FnCall::ConnectConsumer(ConsumerPowerCapability { - capability: HIGH_POWER, - flags: ConsumerFlags::none(), - }) - ) - ); - device1_signal.reset(); - // Should receive a disconnect event from device0 first assert_consumer_disconnected(service_receiver, device0).await; @@ -196,31 +171,31 @@ impl Test for TestSwapHigher { ) .await; + { + let mut device0 = device0.lock().await; + assert_eq!(device0.fn_calls.pop_front().unwrap(), FnCall::Disconnect); + assert!(device0.fn_calls.is_empty()); + } + { + let mut device1 = device1.lock().await; + assert_eq!( + device1.fn_calls.pop_front().unwrap(), + FnCall::ConnectConsumer(ConsumerPowerCapability { + capability: HIGH_POWER, + flags: ConsumerFlags::none(), + }) + ); + assert!(device1.fn_calls.is_empty()); + } + // Ensure consumer change doesn't affect provider power computation assert_eq!(service.lock().await.compute_total_provider_power_mw().await, 0); } // Test detach device1, should reconnect device0 { + device0.lock().await.next_result_connect_consumer.push_back(Ok(())); device1.lock().await.simulate_detach().await; - // Power policy shouldn't call any functions on detach so we'll timeout - assert_eq!( - with_timeout(PER_CALL_TIMEOUT, device1_signal.wait()).await, - Err(TimeoutError) - ); - - assert_eq!( - with_timeout(PER_CALL_TIMEOUT, device0_signal.wait()).await.unwrap(), - ( - 1, - FnCall::ConnectConsumer(ConsumerPowerCapability { - capability: LOW_POWER, - flags: ConsumerFlags::none(), - }) - ) - ); - device0_signal.reset(); - // Should receive a disconnect event from device1 first assert_consumer_disconnected(service_receiver, device1).await; @@ -234,6 +209,21 @@ impl Test for TestSwapHigher { ) .await; + // Power policy shouldn't call any functions on device1 for detach + assert!(device1.lock().await.fn_calls.is_empty()); + + { + let mut device0 = device0.lock().await; + assert_eq!( + device0.fn_calls.pop_front().unwrap(), + FnCall::ConnectConsumer(ConsumerPowerCapability { + capability: LOW_POWER, + flags: ConsumerFlags::none(), + }) + ); + assert!(device0.fn_calls.is_empty()); + } + // Ensure consumer change doesn't affect provider power computation assert_eq!(service.lock().await.compute_total_provider_power_mw().await, 0); } @@ -253,31 +243,18 @@ impl Test for TestDisconnect { service: &ServiceMutex<'a, 'a, Self::Customization>, service_receiver: DynamicReceiver<'a, ServiceEvent<'a, DeviceType<'a>>>, device0: &DeviceType<'a>, - device0_signal: &Signal, device1: &DeviceType<'a>, - device1_signal: &Signal, ) { info!("Running test_disconnect"); // Device0 connection at low power { + device0.lock().await.next_result_connect_consumer.push_back(Ok(())); device0 .lock() .await .simulate_consumer_connection(LOW_POWER.into()) .await; - assert_eq!( - with_timeout(PER_CALL_TIMEOUT, device0_signal.wait()).await.unwrap(), - ( - 1, - FnCall::ConnectConsumer(ConsumerPowerCapability { - capability: LOW_POWER, - flags: ConsumerFlags::none(), - }) - ) - ); - device0_signal.reset(); - assert_consumer_connected( service_receiver, device0, @@ -288,35 +265,31 @@ impl Test for TestDisconnect { ) .await; + { + let mut device = device0.lock().await; + assert_eq!( + device.fn_calls.pop_front().unwrap(), + FnCall::ConnectConsumer(ConsumerPowerCapability { + capability: LOW_POWER, + flags: ConsumerFlags::none(), + }) + ); + assert!(device.fn_calls.is_empty()); + } + // Ensure consumer change doesn't affect provider power computation assert_eq!(service.lock().await.compute_total_provider_power_mw().await, 0); } // Device1 connection at high power { + device0.lock().await.next_result_disconnect.push_back(Ok(())); + device1.lock().await.next_result_connect_consumer.push_back(Ok(())); device1 .lock() .await .simulate_consumer_connection(HIGH_POWER.into()) .await; - assert_eq!( - with_timeout(PER_CALL_TIMEOUT, device0_signal.wait()).await.unwrap(), - (1, FnCall::Disconnect) - ); - device0_signal.reset(); - - assert_eq!( - with_timeout(PER_CALL_TIMEOUT, device1_signal.wait()).await.unwrap(), - ( - 1, - FnCall::ConnectConsumer(ConsumerPowerCapability { - capability: HIGH_POWER, - flags: ConsumerFlags::none(), - }) - ) - ); - device1_signal.reset(); - // Should receive a disconnect event from device0 first assert_consumer_disconnected(service_receiver, device0).await; @@ -330,32 +303,32 @@ impl Test for TestDisconnect { ) .await; + { + let mut device0 = device0.lock().await; + assert_eq!(device0.fn_calls.pop_front().unwrap(), FnCall::Disconnect); + assert!(device0.fn_calls.is_empty()); + } + { + let mut device1 = device1.lock().await; + assert_eq!( + device1.fn_calls.pop_front().unwrap(), + FnCall::ConnectConsumer(ConsumerPowerCapability { + capability: HIGH_POWER, + flags: ConsumerFlags::none(), + }) + ); + assert!(device1.fn_calls.is_empty()); + } + // Ensure consumer change doesn't affect provider power computation assert_eq!(service.lock().await.compute_total_provider_power_mw().await, 0); } // Test disconnect device1, should reconnect device0 { + device0.lock().await.next_result_connect_consumer.push_back(Ok(())); device1.lock().await.simulate_disconnect().await; - // Power policy shouldn't call any functions on disconnect so we'll timeout - assert_eq!( - with_timeout(PER_CALL_TIMEOUT, device1_signal.wait()).await, - Err(TimeoutError) - ); - - assert_eq!( - with_timeout(PER_CALL_TIMEOUT, device0_signal.wait()).await.unwrap(), - ( - 1, - FnCall::ConnectConsumer(ConsumerPowerCapability { - capability: LOW_POWER, - flags: ConsumerFlags::none(), - }) - ) - ); - device0_signal.reset(); - // Consume the disconnect event generated by `simulate_disconnect` assert_consumer_disconnected(service_receiver, device1).await; @@ -369,6 +342,21 @@ impl Test for TestDisconnect { ) .await; + // Power policy shouldn't call any functions on device1 for disconnect + assert!(device1.lock().await.fn_calls.is_empty()); + + { + let mut device0 = device0.lock().await; + assert_eq!( + device0.fn_calls.pop_front().unwrap(), + FnCall::ConnectConsumer(ConsumerPowerCapability { + capability: LOW_POWER, + flags: ConsumerFlags::none(), + }) + ); + assert!(device0.fn_calls.is_empty()); + } + // Ensure consumer change doesn't affect provider power computation assert_eq!(service.lock().await.compute_total_provider_power_mw().await, 0); } @@ -388,31 +376,18 @@ impl Test for TestDisconnectOtherConsumer { service: &ServiceMutex<'a, 'a, Self::Customization>, service_receiver: DynamicReceiver<'a, ServiceEvent<'a, DeviceType<'a>>>, device0: &DeviceType<'a>, - device0_signal: &Signal, device1: &DeviceType<'a>, - device1_signal: &Signal, ) { info!("Running test_disconnect_other_consumer"); // Device0 connection at high power { + device0.lock().await.next_result_connect_consumer.push_back(Ok(())); device0 .lock() .await .simulate_consumer_connection(HIGH_POWER.into()) .await; - assert_eq!( - with_timeout(PER_CALL_TIMEOUT, device0_signal.wait()).await.unwrap(), - ( - 1, - FnCall::ConnectConsumer(ConsumerPowerCapability { - capability: HIGH_POWER, - flags: ConsumerFlags::none(), - }) - ) - ); - device0_signal.reset(); - assert_consumer_connected( service_receiver, device0, @@ -423,10 +398,22 @@ impl Test for TestDisconnectOtherConsumer { ) .await; + { + let mut device = device0.lock().await; + assert_eq!( + device.fn_calls.pop_front().unwrap(), + FnCall::ConnectConsumer(ConsumerPowerCapability { + capability: HIGH_POWER, + flags: ConsumerFlags::none(), + }) + ); + assert!(device.fn_calls.is_empty()); + } + // Ensure consumer change doesn't affect provider power computation assert_eq!(service.lock().await.compute_total_provider_power_mw().await, 0); } - // Device1 connection at low power + // Device1 connection at low power (should not swap since device0 has higher power) { device1 .lock() @@ -434,11 +421,11 @@ impl Test for TestDisconnectOtherConsumer { .simulate_consumer_connection(LOW_POWER.into()) .await; - assert_eq!( - with_timeout(PER_CALL_TIMEOUT, device1_signal.wait()).await, - Err(TimeoutError) - ); - device1_signal.reset(); + embassy_time::Timer::after(DEFAULT_PER_CALL_TIMEOUT).await; + + // No fn_calls should be made (no swap) + assert!(device0.lock().await.fn_calls.is_empty()); + assert!(device1.lock().await.fn_calls.is_empty()); // Ensure consumer change doesn't affect provider power computation assert_eq!(service.lock().await.compute_total_provider_power_mw().await, 0); @@ -448,15 +435,11 @@ impl Test for TestDisconnectOtherConsumer { { device1.lock().await.simulate_disconnect().await; - // Power policy shouldn't call any functions on disconnect so we'll timeout - assert_eq!( - with_timeout(PER_CALL_TIMEOUT, device0_signal.wait()).await, - Err(TimeoutError) - ); - assert_eq!( - with_timeout(PER_CALL_TIMEOUT, device1_signal.wait()).await, - Err(TimeoutError) - ); + embassy_time::Timer::after(DEFAULT_PER_CALL_TIMEOUT).await; + + // Power policy shouldn't call any functions on disconnect + assert!(device0.lock().await.fn_calls.is_empty()); + assert!(device1.lock().await.fn_calls.is_empty()); // Ensure consumer change doesn't affect provider power computation assert_eq!(service.lock().await.compute_total_provider_power_mw().await, 0); @@ -477,31 +460,18 @@ impl Test for TestDisconnectOtherProvider { service: &ServiceMutex<'a, 'a, Self::Customization>, service_receiver: DynamicReceiver<'a, ServiceEvent<'a, DeviceType<'a>>>, device0: &DeviceType<'a>, - device0_signal: &Signal, device1: &DeviceType<'a>, - device1_signal: &Signal, ) { info!("Running test_disconnect_other_provider"); // Device0 connection at high power { + device0.lock().await.next_result_connect_consumer.push_back(Ok(())); device0 .lock() .await .simulate_consumer_connection(HIGH_POWER.into()) .await; - assert_eq!( - with_timeout(PER_CALL_TIMEOUT, device0_signal.wait()).await.unwrap(), - ( - 1, - FnCall::ConnectConsumer(ConsumerPowerCapability { - capability: HIGH_POWER, - flags: ConsumerFlags::none(), - }) - ) - ); - device0_signal.reset(); - assert_consumer_connected( service_receiver, device0, @@ -512,25 +482,26 @@ impl Test for TestDisconnectOtherProvider { ) .await; + { + let mut device = device0.lock().await; + assert_eq!( + device.fn_calls.pop_front().unwrap(), + FnCall::ConnectConsumer(ConsumerPowerCapability { + capability: HIGH_POWER, + flags: ConsumerFlags::none(), + }) + ); + assert!(device.fn_calls.is_empty()); + } + // Ensure consumer change doesn't affect provider power computation assert_eq!(service.lock().await.compute_total_provider_power_mw().await, 0); } // Device1 connect as provider { + device1.lock().await.next_result_connect_provider.push_back(Ok(())); device1.lock().await.simulate_provider_connection(LOW_POWER).await; - assert_eq!( - with_timeout(PER_CALL_TIMEOUT, device1_signal.wait()).await.unwrap(), - ( - 1, - FnCall::ConnectProvider(ProviderPowerCapability { - capability: LOW_POWER, - flags: ProviderFlags::none(), - }) - ) - ); - device1_signal.reset(); - assert_provider_connected( service_receiver, device1, @@ -540,6 +511,19 @@ impl Test for TestDisconnectOtherProvider { }, ) .await; + + { + let mut device = device1.lock().await; + assert_eq!( + device.fn_calls.pop_front().unwrap(), + FnCall::ConnectProvider(ProviderPowerCapability { + capability: LOW_POWER, + flags: ProviderFlags::none(), + }) + ); + assert!(device.fn_calls.is_empty()); + } + assert_eq!(service.lock().await.compute_total_provider_power_mw().await, 7500); } @@ -547,17 +531,12 @@ impl Test for TestDisconnectOtherProvider { { device1.lock().await.simulate_disconnect().await; - // Power policy shouldn't call any functions on disconnect so we'll timeout - assert_eq!( - with_timeout(PER_CALL_TIMEOUT, device0_signal.wait()).await, - Err(TimeoutError) - ); - assert_eq!( - with_timeout(PER_CALL_TIMEOUT, device1_signal.wait()).await, - Err(TimeoutError) - ); - assert_provider_disconnected(service_receiver, device1).await; + + // Power policy shouldn't call any functions on disconnect + assert!(device0.lock().await.fn_calls.is_empty()); + assert!(device1.lock().await.fn_calls.is_empty()); + assert_eq!(service.lock().await.compute_total_provider_power_mw().await, 0); } @@ -578,9 +557,7 @@ impl Test for TestMinConsumerPower { service: &ServiceMutex<'a, 'a, Self::Customization>, service_receiver: DynamicReceiver<'a, ServiceEvent<'a, DeviceType<'a>>>, device0: &DeviceType<'a>, - device0_signal: &Signal, _device1: &DeviceType<'a>, - _device1_signal: &Signal, ) { info!("Running test_min_consumer_power"); // Connect with power below the minimum threshold. @@ -591,12 +568,10 @@ impl Test for TestMinConsumerPower { .simulate_consumer_connection(MINIMAL_POWER.into()) .await; - // Power policy shouldn't connect, so this call should timeout. - assert_eq!( - with_timeout(PER_CALL_TIMEOUT, device0_signal.wait()).await, - Err(TimeoutError) - ); - device0_signal.reset(); + embassy_time::Timer::after(DEFAULT_PER_CALL_TIMEOUT).await; + + // Power policy shouldn't connect since power is below threshold + assert!(device0.lock().await.fn_calls.is_empty()); // Ensure consumer change doesn't affect provider power computation assert_eq!(service.lock().await.compute_total_provider_power_mw().await, 0); @@ -618,31 +593,18 @@ impl Test for TestNoSwap { service: &ServiceMutex<'a, 'a, Self::Customization>, service_receiver: DynamicReceiver<'a, ServiceEvent<'a, DeviceType<'a>>>, device0: &DeviceType<'a>, - device0_signal: &Signal, device1: &DeviceType<'a>, - device1_signal: &Signal, ) { info!("Running test_no_swap"); // Device0 connection at low power { + device0.lock().await.next_result_connect_consumer.push_back(Ok(())); device0 .lock() .await .simulate_consumer_connection(LOW_POWER.into()) .await; - assert_eq!( - with_timeout(PER_CALL_TIMEOUT, device0_signal.wait()).await.unwrap(), - ( - 1, - FnCall::ConnectConsumer(ConsumerPowerCapability { - capability: LOW_POWER, - flags: ConsumerFlags::none(), - }) - ) - ); - device0_signal.reset(); - assert_consumer_connected( service_receiver, device0, @@ -653,6 +615,18 @@ impl Test for TestNoSwap { ) .await; + { + let mut device = device0.lock().await; + assert_eq!( + device.fn_calls.pop_front().unwrap(), + FnCall::ConnectConsumer(ConsumerPowerCapability { + capability: LOW_POWER, + flags: ConsumerFlags::none(), + }) + ); + assert!(device.fn_calls.is_empty()); + } + // Ensure consumer change doesn't affect provider power computation assert_eq!(service.lock().await.compute_total_provider_power_mw().await, 0); } @@ -664,18 +638,11 @@ impl Test for TestNoSwap { .simulate_consumer_connection(LOW_POWER.into()) .await; - // These should timeout since we shouldn't swap - assert_eq!( - with_timeout(PER_CALL_TIMEOUT, device0_signal.wait()).await, - Err(TimeoutError) - ); - device0_signal.reset(); + embassy_time::Timer::after(DEFAULT_PER_CALL_TIMEOUT).await; - assert_eq!( - with_timeout(PER_CALL_TIMEOUT, device1_signal.wait()).await, - Err(TimeoutError) - ); - device1_signal.reset(); + // These should be empty since we shouldn't swap + assert!(device0.lock().await.fn_calls.is_empty()); + assert!(device1.lock().await.fn_calls.is_empty()); // Shouldn't affect provider power computation assert_eq!(service.lock().await.compute_total_provider_power_mw().await, 0); @@ -719,32 +686,19 @@ impl Test for TestFindBestConsumerCustomization { _service: &ServiceMutex<'a, 'a, Self::Customization>, service_receiver: DynamicReceiver<'a, ServiceEvent<'a, DeviceType<'a>>>, device0: &DeviceType<'a>, - device0_signal: &Signal, device1: &DeviceType<'a>, - device1_signal: &Signal, ) { info!("Running TestFindBestConsumerCustomization"); // Device1 connection at high power { + device1.lock().await.next_result_connect_consumer.push_back(Ok(())); device1 .lock() .await .simulate_consumer_connection(HIGH_POWER.into()) .await; - assert_eq!( - with_timeout(PER_CALL_TIMEOUT, device1_signal.wait()).await.unwrap(), - ( - 1, - FnCall::ConnectConsumer(ConsumerPowerCapability { - capability: HIGH_POWER, - flags: ConsumerFlags::none(), - }) - ) - ); - device1_signal.reset(); - assert_consumer_connected( service_receiver, device1, @@ -754,29 +708,31 @@ impl Test for TestFindBestConsumerCustomization { }, ) .await; + + { + let mut device = device1.lock().await; + assert_eq!( + device.fn_calls.pop_front().unwrap(), + FnCall::ConnectConsumer(ConsumerPowerCapability { + capability: HIGH_POWER, + flags: ConsumerFlags::none(), + }) + ); + assert!(device.fn_calls.is_empty()); + } } // Device0 connection at low power // Since we're using a custom hook, we expect to switch to it { + device1.lock().await.next_result_disconnect.push_back(Ok(())); + device0.lock().await.next_result_connect_consumer.push_back(Ok(())); device0 .lock() .await .simulate_consumer_connection(LOW_POWER.into()) .await; - assert_eq!( - with_timeout(PER_CALL_TIMEOUT, device0_signal.wait()).await.unwrap(), - ( - 1, - FnCall::ConnectConsumer(ConsumerPowerCapability { - capability: LOW_POWER, - flags: ConsumerFlags::none(), - }) - ) - ); - device0_signal.reset(); - // Should receive a disconnect event from device1 first assert_consumer_disconnected(service_receiver, device1).await; @@ -789,6 +745,23 @@ impl Test for TestFindBestConsumerCustomization { }, ) .await; + + { + let mut device1 = device1.lock().await; + assert_eq!(device1.fn_calls.pop_front().unwrap(), FnCall::Disconnect); + assert!(device1.fn_calls.is_empty()); + } + { + let mut device0 = device0.lock().await; + assert_eq!( + device0.fn_calls.pop_front().unwrap(), + FnCall::ConnectConsumer(ConsumerPowerCapability { + capability: LOW_POWER, + flags: ConsumerFlags::none(), + }) + ); + assert!(device0.fn_calls.is_empty()); + } } } } @@ -805,28 +778,16 @@ impl Test for TestConsumerDisconnectSwitchingFlag { _service: &ServiceMutex<'a, 'a, Self::Customization>, service_receiver: DynamicReceiver<'a, ServiceEvent<'a, DeviceType<'a>>>, device0: &DeviceType<'a>, - device0_signal: &Signal, device1: &DeviceType<'a>, - device1_signal: &Signal, ) { info!("Running test_consumer_disconnect_switching_flag"); // Connect device0 at low power. + device0.lock().await.next_result_connect_consumer.push_back(Ok(())); device0 .lock() .await .simulate_consumer_connection(LOW_POWER.into()) .await; - assert_eq!( - with_timeout(PER_CALL_TIMEOUT, device0_signal.wait()).await.unwrap(), - ( - 1, - FnCall::ConnectConsumer(ConsumerPowerCapability { - capability: LOW_POWER, - flags: ConsumerFlags::none(), - }) - ) - ); - device0_signal.reset(); assert_consumer_connected( service_receiver, device0, @@ -837,28 +798,26 @@ impl Test for TestConsumerDisconnectSwitchingFlag { ) .await; + { + let mut device0 = device0.lock().await; + assert_eq!( + device0.fn_calls.pop_front().unwrap(), + FnCall::ConnectConsumer(ConsumerPowerCapability { + capability: LOW_POWER, + flags: ConsumerFlags::none(), + }) + ); + assert!(device0.fn_calls.is_empty()); + } + // Connect device1 at high power, the service should switch to it. + device0.lock().await.next_result_disconnect.push_back(Ok(())); + device1.lock().await.next_result_connect_consumer.push_back(Ok(())); device1 .lock() .await .simulate_consumer_connection(HIGH_POWER.into()) .await; - assert_eq!( - with_timeout(PER_CALL_TIMEOUT, device0_signal.wait()).await.unwrap(), - (1, FnCall::Disconnect) - ); - device0_signal.reset(); - assert_eq!( - with_timeout(PER_CALL_TIMEOUT, device1_signal.wait()).await.unwrap(), - ( - 1, - FnCall::ConnectConsumer(ConsumerPowerCapability { - capability: HIGH_POWER, - flags: ConsumerFlags::none(), - }) - ) - ); - device1_signal.reset(); // device0 should be disconnected with the switching flag set since we're switching to device1. assert_consumer_disconnected_with_flags( @@ -877,6 +836,23 @@ impl Test for TestConsumerDisconnectSwitchingFlag { ) .await; + { + let mut device0 = device0.lock().await; + assert_eq!(device0.fn_calls.pop_front().unwrap(), FnCall::Disconnect); + assert!(device0.fn_calls.is_empty()); + } + { + let mut device1 = device1.lock().await; + assert_eq!( + device1.fn_calls.pop_front().unwrap(), + FnCall::ConnectConsumer(ConsumerPowerCapability { + capability: HIGH_POWER, + flags: ConsumerFlags::none(), + }) + ); + assert!(device1.fn_calls.is_empty()); + } + assert_no_event(service_receiver); } } @@ -893,28 +869,16 @@ impl Test for TestConsumerDisconnectRenegotiationFlag { _service: &ServiceMutex<'a, 'a, Self::Customization>, service_receiver: DynamicReceiver<'a, ServiceEvent<'a, DeviceType<'a>>>, device0: &DeviceType<'a>, - device0_signal: &Signal, _device1: &DeviceType<'a>, - _device1_signal: &Signal, ) { info!("Running test_consumer_disconnect_renegotiation_flag"); // Connect device0 at low power. + device0.lock().await.next_result_connect_consumer.push_back(Ok(())); device0 .lock() .await .simulate_consumer_connection(LOW_POWER.into()) .await; - assert_eq!( - with_timeout(PER_CALL_TIMEOUT, device0_signal.wait()).await.unwrap(), - ( - 1, - FnCall::ConnectConsumer(ConsumerPowerCapability { - capability: LOW_POWER, - flags: ConsumerFlags::none(), - }) - ) - ); - device0_signal.reset(); assert_consumer_connected( service_receiver, device0, @@ -925,9 +889,23 @@ impl Test for TestConsumerDisconnectRenegotiationFlag { ) .await; + { + let mut device0 = device0.lock().await; + assert_eq!( + device0.fn_calls.pop_front().unwrap(), + FnCall::ConnectConsumer(ConsumerPowerCapability { + capability: LOW_POWER, + flags: ConsumerFlags::none(), + }) + ); + assert!(device0.fn_calls.is_empty()); + } + // The same device renegotiates a new (higher) power capability. Since the best consumer is // still the same device but with a different capability, the service disconnects and // reconnects it. The disconnect event should carry the renegotiation flag. + device0.lock().await.next_result_disconnect.push_back(Ok(())); + device0.lock().await.next_result_connect_consumer.push_back(Ok(())); device0 .lock() .await @@ -950,6 +928,19 @@ impl Test for TestConsumerDisconnectRenegotiationFlag { ) .await; + { + let mut device0 = device0.lock().await; + assert_eq!(device0.fn_calls.pop_front().unwrap(), FnCall::Disconnect); + assert_eq!( + device0.fn_calls.pop_front().unwrap(), + FnCall::ConnectConsumer(ConsumerPowerCapability { + capability: HIGH_POWER, + flags: ConsumerFlags::none(), + }) + ); + assert!(device0.fn_calls.is_empty()); + } + assert_no_event(service_receiver); } } diff --git a/power-policy-service/tests/provider.rs b/power-policy-service/tests/provider.rs index d205c3242..9533c9b70 100644 --- a/power-policy-service/tests/provider.rs +++ b/power-policy-service/tests/provider.rs @@ -1,8 +1,5 @@ #![allow(clippy::unwrap_used)] use embassy_sync::channel::DynamicReceiver; -use embassy_sync::signal::Signal; -use embassy_time::{Duration, TimeoutError, with_timeout}; -use embedded_services::GlobalRawMutex; use embedded_services::info; use power_policy_interface::capability::ProviderFlags; use power_policy_interface::capability::ProviderPowerCapability; @@ -20,8 +17,6 @@ use crate::common::assert_no_event; use crate::common::{DEFAULT_TIMEOUT, assert_provider_connected, assert_provider_disconnected, run_test}; use power_policy_interface_test_mocks::psu::FnCall; -const PER_CALL_TIMEOUT: Duration = Duration::from_millis(1000); - /// Test the basic provider flow with a single device. struct TestSingle; @@ -33,27 +28,14 @@ impl Test for TestSingle { _service: &ServiceMutex<'a, 'a, Self::Customization>, service_receiver: DynamicReceiver<'a, ServiceEvent<'a, DeviceType<'a>>>, device0: &DeviceType<'a>, - device0_signal: &Signal, _device1: &DeviceType<'a>, - _device1_signal: &Signal, ) { info!("Running test_single"); // Test initial connection { + device0.lock().await.next_result_connect_provider.push_back(Ok(())); device0.lock().await.simulate_provider_connection(LOW_POWER).await; - assert_eq!( - with_timeout(PER_CALL_TIMEOUT, device0_signal.wait()).await.unwrap(), - ( - 1, - FnCall::ConnectProvider(ProviderPowerCapability { - capability: LOW_POWER, - flags: ProviderFlags::none(), - }) - ) - ); - device0_signal.reset(); - assert_provider_connected( service_receiver, device0, @@ -63,19 +45,27 @@ impl Test for TestSingle { }, ) .await; + + { + let mut device = device0.lock().await; + assert_eq!( + device.fn_calls.pop_front().unwrap(), + FnCall::ConnectProvider(ProviderPowerCapability { + capability: LOW_POWER, + flags: ProviderFlags::none(), + }) + ); + assert!(device.fn_calls.is_empty()); + } } // Test detach { device0.lock().await.simulate_detach().await; - // Power policy shouldn't call any functions on detach so we'll timeout - assert_eq!( - with_timeout(PER_CALL_TIMEOUT, device0_signal.wait()).await, - Err(TimeoutError) - ); - device0_signal.reset(); - assert_provider_disconnected(service_receiver, device0).await; + + // Power policy shouldn't call any functions on detach + assert!(device0.lock().await.fn_calls.is_empty()); } assert_no_event(service_receiver); @@ -93,27 +83,14 @@ impl Test for TestUpgrade { service: &ServiceMutex<'a, 'a, Self::Customization>, service_receiver: DynamicReceiver<'a, ServiceEvent<'a, DeviceType<'a>>>, device0: &DeviceType<'a>, - device0_signal: &Signal, device1: &DeviceType<'a>, - device1_signal: &Signal, ) { info!("Running test_upgrade"); { // Connect device0 at high power, default service config should allow this + device0.lock().await.next_result_connect_provider.push_back(Ok(())); device0.lock().await.simulate_provider_connection(HIGH_POWER).await; - assert_eq!( - with_timeout(PER_CALL_TIMEOUT, device0_signal.wait()).await.unwrap(), - ( - 1, - FnCall::ConnectProvider(ProviderPowerCapability { - capability: HIGH_POWER, - flags: ProviderFlags::none(), - }) - ) - ); - device0_signal.reset(); - assert_provider_connected( service_receiver, device0, @@ -124,25 +101,26 @@ impl Test for TestUpgrade { ) .await; + { + let mut device = device0.lock().await; + assert_eq!( + device.fn_calls.pop_front().unwrap(), + FnCall::ConnectProvider(ProviderPowerCapability { + capability: HIGH_POWER, + flags: ProviderFlags::none(), + }) + ); + assert!(device.fn_calls.is_empty()); + } + assert_eq!(service.lock().await.compute_total_provider_power_mw().await, 15000); } { // Connect device1 at low power, default service config should allow this + device1.lock().await.next_result_connect_provider.push_back(Ok(())); device1.lock().await.simulate_provider_connection(LOW_POWER).await; - assert_eq!( - with_timeout(PER_CALL_TIMEOUT, device1_signal.wait()).await.unwrap(), - ( - 1, - FnCall::ConnectProvider(ProviderPowerCapability { - capability: LOW_POWER, - flags: ProviderFlags::none(), - }) - ) - ); - device1_signal.reset(); - assert_provider_connected( service_receiver, device1, @@ -153,30 +131,31 @@ impl Test for TestUpgrade { ) .await; + { + let mut device = device1.lock().await; + assert_eq!( + device.fn_calls.pop_front().unwrap(), + FnCall::ConnectProvider(ProviderPowerCapability { + capability: LOW_POWER, + flags: ProviderFlags::none(), + }) + ); + assert!(device.fn_calls.is_empty()); + } + assert_eq!(service.lock().await.compute_total_provider_power_mw().await, 22500); } { // Attempt to upgrade device1 to high power, power policy should reject this since device0 is already connected at high power // Power policy will instead allow us to connect at low power + device1.lock().await.next_result_connect_provider.push_back(Ok(())); device1 .lock() .await .simulate_update_requested_provider_power_capability(Some(HIGH_POWER.into())) .await; - assert_eq!( - with_timeout(PER_CALL_TIMEOUT, device1_signal.wait()).await.unwrap(), - ( - 1, - FnCall::ConnectProvider(ProviderPowerCapability { - capability: LOW_POWER, - flags: ProviderFlags::none(), - }) - ) - ); - device1_signal.reset(); - assert_provider_connected( service_receiver, device1, @@ -187,6 +166,18 @@ impl Test for TestUpgrade { ) .await; + { + let mut device = device1.lock().await; + assert_eq!( + device.fn_calls.pop_front().unwrap(), + FnCall::ConnectProvider(ProviderPowerCapability { + capability: LOW_POWER, + flags: ProviderFlags::none(), + }) + ); + assert!(device.fn_calls.is_empty()); + } + assert_eq!(service.lock().await.compute_total_provider_power_mw().await, 22500); } @@ -194,37 +185,23 @@ impl Test for TestUpgrade { // Detach device0, this should allow us to upgrade device1 to high power device0.lock().await.simulate_detach().await; - // Power policy shouldn't call any functions on detach so we'll timeout - assert_eq!( - with_timeout(PER_CALL_TIMEOUT, device0_signal.wait()).await, - Err(TimeoutError) - ); - device0_signal.reset(); - assert_provider_disconnected(service_receiver, device0).await; + + // Power policy shouldn't call any functions on detach + assert!(device0.lock().await.fn_calls.is_empty()); + assert_eq!(service.lock().await.compute_total_provider_power_mw().await, 7500); } { // Attempt to upgrade device1 to high power should now succeed + device1.lock().await.next_result_connect_provider.push_back(Ok(())); device1 .lock() .await .simulate_update_requested_provider_power_capability(Some(HIGH_POWER.into())) .await; - assert_eq!( - with_timeout(PER_CALL_TIMEOUT, device1_signal.wait()).await.unwrap(), - ( - 1, - FnCall::ConnectProvider(ProviderPowerCapability { - capability: HIGH_POWER, - flags: ProviderFlags::none(), - }) - ) - ); - device1_signal.reset(); - assert_provider_connected( service_receiver, device1, @@ -234,6 +211,19 @@ impl Test for TestUpgrade { }, ) .await; + + { + let mut device = device1.lock().await; + assert_eq!( + device.fn_calls.pop_front().unwrap(), + FnCall::ConnectProvider(ProviderPowerCapability { + capability: HIGH_POWER, + flags: ProviderFlags::none(), + }) + ); + assert!(device.fn_calls.is_empty()); + } + assert_eq!(service.lock().await.compute_total_provider_power_mw().await, 15000); } @@ -252,27 +242,14 @@ impl Test for TestDisconnect { service: &ServiceMutex<'a, 'a, Self::Customization>, service_receiver: DynamicReceiver<'a, ServiceEvent<'a, DeviceType<'a>>>, device0: &DeviceType<'a>, - device0_signal: &Signal, _device1: &DeviceType<'a>, - _device1_signal: &Signal, ) { info!("Running test_disconnect"); // Test initial connection { + device0.lock().await.next_result_connect_provider.push_back(Ok(())); device0.lock().await.simulate_provider_connection(LOW_POWER).await; - assert_eq!( - with_timeout(PER_CALL_TIMEOUT, device0_signal.wait()).await.unwrap(), - ( - 1, - FnCall::ConnectProvider(ProviderPowerCapability { - capability: LOW_POWER, - flags: ProviderFlags::none(), - }) - ) - ); - device0_signal.reset(); - assert_provider_connected( service_receiver, device0, @@ -282,20 +259,30 @@ impl Test for TestDisconnect { }, ) .await; + + { + let mut device = device0.lock().await; + assert_eq!( + device.fn_calls.pop_front().unwrap(), + FnCall::ConnectProvider(ProviderPowerCapability { + capability: LOW_POWER, + flags: ProviderFlags::none(), + }) + ); + assert!(device.fn_calls.is_empty()); + } + assert_eq!(service.lock().await.compute_total_provider_power_mw().await, 7500); } // Test disconnect { device0.lock().await.simulate_disconnect().await; - // Power policy shouldn't call any functions on disconnect so we'll timeout - assert_eq!( - with_timeout(PER_CALL_TIMEOUT, device0_signal.wait()).await, - Err(TimeoutError) - ); - device0_signal.reset(); - assert_provider_disconnected(service_receiver, device0).await; + + // Power policy shouldn't call any functions on disconnect + assert!(device0.lock().await.fn_calls.is_empty()); + assert_eq!(service.lock().await.compute_total_provider_power_mw().await, 0); } diff --git a/power-policy-service/tests/unconstrained.rs b/power-policy-service/tests/unconstrained.rs index a6ca897f8..a3e206d1f 100644 --- a/power-policy-service/tests/unconstrained.rs +++ b/power-policy-service/tests/unconstrained.rs @@ -1,9 +1,5 @@ #![allow(clippy::unwrap_used)] use embassy_sync::channel::DynamicReceiver; -use embassy_sync::signal::Signal; -use embassy_time::TimeoutError; -use embassy_time::{Duration, with_timeout}; -use embedded_services::GlobalRawMutex; use embedded_services::info; use power_policy_interface::capability::{ConsumerFlags, ConsumerPowerCapability}; @@ -19,10 +15,8 @@ use crate::common::{ DEFAULT_TIMEOUT, assert_consumer_connected, assert_consumer_disconnected, assert_no_event, assert_unconstrained, run_test, }; -use power_policy_interface_test_mocks::psu::FnCall; use crate::common::{DeviceType, ServiceMutex, Test}; - -const PER_CALL_TIMEOUT: Duration = Duration::from_millis(1000); +use power_policy_interface_test_mocks::psu::FnCall; /// Test unconstrained consumer flow with multiple devices. struct TestUnconstrained; @@ -35,31 +29,18 @@ impl Test for TestUnconstrained { _service: &ServiceMutex<'a, 'a, Self::Customization>, service_receiver: DynamicReceiver<'a, ServiceEvent<'a, DeviceType<'a>>>, device0: &DeviceType<'a>, - device0_signal: &Signal, device1: &DeviceType<'a>, - device1_signal: &Signal, ) { info!("Running test_unconstrained"); { // Connect device0, without unconstrained, + device0.lock().await.next_result_connect_consumer.push_back(Ok(())); device0 .lock() .await .simulate_consumer_connection(LOW_POWER.into()) .await; - assert_eq!( - with_timeout(PER_CALL_TIMEOUT, device0_signal.wait()).await.unwrap(), - ( - 1, - FnCall::ConnectConsumer(ConsumerPowerCapability { - capability: LOW_POWER, - flags: ConsumerFlags::none(), - }) - ) - ); - device0_signal.reset(); - assert_consumer_connected( service_receiver, device0, @@ -70,12 +51,24 @@ impl Test for TestUnconstrained { ) .await; + let mut device = device0.lock().await; + assert_eq!( + device.fn_calls.pop_front().unwrap(), + FnCall::ConnectConsumer(ConsumerPowerCapability { + capability: LOW_POWER, + flags: ConsumerFlags::none(), + }) + ); + assert!(device.fn_calls.is_empty()); + // Should not have any unconstrained events assert!(service_receiver.try_receive().is_err()); } { // Connect device1 with unconstrained at HIGH_POWER to force power policy to select this consumer. + device0.lock().await.next_result_disconnect.push_back(Ok(())); + device1.lock().await.next_result_connect_consumer.push_back(Ok(())); device1 .lock() .await @@ -85,24 +78,6 @@ impl Test for TestUnconstrained { }) .await; - assert_eq!( - with_timeout(PER_CALL_TIMEOUT, device0_signal.wait()).await.unwrap(), - (1, FnCall::Disconnect) - ); - device0_signal.reset(); - - assert_eq!( - with_timeout(PER_CALL_TIMEOUT, device1_signal.wait()).await.unwrap(), - ( - 1, - FnCall::ConnectConsumer(ConsumerPowerCapability { - capability: HIGH_POWER, - flags: ConsumerFlags::none().with_unconstrained_power(), - }) - ) - ); - device1_signal.reset(); - // Should receive a disconnect event from device0 first assert_consumer_disconnected(service_receiver, device0).await; @@ -124,30 +99,30 @@ impl Test for TestUnconstrained { }, ) .await; + + { + let mut device0 = device0.lock().await; + assert_eq!(device0.fn_calls.pop_front().unwrap(), FnCall::Disconnect); + assert!(device0.fn_calls.is_empty()); + } + { + let mut device1 = device1.lock().await; + assert_eq!( + device1.fn_calls.pop_front().unwrap(), + FnCall::ConnectConsumer(ConsumerPowerCapability { + capability: HIGH_POWER, + flags: ConsumerFlags::none().with_unconstrained_power(), + }) + ); + assert!(device1.fn_calls.is_empty()); + } } { // Test detach device1, unconstrained state should change + device0.lock().await.next_result_connect_consumer.push_back(Ok(())); device1.lock().await.simulate_detach().await; - // Power policy shouldn't call any functions on detach so we'll timeout - assert_eq!( - with_timeout(PER_CALL_TIMEOUT, device1_signal.wait()).await, - Err(TimeoutError) - ); - - assert_eq!( - with_timeout(PER_CALL_TIMEOUT, device0_signal.wait()).await.unwrap(), - ( - 1, - FnCall::ConnectConsumer(ConsumerPowerCapability { - capability: LOW_POWER, - flags: ConsumerFlags::none(), - }) - ) - ); - device0_signal.reset(); - // Should receive a disconnect event from device1 first assert_consumer_disconnected(service_receiver, device1).await; @@ -169,6 +144,19 @@ impl Test for TestUnconstrained { }, ) .await; + + // Power policy shouldn't call any functions on device1 for detach + assert!(device1.lock().await.fn_calls.is_empty()); + + let mut device0 = device0.lock().await; + assert_eq!( + device0.fn_calls.pop_front().unwrap(), + FnCall::ConnectConsumer(ConsumerPowerCapability { + capability: LOW_POWER, + flags: ConsumerFlags::none(), + }) + ); + assert!(device0.fn_calls.is_empty()); } assert_no_event(service_receiver);