From 9fced2363ba7ac2d42277fe04375fd910b1d0b0c Mon Sep 17 00:00:00 2001 From: Jerry Xie Date: Fri, 21 Aug 2026 16:26:05 -0500 Subject: [PATCH 1/3] test(thermal): Add behavioral coverage Exercise fan state transitions, manual overrides, sampling, and failure events through the service runner. Cover sensor retries, thresholds, hysteresis, sampling control, and buffer behavior. Assisted-by: GitHub Copilot:GPT 5.3 Codex --- Cargo.lock | 2 + thermal-service/Cargo.toml | 6 + thermal-service/src/utils.rs | 45 +++ thermal-service/tests/fan.rs | 483 ++++++++++++++++++++++++++++++++ thermal-service/tests/sensor.rs | 388 +++++++++++++++++++++++++ 5 files changed, 924 insertions(+) create mode 100644 thermal-service/tests/fan.rs create mode 100644 thermal-service/tests/sensor.rs diff --git a/Cargo.lock b/Cargo.lock index d801744ec..dd56ca124 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1922,6 +1922,7 @@ dependencies = [ name = "thermal-service" version = "0.1.0" dependencies = [ + "critical-section", "defmt 0.3.100", "embassy-futures", "embassy-sync", @@ -1933,6 +1934,7 @@ dependencies = [ "log", "odp-service-common", "thermal-service-interface", + "tokio", ] [[package]] diff --git a/thermal-service/Cargo.toml b/thermal-service/Cargo.toml index 01db4314f..c739c5ae3 100644 --- a/thermal-service/Cargo.toml +++ b/thermal-service/Cargo.toml @@ -23,6 +23,12 @@ thermal-service-interface.workspace = true embedded-fans-async = "0.2.0" embedded-sensors-hal-async = "0.3.0" +[dev-dependencies] +critical-section = { workspace = true, features = ["std"] } +embassy-sync = { workspace = true, features = ["std"] } +embassy-time = { workspace = true, features = ["std", "generic-queue-8"] } +tokio = { workspace = true, features = ["rt", "macros", "time"] } + [features] default = [] defmt = [ diff --git a/thermal-service/src/utils.rs b/thermal-service/src/utils.rs index f8598c130..4e377bbd4 100644 --- a/thermal-service/src/utils.rs +++ b/thermal-service/src/utils.rs @@ -46,3 +46,48 @@ impl SampleBuf { sum.checked_div(self.deque.len() as u32).unwrap_or(0) as u16 } } + +#[cfg(test)] +mod tests { + use super::SampleBuf; + + #[test] + fn empty_float_buffer_returns_default_values() { + let samples = SampleBuf::::create(); + + assert_eq!(samples.recent(), 0.0); + assert_eq!(samples.average(), 0.0); + } + + #[test] + fn float_buffer_reports_recent_and_average_samples() { + let mut samples = SampleBuf::::create(); + samples.push(2.0); + samples.push(4.0); + samples.push(6.0); + + assert_eq!(samples.recent(), 6.0); + assert_eq!(samples.average(), 4.0); + } + + #[test] + fn full_buffer_evicts_oldest_sample() { + let mut samples = SampleBuf::::create(); + samples.push(2); + samples.push(4); + samples.push(6); + samples.push(8); + + assert_eq!(samples.recent(), 8); + assert_eq!(samples.average(), 6); + } + + #[test] + fn integer_average_uses_wider_accumulator() { + let mut samples = SampleBuf::::create(); + samples.push(u16::MAX); + samples.push(u16::MAX); + + assert_eq!(samples.average(), u16::MAX); + } +} diff --git a/thermal-service/tests/fan.rs b/thermal-service/tests/fan.rs new file mode 100644 index 000000000..fb2d5f1a0 --- /dev/null +++ b/thermal-service/tests/fan.rs @@ -0,0 +1,483 @@ +#![allow(clippy::unwrap_used)] + +use std::{ + collections::VecDeque, + sync::{Arc, Mutex}, +}; + +use embassy_futures::select::{Either, select}; +use embassy_sync::channel::Channel; +use embassy_time::{Duration, with_timeout}; +use embedded_fans_async::{ErrorKind, ErrorType, Fan, RpmSense}; +use embedded_sensors_hal_async::temperature::DegreesCelsius; +use embedded_services::GlobalRawMutex; +use embedded_services::event::NoopSender; +use odp_service_common::runnable_service::ServiceRunner as _; +use thermal_service::fan::{Config, InitParams, Resources, Service}; +use thermal_service_interface::{ + fan::{self, FanService as _}, + sensor::{self, SensorService}, +}; + +#[derive(Clone, Copy, Debug)] +struct TestError; + +impl embedded_fans_async::Error for TestError { + fn kind(&self) -> ErrorKind { + ErrorKind::Other + } +} + +#[derive(Default)] +struct FanState { + rpm: u16, + fail_commands: bool, + requested_rpms: Vec, + rpm_readings: VecDeque, +} + +struct TestFan { + state: Arc>, +} + +impl ErrorType for TestFan { + type Error = TestError; +} + +impl Fan for TestFan { + fn min_rpm(&self) -> u16 { + 1_000 + } + + fn max_rpm(&self) -> u16 { + 6_000 + } + + fn min_start_rpm(&self) -> u16 { + 1_500 + } + + async fn set_speed_rpm(&mut self, rpm: u16) -> Result { + let mut state = self.state.lock().map_err(|_| TestError)?; + state.requested_rpms.push(rpm); + if state.fail_commands { + return Err(TestError); + } + state.rpm = rpm; + Ok(rpm) + } +} + +impl RpmSense for TestFan { + async fn rpm(&mut self) -> Result { + self.state + .lock() + .map(|mut state| { + if let Some(rpm) = state.rpm_readings.pop_front() { + state.rpm = rpm; + } + state.rpm + }) + .map_err(|_| TestError) + } +} + +impl fan::Driver for TestFan {} + +#[derive(Clone, Copy)] +struct FixedSensor(DegreesCelsius); + +impl SensorService for FixedSensor { + async fn temperature(&self) -> DegreesCelsius { + self.0 + } + + async fn temperature_average(&self) -> DegreesCelsius { + self.0 + } + + async fn temperature_immediate(&self) -> Result { + Ok(self.0) + } + + async fn set_threshold(&self, _threshold: sensor::Threshold, _value: DegreesCelsius) {} + + async fn threshold(&self, _threshold: sensor::Threshold) -> DegreesCelsius { + 0.0 + } + + async fn set_sample_period(&self, _period: Duration) {} + + async fn enable_sampling(&self) {} + + async fn disable_sampling(&self) {} +} + +#[derive(Clone)] +struct ScriptedSensor { + temperatures: Arc>>, + fallback: DegreesCelsius, +} + +impl SensorService for ScriptedSensor { + async fn temperature(&self) -> DegreesCelsius { + self.temperatures + .lock() + .map(|mut temperatures| temperatures.pop_front().unwrap_or(self.fallback)) + .unwrap_or(self.fallback) + } + + async fn temperature_average(&self) -> DegreesCelsius { + self.temperature().await + } + + async fn temperature_immediate(&self) -> Result { + Ok(self.temperature().await) + } + + async fn set_threshold(&self, _threshold: sensor::Threshold, _value: DegreesCelsius) {} + + async fn threshold(&self, _threshold: sensor::Threshold) -> DegreesCelsius { + 0.0 + } + + async fn set_sample_period(&self, _period: Duration) {} + + async fn enable_sampling(&self) {} + + async fn disable_sampling(&self) {} +} + +#[tokio::test] +async fn auto_control_transitions_from_min_through_ramping_to_max() { + let driver_state = Arc::new(Mutex::new(FanState::default())); + let driver = TestFan { + state: Arc::clone(&driver_state), + }; + let sensor = ScriptedSensor { + temperatures: Arc::new(Mutex::new(VecDeque::from([25.0, 35.0, 40.0, 45.0]))), + fallback: 45.0, + }; + let mut resources = Resources::::default(); + let mut event_senders: [NoopSender; 0] = []; + let (_service, runner) = Service::new( + &mut resources, + InitParams { + driver, + config: Config { + update_period: Duration::from_millis(1), + ..Default::default() + }, + sensor_service: sensor, + event_senders: &mut event_senders, + }, + ) + .await + .unwrap(); + + let assertion = async { + loop { + let requested_rpms = driver_state.lock().unwrap().requested_rpms.clone(); + if requested_rpms.len() >= 3 { + assert_eq!(requested_rpms, [1_500, 3_750, 6_000]); + break; + } + embassy_time::Timer::after_millis(1).await; + } + }; + + let result = with_timeout(Duration::from_millis(200), select(runner.run(), assertion)) + .await + .unwrap(); + match result { + Either::First(never) => match never {}, + Either::Second(()) => {} + } +} + +#[tokio::test] +async fn manual_rpm_disables_auto_control_until_reenabled() { + let driver_state = Arc::new(Mutex::new(FanState::default())); + let driver = TestFan { + state: Arc::clone(&driver_state), + }; + let mut resources = Resources::::default(); + let mut event_senders: [NoopSender; 0] = []; + let (service, runner) = Service::new( + &mut resources, + InitParams { + driver, + config: Config { + update_period: Duration::from_millis(1), + ..Default::default() + }, + sensor_service: FixedSensor(45.0), + event_senders: &mut event_senders, + }, + ) + .await + .unwrap(); + + service.set_rpm(3_250).await.unwrap(); + let assertion = async { + embassy_time::Timer::after_millis(5).await; + assert_eq!(driver_state.lock().unwrap().requested_rpms, [3_250]); + + service.enable_auto_control().await.unwrap(); + loop { + let requested_rpms = driver_state.lock().unwrap().requested_rpms.clone(); + if requested_rpms.len() >= 4 { + assert_eq!(requested_rpms, [3_250, 0, 1_500, 6_000]); + break; + } + embassy_time::Timer::after_millis(1).await; + } + }; + + let result = with_timeout(Duration::from_millis(200), select(runner.run(), assertion)) + .await + .unwrap(); + match result { + Either::First(never) => match never {}, + Either::Second(()) => {} + } +} + +#[tokio::test] +async fn auto_control_failure_emits_event_and_stops_retrying() { + let driver_state = Arc::new(Mutex::new(FanState { + fail_commands: true, + ..Default::default() + })); + let driver = TestFan { + state: Arc::clone(&driver_state), + }; + let event_channel = Channel::::new(); + let mut event_senders = [event_channel.sender()]; + let mut resources = Resources::::default(); + let (_service, runner) = Service::new( + &mut resources, + InitParams { + driver, + config: Config { + update_period: Duration::from_millis(1), + ..Default::default() + }, + sensor_service: FixedSensor(30.0), + event_senders: &mut event_senders, + }, + ) + .await + .unwrap(); + + let assertion = async { + assert_eq!(event_channel.receive().await, fan::Event::Failure(fan::Error::Hardware)); + embassy_time::Timer::after_millis(5).await; + assert_eq!(driver_state.lock().unwrap().requested_rpms, [1_500]); + }; + + let result = with_timeout(Duration::from_millis(200), select(runner.run(), assertion)) + .await + .unwrap(); + match result { + Either::First(never) => match never {}, + Either::Second(()) => {} + } +} + +#[tokio::test] +async fn runner_reports_recent_and_average_rpm_samples() { + let driver = TestFan { + state: Arc::new(Mutex::new(FanState { + rpm_readings: VecDeque::from([1_000, 2_000, 3_000]), + ..Default::default() + })), + }; + let mut resources = Resources::::default(); + let mut event_senders: [NoopSender; 0] = []; + let (service, runner) = Service::new( + &mut resources, + InitParams { + driver, + config: Config { + sample_period: Duration::from_millis(1), + auto_control: false, + ..Default::default() + }, + sensor_service: FixedSensor(30.0), + event_senders: &mut event_senders, + }, + ) + .await + .unwrap(); + + let assertion = async { + loop { + if service.rpm().await == 3_000 { + assert_eq!(service.rpm_average().await, 2_000); + break; + } + embassy_time::Timer::after_millis(1).await; + } + }; + + let result = with_timeout(Duration::from_millis(200), select(runner.run(), assertion)) + .await + .unwrap(); + match result { + Either::First(never) => match never {}, + Either::Second(()) => {} + } +} + +#[tokio::test] +async fn manual_duty_and_stop_forward_expected_rpm_commands() { + let driver_state = Arc::new(Mutex::new(FanState::default())); + let driver = TestFan { + state: Arc::clone(&driver_state), + }; + let mut resources = Resources::::default(); + let mut event_senders: [NoopSender; 0] = []; + let (service, _runner) = Service::new( + &mut resources, + InitParams { + driver, + config: Config::default(), + sensor_service: FixedSensor(30.0), + event_senders: &mut event_senders, + }, + ) + .await + .unwrap(); + + service.set_duty_percent(50).await.unwrap(); + service.stop().await.unwrap(); + + assert_eq!(driver_state.lock().unwrap().requested_rpms, [3_000, 0]); +} + +#[tokio::test] +async fn configured_state_temperatures_drive_auto_control() { + let driver_state = Arc::new(Mutex::new(FanState::default())); + let driver = TestFan { + state: Arc::clone(&driver_state), + }; + let mut resources = Resources::::default(); + let mut event_senders: [NoopSender; 0] = []; + let (service, runner) = Service::new( + &mut resources, + InitParams { + driver, + config: Config { + update_period: Duration::from_millis(1), + ..Default::default() + }, + sensor_service: FixedSensor(30.0), + event_senders: &mut event_senders, + }, + ) + .await + .unwrap(); + + service.set_state_temp(fan::OnState::Min, 20.0).await; + service.set_state_temp(fan::OnState::Ramping, 25.0).await; + service.set_state_temp(fan::OnState::Max, 30.0).await; + + let assertion = async { + loop { + let requested_rpms = driver_state.lock().unwrap().requested_rpms.clone(); + if requested_rpms.len() >= 2 { + assert_eq!(requested_rpms, [1_500, 6_000]); + break; + } + embassy_time::Timer::after_millis(1).await; + } + }; + + let result = with_timeout(Duration::from_millis(200), select(runner.run(), assertion)) + .await + .unwrap(); + match result { + Either::First(never) => match never {}, + Either::Second(()) => {} + } +} + +#[tokio::test] +async fn manual_rpm_round_trips_through_driver() { + let driver_state = Arc::new(Mutex::new(FanState::default())); + let driver = TestFan { + state: Arc::clone(&driver_state), + }; + let mut resources = Resources::::default(); + let mut event_senders: [NoopSender; 0] = []; + let (service, _runner) = Service::new( + &mut resources, + InitParams { + driver, + config: Config::default(), + sensor_service: FixedSensor(30.0), + event_senders: &mut event_senders, + }, + ) + .await + .unwrap(); + + assert_eq!(service.min_rpm().await, 1_000); + assert_eq!(service.max_rpm().await, 6_000); + assert_eq!(service.set_rpm(3_250).await, Ok(())); + assert_eq!(service.rpm_immediate().await, Ok(3_250)); +} + +#[tokio::test] +async fn manual_rpm_maps_driver_failure_to_hardware_error() { + let driver = TestFan { + state: Arc::new(Mutex::new(FanState { + fail_commands: true, + ..Default::default() + })), + }; + let mut resources = Resources::::default(); + let mut event_senders: [NoopSender; 0] = []; + let (service, _runner) = Service::new( + &mut resources, + InitParams { + driver, + config: Config::default(), + sensor_service: FixedSensor(30.0), + event_senders: &mut event_senders, + }, + ) + .await + .unwrap(); + + assert_eq!(service.set_rpm(3_250).await, Err(fan::Error::Hardware)); +} + +#[tokio::test] +async fn state_temperatures_can_be_configured_independently() { + let driver = TestFan { + state: Arc::new(Mutex::new(FanState::default())), + }; + let mut resources = Resources::::default(); + let mut event_senders: [NoopSender; 0] = []; + let (service, _runner) = Service::new( + &mut resources, + InitParams { + driver, + config: Config::default(), + sensor_service: FixedSensor(30.0), + event_senders: &mut event_senders, + }, + ) + .await + .unwrap(); + + service.set_state_temp(fan::OnState::Min, 20.0).await; + service.set_state_temp(fan::OnState::Ramping, 40.0).await; + service.set_state_temp(fan::OnState::Max, 60.0).await; + + assert_eq!(service.state_temp(fan::OnState::Min).await, 20.0); + assert_eq!(service.state_temp(fan::OnState::Ramping).await, 40.0); + assert_eq!(service.state_temp(fan::OnState::Max).await, 60.0); +} diff --git a/thermal-service/tests/sensor.rs b/thermal-service/tests/sensor.rs new file mode 100644 index 000000000..8c282baa8 --- /dev/null +++ b/thermal-service/tests/sensor.rs @@ -0,0 +1,388 @@ +#![allow(clippy::unwrap_used)] + +use std::collections::VecDeque; + +use embassy_futures::select::{Either, select}; +use embassy_sync::channel::Channel; +use embassy_time::{Duration, with_timeout}; +use embedded_sensors_hal_async::{ + sensor::{ErrorKind, ErrorType}, + temperature::{DegreesCelsius, TemperatureSensor}, +}; +use embedded_services::GlobalRawMutex; +use embedded_services::event::NoopSender; +use odp_service_common::runnable_service::ServiceRunner as _; +use thermal_service::sensor::{Config, InitParams, Resources, Service}; +use thermal_service_interface::sensor::{self, SensorService as _}; + +#[derive(Clone, Copy, Debug)] +struct TestError; + +impl embedded_sensors_hal_async::sensor::Error for TestError { + fn kind(&self) -> ErrorKind { + ErrorKind::Other + } +} + +struct ScriptedSensor { + readings: VecDeque>, +} + +impl ErrorType for ScriptedSensor { + type Error = TestError; +} + +impl TemperatureSensor for ScriptedSensor { + async fn temperature(&mut self) -> Result { + match self.readings.pop_front() { + Some(reading) => reading, + None => std::future::pending().await, + } + } +} + +impl sensor::Driver for ScriptedSensor {} + +#[tokio::test] +async fn immediate_temperature_retries_transient_failures() { + let driver = ScriptedSensor { + readings: VecDeque::from([Err(TestError), Err(TestError), Ok(42.5)]), + }; + let mut resources = Resources::::default(); + let mut event_senders: [NoopSender; 0] = []; + let (service, _runner) = Service::new( + &mut resources, + InitParams { + driver, + config: Config { + retry_attempts: 3, + ..Default::default() + }, + event_senders: &mut event_senders, + }, + ) + .await + .unwrap(); + + assert_eq!(service.temperature_immediate().await, Ok(42.5)); +} + +#[tokio::test] +async fn immediate_temperature_reports_retry_exhaustion() { + let driver = ScriptedSensor { + readings: VecDeque::from([Err(TestError), Err(TestError), Ok(51.0)]), + }; + let mut resources = Resources::::default(); + let mut event_senders: [NoopSender; 0] = []; + let (service, _runner) = Service::new( + &mut resources, + InitParams { + driver, + config: Config { + retry_attempts: 2, + ..Default::default() + }, + event_senders: &mut event_senders, + }, + ) + .await + .unwrap(); + + assert_eq!( + service.temperature_immediate().await, + Err(sensor::Error::RetryExhausted) + ); + assert_eq!(service.temperature_immediate().await, Ok(51.0)); +} + +#[tokio::test] +async fn runner_applies_offset_and_emits_high_threshold_event() { + let driver = ScriptedSensor { + readings: VecDeque::from([Ok(40.0)]), + }; + let event_channel = Channel::::new(); + let mut event_senders = [event_channel.sender()]; + let mut resources = Resources::::default(); + let (service, runner) = Service::new( + &mut resources, + InitParams { + driver, + config: Config { + sample_period: Duration::from_secs(1), + warn_high_threshold: 42.0, + offset: 2.0, + retry_attempts: 1, + ..Default::default() + }, + event_senders: &mut event_senders, + }, + ) + .await + .unwrap(); + + let assertion = async { + assert_eq!( + event_channel.receive().await, + sensor::Event::ThresholdExceeded(sensor::Threshold::WarnHigh) + ); + assert_eq!(service.temperature().await, 42.0); + assert_eq!(service.temperature_average().await, 42.0); + }; + + let result = with_timeout(Duration::from_millis(100), select(runner.run(), assertion)) + .await + .unwrap(); + match result { + Either::First(never) => match never {}, + Either::Second(()) => {} + } +} + +#[tokio::test] +async fn runner_emits_high_threshold_once_and_clears_after_hysteresis() { + let driver = ScriptedSensor { + readings: VecDeque::from([Ok(42.0), Ok(43.0), Ok(39.0)]), + }; + let event_channel = Channel::::new(); + let mut event_senders = [event_channel.sender()]; + let mut resources = Resources::::default(); + let (_service, runner) = Service::new( + &mut resources, + InitParams { + driver, + config: Config { + sample_period: Duration::from_millis(1), + warn_high_threshold: 42.0, + hysteresis: 2.0, + ..Default::default() + }, + event_senders: &mut event_senders, + }, + ) + .await + .unwrap(); + + let assertion = async { + assert_eq!( + event_channel.receive().await, + sensor::Event::ThresholdExceeded(sensor::Threshold::WarnHigh) + ); + assert_eq!( + event_channel.receive().await, + sensor::Event::ThresholdCleared(sensor::Threshold::WarnHigh) + ); + }; + + let result = with_timeout(Duration::from_millis(200), select(runner.run(), assertion)) + .await + .unwrap(); + match result { + Either::First(never) => match never {}, + Either::Second(()) => {} + } +} + +#[tokio::test] +async fn runner_applies_low_threshold_hysteresis() { + let driver = ScriptedSensor { + readings: VecDeque::from([Ok(10.0), Ok(9.0), Ok(13.0)]), + }; + let event_channel = Channel::::new(); + let mut event_senders = [event_channel.sender()]; + let mut resources = Resources::::default(); + let (_service, runner) = Service::new( + &mut resources, + InitParams { + driver, + config: Config { + sample_period: Duration::from_millis(1), + warn_low_threshold: 10.0, + hysteresis: 2.0, + ..Default::default() + }, + event_senders: &mut event_senders, + }, + ) + .await + .unwrap(); + + let assertion = async { + assert_eq!( + event_channel.receive().await, + sensor::Event::ThresholdExceeded(sensor::Threshold::WarnLow) + ); + assert_eq!( + event_channel.receive().await, + sensor::Event::ThresholdCleared(sensor::Threshold::WarnLow) + ); + }; + + let result = with_timeout(Duration::from_millis(200), select(runner.run(), assertion)) + .await + .unwrap(); + match result { + Either::First(never) => match never {}, + Either::Second(()) => {} + } +} + +#[tokio::test] +async fn runner_failure_disables_sampling_until_reenabled() { + let driver = ScriptedSensor { + readings: VecDeque::from([Err(TestError), Err(TestError), Ok(51.0)]), + }; + let event_channel = Channel::::new(); + let mut event_senders = [event_channel.sender()]; + let mut resources = Resources::::default(); + let (service, runner) = Service::new( + &mut resources, + InitParams { + driver, + config: Config { + sample_period: Duration::from_millis(1), + retry_attempts: 2, + ..Default::default() + }, + event_senders: &mut event_senders, + }, + ) + .await + .unwrap(); + + let assertion = async { + assert_eq!( + event_channel.receive().await, + sensor::Event::Failure(sensor::Error::RetryExhausted) + ); + embassy_time::Timer::after_millis(5).await; + assert_eq!(service.temperature().await, 0.0); + + service.enable_sampling().await; + loop { + if service.temperature().await == 51.0 { + break; + } + embassy_time::Timer::after_millis(1).await; + } + }; + + let result = with_timeout(Duration::from_millis(200), select(runner.run(), assertion)) + .await + .unwrap(); + match result { + Either::First(never) => match never {}, + Either::Second(()) => {} + } +} + +#[tokio::test] +async fn configured_prochot_and_critical_thresholds_emit_distinct_events() { + let driver = ScriptedSensor { + readings: VecDeque::from([Ok(50.0)]), + }; + let event_channel = Channel::::new(); + let mut event_senders = [event_channel.sender()]; + let mut resources = Resources::::default(); + let (service, runner) = Service::new( + &mut resources, + InitParams { + driver, + config: Config::default(), + event_senders: &mut event_senders, + }, + ) + .await + .unwrap(); + + service.set_threshold(sensor::Threshold::Prochot, 40.0).await; + service.set_threshold(sensor::Threshold::Critical, 45.0).await; + + let assertion = async { + assert_eq!( + event_channel.receive().await, + sensor::Event::ThresholdExceeded(sensor::Threshold::Prochot) + ); + assert_eq!( + event_channel.receive().await, + sensor::Event::ThresholdExceeded(sensor::Threshold::Critical) + ); + }; + + let result = with_timeout(Duration::from_millis(200), select(runner.run(), assertion)) + .await + .unwrap(); + match result { + Either::First(never) => match never {}, + Either::Second(()) => {} + } +} + +#[tokio::test] +async fn disabled_sampling_waits_until_explicitly_enabled() { + let driver = ScriptedSensor { + readings: VecDeque::from([Ok(27.0)]), + }; + let mut resources = Resources::::default(); + let mut event_senders: [NoopSender; 0] = []; + let (service, runner) = Service::new( + &mut resources, + InitParams { + driver, + config: Config::default(), + event_senders: &mut event_senders, + }, + ) + .await + .unwrap(); + + service.disable_sampling().await; + let assertion = async { + embassy_time::Timer::after_millis(5).await; + assert_eq!(service.temperature().await, 0.0); + + service.enable_sampling().await; + loop { + if service.temperature().await == 27.0 { + break; + } + embassy_time::Timer::after_millis(1).await; + } + }; + + let result = with_timeout(Duration::from_millis(200), select(runner.run(), assertion)) + .await + .unwrap(); + match result { + Either::First(never) => match never {}, + Either::Second(()) => {} + } +} + +#[tokio::test] +async fn immediate_temperature_retries_timed_out_bus_operation() { + let driver = ScriptedSensor { + readings: VecDeque::new(), + }; + let mut resources = Resources::::default(); + let mut event_senders: [NoopSender; 0] = []; + let (service, _runner) = Service::new( + &mut resources, + InitParams { + driver, + config: Config { + retry_attempts: 1, + ..Default::default() + }, + event_senders: &mut event_senders, + }, + ) + .await + .unwrap(); + + assert_eq!( + with_timeout(Duration::from_millis(500), service.temperature_immediate()) + .await + .unwrap(), + Err(sensor::Error::RetryExhausted) + ); +} From 0ef309ce201245f98b7e1e8bebb84fd604bfac96 Mon Sep 17 00:00:00 2001 From: Jerry Xie Date: Fri, 21 Aug 2026 16:53:55 -0500 Subject: [PATCH 2/3] test(thermal): Cover fan and sensor edge cases Extend the fan and sensor service test suites to close coverage gaps identified in review. Sensor: assert immediate reads ignore offset, exercise the threshold getter and sample-period setter, cover the fast-sampling branch, verify event fan-out to multiple senders, and check prochot/critical hysteresis clearing. Fan: cover the cool-down state transitions through hysteresis, the RPM sampling and update period setters, the rpm_immediate error path, and event fan-out to multiple senders. Add a scripted RPM-read failure hook to the fan test fake to enable the error path. Assisted-by: GitHub Copilot:claude-opus-4.8 --- thermal-service/tests/fan.rs | 230 ++++++++++++++++++++++++++++-- thermal-service/tests/sensor.rs | 242 ++++++++++++++++++++++++++++++++ 2 files changed, 463 insertions(+), 9 deletions(-) diff --git a/thermal-service/tests/fan.rs b/thermal-service/tests/fan.rs index fb2d5f1a0..7c772a75b 100644 --- a/thermal-service/tests/fan.rs +++ b/thermal-service/tests/fan.rs @@ -32,6 +32,7 @@ impl embedded_fans_async::Error for TestError { struct FanState { rpm: u16, fail_commands: bool, + fail_rpm_reads: bool, requested_rpms: Vec, rpm_readings: VecDeque, } @@ -70,15 +71,14 @@ impl Fan for TestFan { impl RpmSense for TestFan { async fn rpm(&mut self) -> Result { - self.state - .lock() - .map(|mut state| { - if let Some(rpm) = state.rpm_readings.pop_front() { - state.rpm = rpm; - } - state.rpm - }) - .map_err(|_| TestError) + let mut state = self.state.lock().map_err(|_| TestError)?; + if state.fail_rpm_reads { + return Err(TestError); + } + if let Some(rpm) = state.rpm_readings.pop_front() { + state.rpm = rpm; + } + Ok(state.rpm) } } @@ -481,3 +481,215 @@ async fn state_temperatures_can_be_configured_independently() { assert_eq!(service.state_temp(fan::OnState::Ramping).await, 40.0); assert_eq!(service.state_temp(fan::OnState::Max).await, 60.0); } + +#[tokio::test] +async fn auto_control_cools_down_through_hysteresis_to_off() { + let driver_state = Arc::new(Mutex::new(FanState::default())); + let driver = TestFan { + state: Arc::clone(&driver_state), + }; + // Ramp up to max, then descend past each state's hysteresis band back down to off. + let sensor = ScriptedSensor { + temperatures: Arc::new(Mutex::new(VecDeque::from([45.0, 45.0, 45.0, 30.0, 30.0, 20.0]))), + fallback: 20.0, + }; + let mut resources = Resources::::default(); + let mut event_senders: [NoopSender; 0] = []; + let (_service, runner) = Service::new( + &mut resources, + InitParams { + driver, + config: Config { + update_period: Duration::from_millis(1), + ..Default::default() + }, + sensor_service: sensor, + event_senders: &mut event_senders, + }, + ) + .await + .unwrap(); + + let assertion = async { + loop { + let requested_rpms = driver_state.lock().unwrap().requested_rpms.clone(); + if requested_rpms.len() >= 4 { + assert_eq!(requested_rpms, [1_500, 6_000, 1_500, 0]); + break; + } + embassy_time::Timer::after_millis(1).await; + } + }; + + let result = with_timeout(Duration::from_millis(200), select(runner.run(), assertion)) + .await + .unwrap(); + match result { + Either::First(never) => match never {}, + Either::Second(()) => {} + } +} + +#[tokio::test] +async fn set_rpm_update_period_takes_effect() { + let driver_state = Arc::new(Mutex::new(FanState::default())); + let driver = TestFan { + state: Arc::clone(&driver_state), + }; + let mut resources = Resources::::default(); + let mut event_senders: [NoopSender; 0] = []; + let (service, runner) = Service::new( + &mut resources, + InitParams { + driver, + // A ten second update period would let only the first transition through before the + // test times out; shortening it must let auto control reach the max state. + config: Config { + update_period: Duration::from_secs(10), + ..Default::default() + }, + sensor_service: FixedSensor(45.0), + event_senders: &mut event_senders, + }, + ) + .await + .unwrap(); + + service.set_rpm_update_period(Duration::from_millis(1)).await; + + let assertion = async { + loop { + let requested_rpms = driver_state.lock().unwrap().requested_rpms.clone(); + if requested_rpms.len() >= 2 { + assert_eq!(requested_rpms, [1_500, 6_000]); + break; + } + embassy_time::Timer::after_millis(1).await; + } + }; + + let result = with_timeout(Duration::from_millis(200), select(runner.run(), assertion)) + .await + .unwrap(); + match result { + Either::First(never) => match never {}, + Either::Second(()) => {} + } +} + +#[tokio::test] +async fn set_rpm_sampling_period_takes_effect() { + let driver = TestFan { + state: Arc::new(Mutex::new(FanState { + rpm_readings: VecDeque::from([1_000, 2_000, 3_000]), + ..Default::default() + })), + }; + let mut resources = Resources::::default(); + let mut event_senders: [NoopSender; 0] = []; + let (service, runner) = Service::new( + &mut resources, + InitParams { + driver, + // A ten second sampling period would let only the first reading through before the + // test times out; shortening it must let the runner reach the later readings. + config: Config { + sample_period: Duration::from_secs(10), + auto_control: false, + ..Default::default() + }, + sensor_service: FixedSensor(30.0), + event_senders: &mut event_senders, + }, + ) + .await + .unwrap(); + + service.set_rpm_sampling_period(Duration::from_millis(1)).await; + + let assertion = async { + loop { + if service.rpm().await == 3_000 { + break; + } + embassy_time::Timer::after_millis(1).await; + } + }; + + let result = with_timeout(Duration::from_millis(200), select(runner.run(), assertion)) + .await + .unwrap(); + match result { + Either::First(never) => match never {}, + Either::Second(()) => {} + } +} + +#[tokio::test] +async fn rpm_immediate_maps_driver_failure_to_hardware_error() { + let driver = TestFan { + state: Arc::new(Mutex::new(FanState { + fail_rpm_reads: true, + ..Default::default() + })), + }; + let mut resources = Resources::::default(); + let mut event_senders: [NoopSender; 0] = []; + let (service, _runner) = Service::new( + &mut resources, + InitParams { + driver, + config: Config::default(), + sensor_service: FixedSensor(30.0), + event_senders: &mut event_senders, + }, + ) + .await + .unwrap(); + + assert_eq!(service.rpm_immediate().await, Err(fan::Error::Hardware)); +} + +#[tokio::test] +async fn runner_broadcasts_events_to_all_senders() { + let driver = TestFan { + state: Arc::new(Mutex::new(FanState { + fail_commands: true, + ..Default::default() + })), + }; + let first_channel = Channel::::new(); + let second_channel = Channel::::new(); + let mut event_senders = [first_channel.sender(), second_channel.sender()]; + let mut resources = Resources::::default(); + let (_service, runner) = Service::new( + &mut resources, + InitParams { + driver, + config: Config { + update_period: Duration::from_millis(1), + ..Default::default() + }, + sensor_service: FixedSensor(30.0), + event_senders: &mut event_senders, + }, + ) + .await + .unwrap(); + + let assertion = async { + assert_eq!(first_channel.receive().await, fan::Event::Failure(fan::Error::Hardware)); + assert_eq!( + second_channel.receive().await, + fan::Event::Failure(fan::Error::Hardware) + ); + }; + + let result = with_timeout(Duration::from_millis(200), select(runner.run(), assertion)) + .await + .unwrap(); + match result { + Either::First(never) => match never {}, + Either::Second(()) => {} + } +} diff --git a/thermal-service/tests/sensor.rs b/thermal-service/tests/sensor.rs index 8c282baa8..07f502b25 100644 --- a/thermal-service/tests/sensor.rs +++ b/thermal-service/tests/sensor.rs @@ -386,3 +386,245 @@ async fn immediate_temperature_retries_timed_out_bus_operation() { Err(sensor::Error::RetryExhausted) ); } + +#[tokio::test] +async fn immediate_temperature_ignores_offset() { + let driver = ScriptedSensor { + readings: VecDeque::from([Ok(30.0)]), + }; + let mut resources = Resources::::default(); + let mut event_senders: [NoopSender; 0] = []; + let (service, _runner) = Service::new( + &mut resources, + InitParams { + driver, + config: Config { + offset: 5.0, + retry_attempts: 1, + ..Default::default() + }, + event_senders: &mut event_senders, + }, + ) + .await + .unwrap(); + + // Offset is only applied to periodically sampled readings, not immediate ones. + assert_eq!(service.temperature_immediate().await, Ok(30.0)); +} + +#[tokio::test] +async fn threshold_getter_returns_configured_values() { + let driver = ScriptedSensor { + readings: VecDeque::new(), + }; + let mut resources = Resources::::default(); + let mut event_senders: [NoopSender; 0] = []; + let (service, _runner) = Service::new( + &mut resources, + InitParams { + driver, + config: Config { + warn_low_threshold: 5.0, + warn_high_threshold: 60.0, + prochot_threshold: 80.0, + critical_threshold: 95.0, + ..Default::default() + }, + event_senders: &mut event_senders, + }, + ) + .await + .unwrap(); + + assert_eq!(service.threshold(sensor::Threshold::WarnLow).await, 5.0); + assert_eq!(service.threshold(sensor::Threshold::WarnHigh).await, 60.0); + assert_eq!(service.threshold(sensor::Threshold::Prochot).await, 80.0); + assert_eq!(service.threshold(sensor::Threshold::Critical).await, 95.0); + + service.set_threshold(sensor::Threshold::WarnHigh, 70.0).await; + assert_eq!(service.threshold(sensor::Threshold::WarnHigh).await, 70.0); +} + +#[tokio::test] +async fn set_sample_period_takes_effect() { + let driver = ScriptedSensor { + readings: VecDeque::from([Ok(20.0), Ok(21.0)]), + }; + let mut resources = Resources::::default(); + let mut event_senders: [NoopSender; 0] = []; + let (service, runner) = Service::new( + &mut resources, + InitParams { + driver, + // A one second sample period would let only the first reading through before the + // test times out; shortening it must let the runner reach the second reading. + config: Config { + sample_period: Duration::from_secs(1), + ..Default::default() + }, + event_senders: &mut event_senders, + }, + ) + .await + .unwrap(); + + service.set_sample_period(Duration::from_millis(1)).await; + + let assertion = async { + loop { + if service.temperature().await == 21.0 { + break; + } + embassy_time::Timer::after_millis(1).await; + } + }; + + let result = with_timeout(Duration::from_millis(200), select(runner.run(), assertion)) + .await + .unwrap(); + match result { + Either::First(never) => match never {}, + Either::Second(()) => {} + } +} + +#[tokio::test] +async fn fast_sampling_threshold_uses_fast_period() { + let driver = ScriptedSensor { + readings: VecDeque::from([Ok(80.0), Ok(81.0)]), + }; + let mut resources = Resources::::default(); + let mut event_senders: [NoopSender; 0] = []; + let (service, runner) = Service::new( + &mut resources, + InitParams { + driver, + // The slow period is long enough that only the fast period lets the runner reach + // the second reading before the test times out. + config: Config { + sample_period: Duration::from_secs(10), + fast_sample_period: Duration::from_millis(1), + fast_sampling_threshold: 50.0, + ..Default::default() + }, + event_senders: &mut event_senders, + }, + ) + .await + .unwrap(); + + let assertion = async { + loop { + if service.temperature().await == 81.0 { + break; + } + embassy_time::Timer::after_millis(1).await; + } + }; + + let result = with_timeout(Duration::from_millis(200), select(runner.run(), assertion)) + .await + .unwrap(); + match result { + Either::First(never) => match never {}, + Either::Second(()) => {} + } +} + +#[tokio::test] +async fn runner_broadcasts_events_to_all_senders() { + let driver = ScriptedSensor { + readings: VecDeque::from([Ok(50.0)]), + }; + let first_channel = Channel::::new(); + let second_channel = Channel::::new(); + let mut event_senders = [first_channel.sender(), second_channel.sender()]; + let mut resources = Resources::::default(); + let (_service, runner) = Service::new( + &mut resources, + InitParams { + driver, + config: Config { + sample_period: Duration::from_secs(1), + warn_high_threshold: 42.0, + ..Default::default() + }, + event_senders: &mut event_senders, + }, + ) + .await + .unwrap(); + + let assertion = async { + assert_eq!( + first_channel.receive().await, + sensor::Event::ThresholdExceeded(sensor::Threshold::WarnHigh) + ); + assert_eq!( + second_channel.receive().await, + sensor::Event::ThresholdExceeded(sensor::Threshold::WarnHigh) + ); + }; + + let result = with_timeout(Duration::from_millis(100), select(runner.run(), assertion)) + .await + .unwrap(); + match result { + Either::First(never) => match never {}, + Either::Second(()) => {} + } +} + +#[tokio::test] +async fn runner_clears_prochot_and_critical_after_hysteresis() { + let driver = ScriptedSensor { + readings: VecDeque::from([Ok(50.0), Ok(30.0)]), + }; + let event_channel = Channel::::new(); + let mut event_senders = [event_channel.sender()]; + let mut resources = Resources::::default(); + let (_service, runner) = Service::new( + &mut resources, + InitParams { + driver, + config: Config { + sample_period: Duration::from_millis(1), + prochot_threshold: 40.0, + critical_threshold: 45.0, + hysteresis: 2.0, + ..Default::default() + }, + event_senders: &mut event_senders, + }, + ) + .await + .unwrap(); + + let assertion = async { + assert_eq!( + event_channel.receive().await, + sensor::Event::ThresholdExceeded(sensor::Threshold::Prochot) + ); + assert_eq!( + event_channel.receive().await, + sensor::Event::ThresholdExceeded(sensor::Threshold::Critical) + ); + assert_eq!( + event_channel.receive().await, + sensor::Event::ThresholdCleared(sensor::Threshold::Prochot) + ); + assert_eq!( + event_channel.receive().await, + sensor::Event::ThresholdCleared(sensor::Threshold::Critical) + ); + }; + + let result = with_timeout(Duration::from_millis(200), select(runner.run(), assertion)) + .await + .unwrap(); + match result { + Either::First(never) => match never {}, + Either::Second(()) => {} + } +} From e4e2ad243b446491ca78e20ba7bda21b74a0292d Mon Sep 17 00:00:00 2001 From: Jerry Xie Date: Mon, 24 Aug 2026 12:12:03 -0500 Subject: [PATCH 3/3] fix(thermal): Apply offset to immediate reads Apply the configured sensor offset to immediate temperature reads. This keeps immediate values consistent with periodically sampled values. Update the regression test to cover the corrected result. Assisted-by: GitHub Copilot:claude-opus-4.6 --- thermal-service/src/sensor.rs | 3 ++- thermal-service/tests/sensor.rs | 5 ++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/thermal-service/src/sensor.rs b/thermal-service/src/sensor.rs index 2ccbc92a9..6d0365824 100644 --- a/thermal-service/src/sensor.rs +++ b/thermal-service/src/sensor.rs @@ -134,7 +134,8 @@ impl<'hw, T: sensor::Driver, E: NonBlockingSender, const SAMPLE_B } async fn temperature_immediate(&self) -> Result { - with_retry!(self.inner, self.inner.driver.lock().await.temperature()) + let temperature = with_retry!(self.inner, self.inner.driver.lock().await.temperature())?; + Ok(temperature + self.inner.config.lock().await.offset) } async fn set_threshold(&self, threshold: sensor::Threshold, value: DegreesCelsius) { diff --git a/thermal-service/tests/sensor.rs b/thermal-service/tests/sensor.rs index 07f502b25..492dd1d47 100644 --- a/thermal-service/tests/sensor.rs +++ b/thermal-service/tests/sensor.rs @@ -388,7 +388,7 @@ async fn immediate_temperature_retries_timed_out_bus_operation() { } #[tokio::test] -async fn immediate_temperature_ignores_offset() { +async fn immediate_temperature_applies_offset() { let driver = ScriptedSensor { readings: VecDeque::from([Ok(30.0)]), }; @@ -409,8 +409,7 @@ async fn immediate_temperature_ignores_offset() { .await .unwrap(); - // Offset is only applied to periodically sampled readings, not immediate ones. - assert_eq!(service.temperature_immediate().await, Ok(30.0)); + assert_eq!(service.temperature_immediate().await, Ok(35.0)); } #[tokio::test]