From 72a952ced70d41495df6790437806eb0699a3180 Mon Sep 17 00:00:00 2001 From: Lewis Lakerink Date: Sun, 22 Mar 2026 18:18:46 +1100 Subject: [PATCH 1/2] refactor: concurrent brightness and colour transitions - Replace monolithic old/current/new LightState triple with three independent Transition structs (brightness, CW, WW), each with their own timeline - Brightness and colour channels now transition concurrently instead of serially - Extract ease-in-out function as a free function (removed unused self receiver) - Remove overloaded timestamp field from LightState; semantics are now clear via explicit start/end Instants on each Transition - Add same-target guard in Transition::begin() so receiving the same state back mid-transition (e.g. controller echo on boot) no longer resets the timer, fixing a bug where startup transitions could take up to 2x longer than intended - light_onoff_hw() now only modifies the brightness channel, leaving colour transitions undisturbed - get_current_light_state() returns by value, ending the borrow of app() earlier - Expand test coverage to 100% lines/functions on the light management module, covering all flash retrieve paths, recover-status branches, and channel independence invariants - Fix unused-mut warning in unrelated test helper - Apply rustfmt --- rust/src/light_manager.rs | 912 +++++++++++++++++++++++++++++--------- rust/src/version.rs | 2 +- sdk/version.in | 2 +- 3 files changed, 697 insertions(+), 219 deletions(-) diff --git a/rust/src/light_manager.rs b/rust/src/light_manager.rs index 4542168..12a8e07 100644 --- a/rust/src/light_manager.rs +++ b/rust/src/light_manager.rs @@ -40,7 +40,6 @@ pub struct LightState { pub cw: I16F16, pub ww: I16F16, pub brightness: I16F16, - timestamp: Instant, } #[derive(Copy, Clone)] @@ -59,7 +58,6 @@ impl LightState { cw: I16F16::lit(formatcp!("{}", MAX_LUM_BRIGHTNESS_VALUE)), ww: I16F16::lit(formatcp!("{}", 0u16)), brightness: I16F16::lit(formatcp!("{}", 0u16)), - timestamp: Instant::from_ticks(0), } } @@ -69,23 +67,87 @@ impl LightState { cw: I16F16::lit(formatcp!("{}", MAX_LUM_BRIGHTNESS_VALUE)), ww: I16F16::lit(formatcp!("{}", 0u16)), brightness: I16F16::lit(formatcp!("{}", 0u16)), - timestamp: Instant::from_ticks(0), } } } +fn ease_in_out(t: I16F16, b: I16F16, c: I16F16) -> I16F16 { + static TWO: I16F16 = I16F16::lit(formatcp!("{}", 2u16)); + static D: I16F16 = I16F16::lit(formatcp!("{}", MAX_LUM_BRIGHTNESS_VALUE)); + + let t = t / (D / TWO); + if t < 1 { + c / TWO * (t * t * t) + b + } else { + let t = t - TWO; + c / TWO * (t * t * t + TWO) + b + } +} + +#[derive(Copy, Clone, Debug, PartialEq)] +struct Transition { + from: I16F16, + current: I16F16, + to: I16F16, + start: Instant, + end: Instant, +} + +impl Transition { + const fn new(initial: I16F16) -> Self { + Self { + from: initial, + current: initial, + to: initial, + start: Instant::from_ticks(0), + end: Instant::from_ticks(0), + } + } + + fn begin(&mut self, to: I16F16) { + // Same-target guard: if already heading to this target, don't restart the transition. + if to == self.to { + return; + } + self.from = self.current; + self.start = Instant::now(); + self.end = self.start + Duration::from_millis(TRANSITION_TIME_MS); + self.to = to; + } + + fn step(&mut self) -> bool { + let now = Instant::now(); + if now >= self.end { + self.current = self.to; + return false; + } + let elapsed = (now - self.start).as_ticks(); + let total = (self.end - self.start).as_ticks(); + let t = I16F16::from_num(elapsed * MAX_LUM_BRIGHTNESS_VALUE as u64 / total); + self.current = ease_in_out(t, self.from, self.to - self.from); + true + } + + /// Returns true if a transition was set up (end > start), regardless of + /// whether it has completed. After completion current == to, so callers + /// that use this to choose between `to` and `current` get the same value. + fn is_active(&self) -> bool { + self.end > self.start + } +} + #[cfg_attr(test, mry::mry)] pub struct LightManager { channel: Deque, - old_light_state: LightState, - new_light_state: LightState, - current_light_state: LightState, + brightness_tr: Transition, + cw_tr: Transition, + ww_tr: Transition, light_lum_addr: u32, last_transition_time: u32, - // This brightness is separate to the current light state brightness since it stores the brightness if the light is off + // Stores the brightness when the light is off, to restore when turned back on brightness: u16, } @@ -95,9 +157,9 @@ impl LightManager { pub const fn default_const() -> Self { Self { channel: Deque::new(), - old_light_state: LightState::default_const(), - new_light_state: LightState::default_const(), - current_light_state: LightState::default_const(), + brightness_tr: Transition::new(I16F16::lit(formatcp!("{}", 0u16))), + cw_tr: Transition::new(I16F16::lit(formatcp!("{}", MAX_LUM_BRIGHTNESS_VALUE))), + ww_tr: Transition::new(I16F16::lit(formatcp!("{}", 0u16))), light_lum_addr: 0, last_transition_time: 0, brightness: MAX_LUM_BRIGHTNESS_VALUE, @@ -108,9 +170,9 @@ impl LightManager { pub fn default() -> Self { Self { channel: Deque::new(), - old_light_state: LightState::default(), - new_light_state: LightState::default(), - current_light_state: LightState::default(), + brightness_tr: Transition::new(I16F16::from_num(0u16)), + cw_tr: Transition::new(I16F16::from_num(MAX_LUM_BRIGHTNESS_VALUE)), + ww_tr: Transition::new(I16F16::from_num(0u16)), light_lum_addr: 0, last_transition_time: 0, brightness: MAX_LUM_BRIGHTNESS_VALUE, @@ -128,8 +190,8 @@ impl LightManager { fn handle_transition(&mut self, params: &[u8; 16]) { let mut brightness = self.brightness; - let mut cw = self.current_light_state.cw.to_num(); - let mut ww = self.current_light_state.ww.to_num(); + let mut cw = self.cw_tr.current.to_num(); + let mut ww = self.ww_tr.current.to_num(); if params[8] & 0x1 != 0 { // Brightness @@ -195,20 +257,14 @@ impl LightManager { pub fn begin_transition(&mut self, cw: u16, ww: u16, brightness: u16) { critical_section::with(|_| { - // Check if the cw or ww are changing and update the transition time so we save in a bit - if self.current_light_state.cw != cw || self.current_light_state.ww != ww { + // Check if the cw or ww targets are changing and mark for save + if self.cw_tr.to != cw || self.ww_tr.to != ww { self.last_transition_time = clock_time(); } - self.old_light_state = self.current_light_state; - self.old_light_state.timestamp = Instant::now(); - - self.new_light_state = LightState { - cw: I16F16::from_num(cw), - ww: I16F16::from_num(ww), - brightness: I16F16::from_num(brightness), - timestamp: Instant::now() + Duration::from_millis(TRANSITION_TIME_MS), - }; + self.brightness_tr.begin(I16F16::from_num(brightness)); + self.cw_tr.begin(I16F16::from_num(cw)); + self.ww_tr.begin(I16F16::from_num(ww)); // Enable timer1 write_reg_tmr1_tick(0); @@ -219,78 +275,28 @@ impl LightManager { }); } - pub fn ease_in_out(&self, t: I16F16, b: I16F16, c: I16F16) -> I16F16 { - static TWO: I16F16 = I16F16::lit(formatcp!("{}", 2u16)); - static D: I16F16 = I16F16::lit(formatcp!("{}", MAX_LUM_BRIGHTNESS_VALUE)); - - let t = t / (D / TWO); - if t < 1 { - c / TWO * (t * t * t) + b - } else { - let t = t - TWO; - c / TWO * (t * t * t + TWO) + b - } - } - pub fn transition_step(&mut self) { - self.current_light_state.timestamp = min(Instant::now(), self.new_light_state.timestamp); + let b = self.brightness_tr.step(); + let c = self.cw_tr.step(); + let w = self.ww_tr.step(); - if self.current_light_state.timestamp == self.new_light_state.timestamp { - // Disable timer1 + if !b && !c && !w { write_reg_tmr_ctrl(read_reg_tmr_ctrl() & !FLD_TMR::TMR1_EN.bits()); - - // Save a computation - self.current_light_state.cw = self.new_light_state.cw; - self.current_light_state.ww = self.new_light_state.ww; - self.current_light_state.brightness = self.new_light_state.brightness; - - // Make sure we do a final light update - self.light_adjust_rgb_hw( - self.current_light_state.cw, - self.current_light_state.ww, - self.current_light_state.brightness, - ); - - // Nothing more to do - return; } - // We're still transitioning. Run the calculations - let time = I16F16::from_num( - (self.current_light_state.timestamp - self.old_light_state.timestamp).as_ticks() - * MAX_LUM_BRIGHTNESS_VALUE as u64 - / (self.new_light_state.timestamp - self.old_light_state.timestamp).as_ticks(), - ); - self.current_light_state.cw = self.ease_in_out( - time, - self.old_light_state.cw, - self.new_light_state.cw - self.old_light_state.cw, - ); - - self.current_light_state.ww = self.ease_in_out( - time, - self.old_light_state.ww, - self.new_light_state.ww - self.old_light_state.ww, - ); - - self.current_light_state.brightness = self.ease_in_out( - time, - self.old_light_state.brightness, - self.new_light_state.brightness - self.old_light_state.brightness, - ); - self.light_adjust_rgb_hw( - self.current_light_state.cw, - self.current_light_state.ww, - self.current_light_state.brightness, + self.cw_tr.current, + self.ww_tr.current, + self.brightness_tr.current, ); } pub fn is_light_off(&self) -> bool { - if self.new_light_state.timestamp > self.current_light_state.timestamp { - return self.new_light_state.brightness == 0; + if self.brightness_tr.is_active() { + self.brightness_tr.to == 0 + } else { + self.brightness_tr.current == 0 } - self.current_light_state.brightness == 0 } //erase flash @@ -310,8 +316,8 @@ impl LightManager { let lum_save = LumSaveT { save_flag: LIGHT_SAVE_VALID_FLAG, brightness: self.brightness, - cw: self.current_light_state.cw.to_num(), - ww: self.current_light_state.ww.to_num(), + cw: self.cw_tr.current.to_num(), + ww: self.ww_tr.current.to_num(), }; flash_write_page( @@ -348,10 +354,16 @@ impl LightManager { LIGHT_SAVE_VALID_FLAG => { // Found valid saved state - update current values self.brightness = min(entry.brightness, MAX_LUM_BRIGHTNESS_VALUE); - self.current_light_state.cw = - I16F16::from_num(min(entry.cw, MAX_LUM_BRIGHTNESS_VALUE)); - self.current_light_state.ww = - I16F16::from_num(min(entry.ww, MAX_LUM_BRIGHTNESS_VALUE)); + let cw = I16F16::from_num(min(entry.cw, MAX_LUM_BRIGHTNESS_VALUE)); + let ww = I16F16::from_num(min(entry.ww, MAX_LUM_BRIGHTNESS_VALUE)); + // Set from/current/to all to saved value so colour starts + // immediately at the saved colour with no fade on boot. + self.cw_tr.from = cw; + self.cw_tr.current = cw; + self.cw_tr.to = cw; + self.ww_tr.from = ww; + self.ww_tr.current = ww; + self.ww_tr.to = ww; self.light_lum_addr = addr + (idx * size_of::()) as u32; } 0xFF => { @@ -395,12 +407,24 @@ impl LightManager { } } - pub fn get_current_light_state(&mut self) -> &mut LightState { - if self.new_light_state.timestamp > self.current_light_state.timestamp { - return &mut self.new_light_state; + pub fn get_current_light_state(&self) -> LightState { + LightState { + cw: if self.cw_tr.is_active() { + self.cw_tr.to + } else { + self.cw_tr.current + }, + ww: if self.ww_tr.is_active() { + self.ww_tr.to + } else { + self.ww_tr.current + }, + brightness: if self.brightness_tr.is_active() { + self.brightness_tr.to + } else { + self.brightness_tr.current + }, } - - &mut self.current_light_state } pub fn calculate_lumen_map(&self, val: I16F16) -> u32 { @@ -437,15 +461,13 @@ impl LightManager { } pub fn light_onoff_hw(&mut self, on: bool) { - let state = self.current_light_state; - self.begin_transition( - state.cw.to_num(), - state.ww.to_num(), - match on { - true => self.brightness, - false => 0, - }, - ); + let target = if on { self.brightness } else { 0 }; + critical_section::with(|_| { + self.brightness_tr.begin(I16F16::from_num(target)); + write_reg_tmr1_tick(0); + write_reg_tmr_ctrl(read_reg_tmr_ctrl() | FLD_TMR::TMR1_EN.bits()); + self.transition_step(); + }); } pub fn light_onoff(&mut self, on: bool) { @@ -593,12 +615,12 @@ mod tests { assert_eq!(manager.brightness, MAX_LUM_BRIGHTNESS_VALUE); assert_eq!(manager.light_lum_addr, 0); assert_eq!(manager.last_transition_time, 0); - assert_eq!(manager.current_light_state.brightness.to_num::(), 0); + assert_eq!(manager.brightness_tr.current.to_num::(), 0); assert_eq!( - manager.current_light_state.cw.to_num::(), + manager.cw_tr.current.to_num::(), MAX_LUM_BRIGHTNESS_VALUE ); - assert_eq!(manager.current_light_state.ww.to_num::(), 0); + assert_eq!(manager.ww_tr.current.to_num::(), 0); } #[test] @@ -610,7 +632,6 @@ mod tests { assert_eq!(state.cw.to_num::(), MAX_LUM_BRIGHTNESS_VALUE); assert_eq!(state.ww.to_num::(), 0); assert_eq!(state.brightness.to_num::(), 0); - assert_eq!(state.timestamp.as_ticks(), 0); } // --- Message Sending Tests --- @@ -674,7 +695,6 @@ mod tests { let mut manager = create_test_light_manager(); manager.brightness = 500; - manager.current_light_state.brightness = I16F16::from_num(0); mock_read_reg_tmr_ctrl().returns(0x00); mock_write_reg_tmr1_tick(0).returns(()); @@ -688,7 +708,7 @@ mod tests { manager.handle_on_off(LIGHT_ON_PARAM); // New state should target the saved brightness - assert_eq!(manager.new_light_state.brightness.to_num::(), 500); + assert_eq!(manager.brightness_tr.to.to_num::(), 500); } #[test] @@ -704,7 +724,8 @@ mod tests { // This test verifies that handle_on_off correctly processes LIGHT_OFF_PARAM let mut manager = create_test_light_manager(); - manager.current_light_state.brightness = I16F16::from_num(1000); + manager.brightness_tr.current = I16F16::from_num(1000); + manager.brightness_tr.to = I16F16::from_num(1000); mock_read_reg_tmr_ctrl().returns(0x00); mock_write_reg_tmr1_tick(0).returns(()); @@ -718,7 +739,7 @@ mod tests { manager.handle_on_off(LIGHT_OFF_PARAM); // New state should target brightness 0 - assert_eq!(manager.new_light_state.brightness.to_num::(), 0); + assert_eq!(manager.brightness_tr.to.to_num::(), 0); } #[test] @@ -747,7 +768,7 @@ mod tests { manager.handle_on_off(0x99); // Invalid value - assert_eq!(manager.new_light_state.brightness.to_num::(), 300); + assert_eq!(manager.brightness_tr.to.to_num::(), 300); } // --- Transition Handling Tests --- @@ -766,7 +787,8 @@ mod tests { let mut manager = create_test_light_manager(); manager.brightness = 100; - manager.current_light_state.brightness = I16F16::from_num(100); + manager.brightness_tr.current = I16F16::from_num(100); + manager.brightness_tr.to = I16F16::from_num(100); let params = [ 0x34, 0x12, // brightness = 0x1234 0, 0, // temperature @@ -788,7 +810,7 @@ mod tests { manager.handle_transition(¶ms); assert_eq!(manager.brightness, 0x1234); - assert_eq!(manager.new_light_state.brightness.to_num::(), 0x1234); + assert_eq!(manager.brightness_tr.to.to_num::(), 0x1234); assert_eq!(manager.last_transition_time, 1000); } @@ -805,7 +827,8 @@ mod tests { // This test verifies that handle_transition correctly processes temperature changes let mut manager = create_test_light_manager(); - manager.current_light_state.brightness = I16F16::from_num(500); + manager.brightness_tr.current = I16F16::from_num(500); + manager.brightness_tr.to = I16F16::from_num(500); let params = [ 0, 0, // brightness 0x78, 0x56, // temperature = 0x5678 @@ -828,10 +851,10 @@ mod tests { // CW should be MAX - temperature, WW should be temperature assert_eq!( - manager.new_light_state.cw.to_num::(), + manager.cw_tr.to.to_num::(), MAX_LUM_BRIGHTNESS_VALUE - 0x5678 ); - assert_eq!(manager.new_light_state.ww.to_num::(), 0x5678); + assert_eq!(manager.ww_tr.to.to_num::(), 0x5678); } #[test] @@ -847,7 +870,8 @@ mod tests { // This test verifies that handle_transition correctly processes independent CW/WW let mut manager = create_test_light_manager(); - manager.current_light_state.brightness = I16F16::from_num(500); + manager.brightness_tr.current = I16F16::from_num(500); + manager.brightness_tr.to = I16F16::from_num(500); let params = [ 0, 0, // brightness 0x34, 0x12, // cw = 0x1234 @@ -868,8 +892,8 @@ mod tests { manager.handle_transition(¶ms); - assert_eq!(manager.new_light_state.cw.to_num::(), 0x1234); - assert_eq!(manager.new_light_state.ww.to_num::(), 0x5678); + assert_eq!(manager.cw_tr.to.to_num::(), 0x1234); + assert_eq!(manager.ww_tr.to.to_num::(), 0x5678); } #[test] @@ -886,8 +910,7 @@ mod tests { let mut manager = create_test_light_manager(); manager.brightness = 1000; - manager.current_light_state.brightness = I16F16::from_num(0); - manager.new_light_state.brightness = I16F16::from_num(0); + // brightness_tr defaults to current=0, to=0 — light is off let params = [ 0x34, 0x12, // brightness 0x78, 0x56, // temperature @@ -908,7 +931,7 @@ mod tests { manager.handle_transition(¶ms); // Brightness should be set to 0 even though 0x1234 was requested - assert_eq!(manager.new_light_state.brightness.to_num::(), 0); + assert_eq!(manager.brightness_tr.to.to_num::(), 0); } // --- Easing Function Tests --- @@ -917,12 +940,11 @@ mod tests { fn test_ease_in_out_at_start() { // This test verifies ease_in_out returns initial value at t=0 - let manager = create_test_light_manager(); let t = I16F16::from_num(0); let b = I16F16::from_num(100); // Start value let c = I16F16::from_num(900); // Change amount - let result = manager.ease_in_out(t, b, c); + let result = ease_in_out(t, b, c); assert_eq!(result.to_num::(), 100); } @@ -931,12 +953,11 @@ mod tests { fn test_ease_in_out_at_end() { // This test verifies ease_in_out returns final value at t=MAX - let manager = create_test_light_manager(); let t = I16F16::from_num(MAX_LUM_BRIGHTNESS_VALUE); let b = I16F16::from_num(100); let c = I16F16::from_num(900); - let result = manager.ease_in_out(t, b, c); + let result = ease_in_out(t, b, c); assert_eq!(result.to_num::(), 1000); } @@ -945,12 +966,11 @@ mod tests { fn test_ease_in_out_at_midpoint() { // This test verifies ease_in_out produces smooth transition at midpoint - let manager = create_test_light_manager(); let t = I16F16::from_num(MAX_LUM_BRIGHTNESS_VALUE / 2); let b = I16F16::from_num(0); let c = I16F16::from_num(1000); - let result = manager.ease_in_out(t, b, c); + let result = ease_in_out(t, b, c); // At midpoint, should be roughly in the middle let result_val = result.to_num::(); @@ -961,12 +981,11 @@ mod tests { fn test_ease_in_out_negative_change() { // This test verifies ease_in_out works with negative change (decreasing) - let manager = create_test_light_manager(); let t = I16F16::from_num(MAX_LUM_BRIGHTNESS_VALUE); let b = I16F16::from_num(1000); let c = I16F16::from_num(-500); - let result = manager.ease_in_out(t, b, c); + let result = ease_in_out(t, b, c); assert_eq!(result.to_num::(), 500); } @@ -979,12 +998,27 @@ mod tests { // This test verifies transition_step finalizes state when target time is reached let mut manager = create_test_light_manager(); - manager.old_light_state.timestamp = Instant::from_ticks(0); - manager.new_light_state.timestamp = Instant::from_ticks(100); - manager.new_light_state.brightness = I16F16::from_num(500); - manager.new_light_state.cw = I16F16::from_num(300); - manager.new_light_state.ww = I16F16::from_num(200); - manager.current_light_state.timestamp = Instant::from_ticks(100); + manager.brightness_tr = Transition { + from: I16F16::from_num(0), + current: I16F16::from_num(0), + to: I16F16::from_num(500), + start: Instant::from_ticks(0), + end: Instant::from_ticks(100), + }; + manager.cw_tr = Transition { + from: I16F16::from_num(0), + current: I16F16::from_num(0), + to: I16F16::from_num(300), + start: Instant::from_ticks(0), + end: Instant::from_ticks(100), + }; + manager.ww_tr = Transition { + from: I16F16::from_num(0), + current: I16F16::from_num(0), + to: I16F16::from_num(200), + start: Instant::from_ticks(0), + end: Instant::from_ticks(100), + }; mock_clock_time64().returns(100); mock_read_reg_tmr_ctrl().returns(0x10); @@ -995,9 +1029,9 @@ mod tests { manager.transition_step(); // Should match new state exactly - assert_eq!(manager.current_light_state.brightness.to_num::(), 500); - assert_eq!(manager.current_light_state.cw.to_num::(), 300); - assert_eq!(manager.current_light_state.ww.to_num::(), 200); + assert_eq!(manager.brightness_tr.current.to_num::(), 500); + assert_eq!(manager.cw_tr.current.to_num::(), 300); + assert_eq!(manager.ww_tr.current.to_num::(), 200); // Timer should be disabled mock_write_reg_tmr_ctrl(0x10).assert_called(1); } @@ -1008,29 +1042,37 @@ mod tests { // This test verifies transition_step correctly interpolates values mid-transition let mut manager = create_test_light_manager(); - manager.old_light_state.timestamp = Instant::from_ticks(0); - manager.old_light_state.brightness = I16F16::from_num(0); - manager.old_light_state.cw = I16F16::from_num(0); - manager.old_light_state.ww = I16F16::from_num(0); - - manager.new_light_state.timestamp = Instant::from_ticks(100); - manager.new_light_state.brightness = I16F16::from_num(1000); - manager.new_light_state.cw = I16F16::from_num(1000); - manager.new_light_state.ww = I16F16::from_num(1000); - - manager.current_light_state.timestamp = Instant::from_ticks(50); - - // During transition at 50% progress, PWM values are interpolated with ease_in_out - // At 50% progress: brightness=500, cw=500, ww=500 -> get_pwm_cmp returns 3 - // But pwm_set_lum does (3 * 10455) / 65280 = 0, so final CW PWM=0 - mock_pwm_set_cmp(0, 0).returns(()); // CW channel (interpolated at 50%) - mock_pwm_set_cmp(1, 10455).returns(()); // WW channel (interpolated at 50%) + manager.brightness_tr = Transition { + from: I16F16::from_num(0), + current: I16F16::from_num(0), + to: I16F16::from_num(1000), + start: Instant::from_ticks(0), + end: Instant::from_ticks(100), + }; + manager.cw_tr = Transition { + from: I16F16::from_num(0), + current: I16F16::from_num(0), + to: I16F16::from_num(1000), + start: Instant::from_ticks(0), + end: Instant::from_ticks(100), + }; + manager.ww_tr = Transition { + from: I16F16::from_num(0), + current: I16F16::from_num(0), + to: I16F16::from_num(1000), + start: Instant::from_ticks(0), + end: Instant::from_ticks(100), + }; + + // At 50% progress + mock_pwm_set_cmp(0, 0).returns(()); // CW channel + mock_pwm_set_cmp(1, 10455).returns(()); // WW channel mock_clock_time64().returns(50); manager.transition_step(); // Should be partway through transition, not at extremes - let brightness = manager.current_light_state.brightness.to_num::(); + let brightness = manager.brightness_tr.current.to_num::(); assert!(brightness > 100 && brightness < 900); } @@ -1044,10 +1086,14 @@ mod tests { mock_clock_time64().returns(1000); let mut manager = create_test_light_manager(); - manager.current_light_state.brightness = I16F16::from_num(0); - manager.new_light_state.brightness = I16F16::from_num(500); - manager.new_light_state.timestamp = Instant::from_ticks(100); - manager.current_light_state.timestamp = Instant::from_ticks(0); + // Active transition from 0 toward 500: is_active() = true, to = 500 + manager.brightness_tr = Transition { + from: I16F16::from_num(0), + current: I16F16::from_num(0), + to: I16F16::from_num(500), + start: Instant::from_ticks(0), + end: Instant::from_ticks(100), + }; assert!(!manager.is_light_off()); } @@ -1057,10 +1103,14 @@ mod tests { // This test verifies is_light_off returns true when new brightness is 0 let mut manager = create_test_light_manager(); - manager.current_light_state.brightness = I16F16::from_num(500); - manager.new_light_state.brightness = I16F16::from_num(0); - manager.new_light_state.timestamp = Instant::from_ticks(100); - manager.current_light_state.timestamp = Instant::from_ticks(0); + // Active transition from 500 toward 0: is_active() = true, to = 0 + manager.brightness_tr = Transition { + from: I16F16::from_num(500), + current: I16F16::from_num(500), + to: I16F16::from_num(0), + start: Instant::from_ticks(0), + end: Instant::from_ticks(100), + }; assert!(manager.is_light_off()); } @@ -1070,8 +1120,9 @@ mod tests { // This test verifies is_light_off returns false when brightness is non-zero let mut manager = create_test_light_manager(); - manager.current_light_state.brightness = I16F16::from_num(500); - manager.new_light_state.brightness = I16F16::from_num(500); + // No active transition (start == end == 0), current = 500 + manager.brightness_tr.current = I16F16::from_num(500); + manager.brightness_tr.to = I16F16::from_num(500); assert!(!manager.is_light_off()); } @@ -1080,13 +1131,17 @@ mod tests { #[test] fn test_get_current_light_state_returns_new_when_newer() { - // This test verifies get_current_light_state returns new state when it has newer timestamp + // This test verifies get_current_light_state returns target when transition is active let mut manager = create_test_light_manager(); - manager.current_light_state.timestamp = Instant::from_ticks(100); - manager.current_light_state.brightness = I16F16::from_num(500); - manager.new_light_state.timestamp = Instant::from_ticks(200); - manager.new_light_state.brightness = I16F16::from_num(1000); + // Active transition (end > start): report to + manager.brightness_tr = Transition { + from: I16F16::from_num(0), + current: I16F16::from_num(500), + to: I16F16::from_num(1000), + start: Instant::from_ticks(100), + end: Instant::from_ticks(200), + }; let state = manager.get_current_light_state(); @@ -1095,13 +1150,17 @@ mod tests { #[test] fn test_get_current_light_state_returns_current_when_equal_timestamp() { - // This test verifies get_current_light_state returns current state when timestamps are equal + // This test verifies get_current_light_state returns current when no transition is active let mut manager = create_test_light_manager(); - manager.current_light_state.timestamp = Instant::from_ticks(100); - manager.current_light_state.brightness = I16F16::from_num(500); - manager.new_light_state.timestamp = Instant::from_ticks(100); - manager.new_light_state.brightness = I16F16::from_num(1000); + // No active transition (start == end): report current + manager.brightness_tr = Transition { + from: I16F16::from_num(0), + current: I16F16::from_num(500), + to: I16F16::from_num(1000), + start: Instant::from_ticks(100), + end: Instant::from_ticks(100), // end == start → not active + }; let state = manager.get_current_light_state(); @@ -1110,13 +1169,12 @@ mod tests { #[test] fn test_get_current_light_state_returns_current_when_older() { - // This test verifies get_current_light_state returns current state when new state is older + // This test verifies get_current_light_state returns current when idle (no transition set up) let mut manager = create_test_light_manager(); - manager.current_light_state.timestamp = Instant::from_ticks(200); - manager.current_light_state.brightness = I16F16::from_num(500); - manager.new_light_state.timestamp = Instant::from_ticks(100); - manager.new_light_state.brightness = I16F16::from_num(1000); + // Default idle state (start == end == 0): report current + manager.brightness_tr.current = I16F16::from_num(500); + manager.brightness_tr.to = I16F16::from_num(1000); let state = manager.get_current_light_state(); @@ -1149,8 +1207,8 @@ mod tests { let mut manager = create_test_light_manager(); manager.light_lum_addr = FLASH_ADR_LUM; manager.brightness = 500; - manager.current_light_state.cw = I16F16::from_num(300); - manager.current_light_state.ww = I16F16::from_num(200); + manager.cw_tr.current = I16F16::from_num(300); + manager.ww_tr.current = I16F16::from_num(200); mock_flash_write_page(FLASH_ADR_LUM, size_of::() as u32, Any).returns(()); @@ -1250,8 +1308,8 @@ mod tests { // Verify the state was restored assert_eq!(manager.brightness, saved_brightness); - assert_eq!(manager.current_light_state.cw.to_num::(), saved_cw); - assert_eq!(manager.current_light_state.ww.to_num::(), saved_ww); + assert_eq!(manager.cw_tr.current.to_num::(), saved_cw); + assert_eq!(manager.ww_tr.current.to_num::(), saved_ww); // The light_onoff should have been called mock_ll_device_status_update([1, 0xff]).assert_called(1); @@ -1428,7 +1486,7 @@ mod tests { // This test verifies device_status_update reports correct status when light is on let mut manager = create_test_light_manager(); - manager.current_light_state.brightness = I16F16::from_num(500); + manager.brightness_tr.current = I16F16::from_num(500); mock_ll_device_status_update([1, 0xff]).returns(()); // Status 0 = light on @@ -1442,9 +1500,8 @@ mod tests { fn test_device_status_update_reports_off() { // This test verifies device_status_update reports correct status when light is off - let mut manager = create_test_light_manager(); - manager.current_light_state.brightness = I16F16::from_num(0); - manager.new_light_state.brightness = I16F16::from_num(0); + let manager = create_test_light_manager(); + // brightness_tr defaults to current=0, to=0 — light is already off mock_ll_device_status_update([0, 0xff]).returns(()); // Status 0 = light off @@ -1468,9 +1525,12 @@ mod tests { // This test verifies begin_transition correctly sets up new transition let mut manager = create_test_light_manager(); - manager.current_light_state.brightness = I16F16::from_num(100); - manager.current_light_state.cw = I16F16::from_num(100); - manager.current_light_state.ww = I16F16::from_num(100); + manager.brightness_tr.current = I16F16::from_num(100); + manager.brightness_tr.to = I16F16::from_num(100); + manager.cw_tr.current = I16F16::from_num(100); + manager.cw_tr.to = I16F16::from_num(100); + manager.ww_tr.current = I16F16::from_num(100); + manager.ww_tr.to = I16F16::from_num(100); mock_clock_time().returns(5000); mock_read_reg_tmr_ctrl().returns(0x00); @@ -1482,10 +1542,10 @@ mod tests { manager.begin_transition(500, 300, 800); - assert_eq!(manager.new_light_state.cw.to_num::(), 500); - assert_eq!(manager.new_light_state.ww.to_num::(), 300); - assert_eq!(manager.new_light_state.brightness.to_num::(), 800); - assert_eq!(manager.old_light_state.brightness.to_num::(), 100); + assert_eq!(manager.cw_tr.to.to_num::(), 500); + assert_eq!(manager.ww_tr.to.to_num::(), 300); + assert_eq!(manager.brightness_tr.to.to_num::(), 800); + assert_eq!(manager.brightness_tr.from.to_num::(), 100); assert_eq!(manager.last_transition_time, 5000); } @@ -1502,8 +1562,10 @@ mod tests { // This test verifies begin_transition doesn't update time if only brightness changes let mut manager = create_test_light_manager(); - manager.current_light_state.cw = I16F16::from_num(500); - manager.current_light_state.ww = I16F16::from_num(300); + manager.cw_tr.current = I16F16::from_num(500); + manager.cw_tr.to = I16F16::from_num(500); + manager.ww_tr.current = I16F16::from_num(300); + manager.ww_tr.to = I16F16::from_num(300); manager.last_transition_time = 0; mock_clock_time().returns(5000); @@ -1549,7 +1611,7 @@ mod tests { manager.send_message(msg.cmd, msg.params); block_on(manager.process_message_impl()); - assert_eq!(manager.new_light_state.brightness.to_num::(), 500); + assert_eq!(manager.brightness_tr.to.to_num::(), 500); } #[test] @@ -1565,7 +1627,8 @@ mod tests { // This test verifies process_message correctly handles transition messages let mut manager = create_test_light_manager(); - manager.current_light_state.brightness = I16F16::from_num(500); + manager.brightness_tr.current = I16F16::from_num(500); + manager.brightness_tr.to = I16F16::from_num(500); mock_read_reg_tmr_ctrl().returns(0x00); mock_write_reg_tmr1_tick(0).returns(()); @@ -1582,7 +1645,7 @@ mod tests { block_on(manager.process_message_impl()); assert_eq!(manager.brightness, 1000); - assert_eq!(manager.new_light_state.brightness.to_num::(), 1000); + assert_eq!(manager.brightness_tr.to.to_num::(), 1000); } #[test] @@ -1621,7 +1684,7 @@ mod tests { let mut manager = create_test_light_manager(); manager.brightness = 1000; - manager.current_light_state.brightness = I16F16::from_num(0); + // brightness_tr defaults to current=0 (light is off) mock_read_reg_tmr_ctrl().returns(0x00); mock_write_reg_tmr1_tick(0).returns(()); @@ -1635,11 +1698,11 @@ mod tests { // Turn light on - should set up a transition manager.light_onoff(true); - assert_eq!(manager.new_light_state.brightness.to_num::(), 1000); - assert_eq!(manager.current_light_state.brightness.to_num::(), 0); + assert_eq!(manager.brightness_tr.to.to_num::(), 1000); + assert_eq!(manager.brightness_tr.current.to_num::(), 0); - // New state timestamp should be in the future - assert!(manager.new_light_state.timestamp > manager.old_light_state.timestamp); + // Transition should be active (end > start) + assert!(manager.brightness_tr.end > manager.brightness_tr.start); } #[test] @@ -1655,9 +1718,12 @@ mod tests { // This test verifies adjusting color temperature while light is on let mut manager = create_test_light_manager(); - manager.current_light_state.brightness = I16F16::from_num(1000); - manager.current_light_state.cw = I16F16::from_num(MAX_LUM_BRIGHTNESS_VALUE); - manager.current_light_state.ww = I16F16::from_num(0); + manager.brightness_tr.current = I16F16::from_num(1000); + manager.brightness_tr.to = I16F16::from_num(1000); + manager.cw_tr.current = I16F16::from_num(MAX_LUM_BRIGHTNESS_VALUE); + manager.cw_tr.to = I16F16::from_num(MAX_LUM_BRIGHTNESS_VALUE); + manager.ww_tr.current = I16F16::from_num(0); + manager.ww_tr.to = I16F16::from_num(0); mock_read_reg_tmr_ctrl().returns(0x00); mock_write_reg_tmr1_tick(0).returns(()); @@ -1672,11 +1738,423 @@ mod tests { let params = create_temperature_message(MAX_LUM_BRIGHTNESS_VALUE - 100); manager.handle_transition(¶ms.params); - assert_eq!(manager.new_light_state.cw.to_num::(), 100); + assert_eq!(manager.cw_tr.to.to_num::(), 100); assert_eq!( - manager.new_light_state.ww.to_num::(), + manager.ww_tr.to.to_num::(), MAX_LUM_BRIGHTNESS_VALUE - 100 ); assert_eq!(manager.last_transition_time, 1000); } + + // --- Get Current Light State (Active CW/WW) Tests --- + + #[test] + fn test_get_current_light_state_cw_active_returns_to() { + // cw_tr.is_active() == true path: must return cw_tr.to, not cw_tr.current + let mut manager = create_test_light_manager(); + manager.cw_tr = Transition { + from: I16F16::from_num(MAX_LUM_BRIGHTNESS_VALUE), + current: I16F16::from_num(30000), + to: I16F16::from_num(100), + start: Instant::from_ticks(1), + end: Instant::from_ticks(200), + }; + + let state = manager.get_current_light_state(); + + assert_eq!(state.cw.to_num::(), 100); + } + + #[test] + fn test_get_current_light_state_ww_active_returns_to() { + // ww_tr.is_active() == true path: must return ww_tr.to, not ww_tr.current + // Use 30000 — within I16F16 signed integer range (max 32767) + let mut manager = create_test_light_manager(); + manager.ww_tr = Transition { + from: I16F16::from_num(0), + current: I16F16::from_num(15000), + to: I16F16::from_num(30000), + start: Instant::from_ticks(1), + end: Instant::from_ticks(200), + }; + + let state = manager.get_current_light_state(); + + assert_eq!(state.ww.to_num::(), 30000); + } + + // --- light_lum_retrieve: light-was-off + invalid-entry + full-sector-scan --- + + #[test] + #[mry::lock( + flash_read_page, + analog_read, + analog_write, + read_reg_tmr_ctrl, + write_reg_tmr1_tick, + write_reg_tmr_ctrl, + ll_device_status_update, + pwm_set_cmp, + clock_time64 + )] + fn test_light_lum_retrieve_light_was_off_turns_light_off() { + // When the LightOff bit is set in analog memory, light_onoff(false) is called. + let mut manager = create_test_light_manager(); + + let saved_brightness = 800u16; + let saved_cw = 600u16; + let saved_ww = 400u16; + + mock_flash_read_page(Any, Any, Any).returns_with( + move |_addr: u32, len: u32, buf: SendWrapper<*mut u8>| { + let mut buffer = vec![0xFFu8; len as usize]; + if len as usize >= size_of::() { + buffer[0] = LIGHT_SAVE_VALID_FLAG; + buffer[1] = (saved_brightness & 0xFF) as u8; + buffer[2] = ((saved_brightness >> 8) & 0xFF) as u8; + buffer[3] = (saved_cw & 0xFF) as u8; + buffer[4] = ((saved_cw >> 8) & 0xFF) as u8; + buffer[5] = (saved_ww & 0xFF) as u8; + buffer[6] = ((saved_ww >> 8) & 0xFF) as u8; + // 0xFF after the first entry → triggers early return + if len as usize >= size_of::() * 2 { + buffer[size_of::()] = 0xFF; + } + } + unsafe { core::ptr::copy_nonoverlapping(buffer.as_ptr(), *buf, len as usize) }; + }, + ); + + // Light WAS off at prior shutdown + let light_off_bit = RecoverStatus::LightOff as u8; + mock_analog_read(REGA_LIGHT_OFF).returns(light_off_bit); + mock_analog_write(REGA_LIGHT_OFF, 0x00).returns(()); // clears the bit + + // light_onoff(false) → light_onoff_hw(false) → timer + pwm + status + // Note: same-target guard fires (brightness_tr starts at 0, target is 0), + // so transition_step immediately disables the timer (write_reg_tmr_ctrl(0x00)). + mock_read_reg_tmr_ctrl().returns(0x00); + mock_write_reg_tmr1_tick(0).returns(()); + mock_write_reg_tmr_ctrl(Any).returns(()); + mock_ll_device_status_update([0, 0xff]).returns(()); + mock_pwm_set_cmp(0, 0).returns(()); + mock_pwm_set_cmp(1, 10455).returns(()); + mock_clock_time64().returns(1000); + + manager.light_lum_retrieve(); + + assert_eq!(manager.brightness, saved_brightness); + // brightness_tr targets 0 (light off) + assert_eq!(manager.brightness_tr.to.to_num::(), 0); + mock_analog_write(REGA_LIGHT_OFF, 0x00).assert_called(1); + mock_ll_device_status_update([0, 0xff]).assert_called(1); + } + + #[test] + #[mry::lock( + flash_read_page, + analog_read, + analog_write, + read_reg_tmr_ctrl, + write_reg_tmr1_tick, + write_reg_tmr_ctrl, + ll_device_status_update, + pwm_set_cmp, + clock_time64 + )] + fn test_light_lum_retrieve_skips_invalid_flash_entry() { + // An entry with a save_flag that is neither 0xA5 nor 0xFF is silently skipped. + // The scan continues until a 0xFF entry is found. + let mut manager = create_test_light_manager(); + + let saved_cw = 1234u16; + let saved_ww = 5678u16; + let saved_brightness = 999u16; + + mock_flash_read_page(Any, Any, Any).returns_with( + move |_addr: u32, len: u32, buf: SendWrapper<*mut u8>| { + let mut buffer = vec![0xFFu8; len as usize]; + let entry_size = size_of::(); + // First entry: invalid flag byte (0x42) + if len as usize >= entry_size { + buffer[0] = 0x42; // neither 0xA5 nor 0xFF + buffer[1] = 0; + buffer[2] = 0; + buffer[3] = 0; + buffer[4] = 0; + buffer[5] = 0; + buffer[6] = 0; + } + // Second entry: valid + if len as usize >= entry_size * 2 { + buffer[entry_size] = LIGHT_SAVE_VALID_FLAG; + buffer[entry_size + 1] = (saved_brightness & 0xFF) as u8; + buffer[entry_size + 2] = ((saved_brightness >> 8) & 0xFF) as u8; + buffer[entry_size + 3] = (saved_cw & 0xFF) as u8; + buffer[entry_size + 4] = ((saved_cw >> 8) & 0xFF) as u8; + buffer[entry_size + 5] = (saved_ww & 0xFF) as u8; + buffer[entry_size + 6] = ((saved_ww >> 8) & 0xFF) as u8; + } + // Third entry: 0xFF → terminates scan + // (buffer is already 0xFF-initialized, so this is implicit) + unsafe { core::ptr::copy_nonoverlapping(buffer.as_ptr(), *buf, len as usize) }; + }, + ); + + mock_analog_read(REGA_LIGHT_OFF).returns(0x00); + mock_analog_write(REGA_LIGHT_OFF, 0x00).returns(()); + mock_read_reg_tmr_ctrl().returns(0x00); + mock_write_reg_tmr1_tick(0).returns(()); + mock_write_reg_tmr_ctrl(0x08).returns(()); + mock_ll_device_status_update([1, 0xff]).returns(()); + mock_pwm_set_cmp(0, 0).returns(()); + mock_pwm_set_cmp(1, 10455).returns(()); + mock_clock_time64().returns(1000); + + manager.light_lum_retrieve(); + + // The valid entry (index 1) was loaded correctly despite the invalid one before it + assert_eq!(manager.brightness, saved_brightness); + assert_eq!(manager.cw_tr.current.to_num::(), saved_cw); + assert_eq!(manager.ww_tr.current.to_num::(), saved_ww); + } + + #[test] + #[mry::lock( + flash_read_page, + analog_read, + analog_write, + read_reg_tmr_ctrl, + write_reg_tmr1_tick, + write_reg_tmr_ctrl, + ll_device_status_update, + pwm_set_cmp, + clock_time64 + )] + fn test_light_lum_retrieve_full_sector_uses_post_loop_restore() { + // When the entire flash sector is full of valid entries (no 0xFF terminator), + // the post-loop code restores the light state from analog memory. + let mut manager = create_test_light_manager(); + + let saved_brightness = 300u16; + let saved_cw = 200u16; + let saved_ww = 100u16; + + // Return only valid entries — no 0xFF anywhere + mock_flash_read_page(Any, Any, Any).returns_with( + move |_addr: u32, len: u32, buf: SendWrapper<*mut u8>| { + let entry_size = size_of::(); + let num_entries = len as usize / entry_size; + let mut buffer = vec![0u8; len as usize]; + for i in 0..num_entries { + let off = i * entry_size; + buffer[off] = LIGHT_SAVE_VALID_FLAG; + buffer[off + 1] = (saved_brightness & 0xFF) as u8; + buffer[off + 2] = ((saved_brightness >> 8) & 0xFF) as u8; + buffer[off + 3] = (saved_cw & 0xFF) as u8; + buffer[off + 4] = ((saved_cw >> 8) & 0xFF) as u8; + buffer[off + 5] = (saved_ww & 0xFF) as u8; + buffer[off + 6] = ((saved_ww >> 8) & 0xFF) as u8; + } + unsafe { core::ptr::copy_nonoverlapping(buffer.as_ptr(), *buf, len as usize) }; + }, + ); + + // Light was on at prior shutdown + mock_analog_read(REGA_LIGHT_OFF).returns(0x00); + mock_analog_write(REGA_LIGHT_OFF, 0x00).returns(()); + mock_read_reg_tmr_ctrl().returns(0x00); + mock_write_reg_tmr1_tick(0).returns(()); + mock_write_reg_tmr_ctrl(0x08).returns(()); + mock_ll_device_status_update([1, 0xff]).returns(()); + mock_pwm_set_cmp(0, 0).returns(()); + mock_pwm_set_cmp(1, 10455).returns(()); + mock_clock_time64().returns(1000); + + manager.light_lum_retrieve(); + + // State from last valid entry in the full sector was restored + assert_eq!(manager.brightness, saved_brightness); + assert_eq!(manager.cw_tr.current.to_num::(), saved_cw); + assert_eq!(manager.ww_tr.current.to_num::(), saved_ww); + mock_ll_device_status_update([1, 0xff]).assert_called(1); + } + + #[test] + #[mry::lock( + flash_read_page, + analog_read, + analog_write, + read_reg_tmr_ctrl, + write_reg_tmr1_tick, + write_reg_tmr_ctrl, + ll_device_status_update, + pwm_set_cmp, + clock_time64 + )] + fn test_light_lum_retrieve_full_sector_light_was_off() { + // Full sector, no 0xFF terminator, AND the light was off at prior shutdown. + // Exercises the post-loop RecoverStatus::LightOff == true branch. + let mut manager = create_test_light_manager(); + + let saved_brightness = 300u16; + let saved_cw = 200u16; + let saved_ww = 100u16; + + mock_flash_read_page(Any, Any, Any).returns_with( + move |_addr: u32, len: u32, buf: SendWrapper<*mut u8>| { + let entry_size = size_of::(); + let num_entries = len as usize / entry_size; + let mut buffer = vec![0u8; len as usize]; + for i in 0..num_entries { + let off = i * entry_size; + buffer[off] = LIGHT_SAVE_VALID_FLAG; + buffer[off + 1] = (saved_brightness & 0xFF) as u8; + buffer[off + 2] = ((saved_brightness >> 8) & 0xFF) as u8; + buffer[off + 3] = (saved_cw & 0xFF) as u8; + buffer[off + 4] = ((saved_cw >> 8) & 0xFF) as u8; + buffer[off + 5] = (saved_ww & 0xFF) as u8; + buffer[off + 6] = ((saved_ww >> 8) & 0xFF) as u8; + } + unsafe { core::ptr::copy_nonoverlapping(buffer.as_ptr(), *buf, len as usize) }; + }, + ); + + // Light WAS off at prior shutdown + let light_off_bit = RecoverStatus::LightOff as u8; + mock_analog_read(REGA_LIGHT_OFF).returns(light_off_bit); + mock_analog_write(REGA_LIGHT_OFF, 0x00).returns(()); + mock_read_reg_tmr_ctrl().returns(0x00); + mock_write_reg_tmr1_tick(0).returns(()); + mock_write_reg_tmr_ctrl(Any).returns(()); + mock_ll_device_status_update([0, 0xff]).returns(()); + mock_pwm_set_cmp(0, 0).returns(()); + mock_pwm_set_cmp(1, 10455).returns(()); + mock_clock_time64().returns(1000); + + manager.light_lum_retrieve(); + + assert_eq!(manager.brightness, saved_brightness); + assert_eq!(manager.brightness_tr.to.to_num::(), 0); + mock_analog_write(REGA_LIGHT_OFF, 0x00).assert_called(1); + mock_ll_device_status_update([0, 0xff]).assert_called(1); + } + + // --- Independent Channel / Same-Target Guard Tests --- + + #[test] + #[mry::lock( + read_reg_tmr_ctrl, + write_reg_tmr1_tick, + write_reg_tmr_ctrl, + pwm_set_cmp, + clock_time64, + clock_time + )] + fn test_same_target_guard_does_not_reset_transition() { + // Verifies that begin_transition with the same brightness target mid-transition + // does not reset the transition end time (same-target guard fires). + + let mut manager = create_test_light_manager(); + + // Set up an in-progress brightness transition to 500. + // Use a far-future end so Instant::now() won't exceed it during the test. + manager.brightness_tr.to = I16F16::from_num(500); + manager.brightness_tr.from = I16F16::from_num(0); + manager.brightness_tr.current = I16F16::from_num(250); + manager.brightness_tr.start = Instant::from_ticks(0); + manager.brightness_tr.end = Instant::from_ticks(100_000_000); + + // cw/ww already at defaults (to = MAX and 0), pass the same so guard fires for them too + mock_clock_time64().returns(50); + mock_clock_time().returns(0); + mock_read_reg_tmr_ctrl().returns(0x00); + mock_write_reg_tmr1_tick(0).returns(()); + mock_write_reg_tmr_ctrl(Any).returns(()); + mock_pwm_set_cmp(Any, Any).returns(()); + + // begin_transition with same brightness target — guard should fire, end unchanged + manager.begin_transition(MAX_LUM_BRIGHTNESS_VALUE, 0, 500); + + assert_eq!(manager.brightness_tr.end, Instant::from_ticks(100_000_000)); + } + + #[test] + #[mry::lock( + read_reg_tmr_ctrl, + write_reg_tmr1_tick, + write_reg_tmr_ctrl, + pwm_set_cmp, + clock_time64 + )] + fn test_onoff_does_not_reset_colour_transition() { + // Verifies that light_onoff_hw only starts a brightness transition and + // does not disturb an in-progress colour transition. + + let mut manager = create_test_light_manager(); + + // Set up an in-progress colour transition + manager.cw_tr.to = I16F16::from_num(100); + manager.cw_tr.from = I16F16::from_num(MAX_LUM_BRIGHTNESS_VALUE); + manager.cw_tr.current = I16F16::from_num(30000); + manager.cw_tr.start = Instant::from_ticks(0); + manager.cw_tr.end = Instant::from_ticks(100_000_000); + + manager.brightness = 500; + + mock_clock_time64().returns(50); + mock_read_reg_tmr_ctrl().returns(0x00); + mock_write_reg_tmr1_tick(0).returns(()); + mock_write_reg_tmr_ctrl(Any).returns(()); + mock_pwm_set_cmp(Any, Any).returns(()); + + // Turn light on — only brightness_tr should change + manager.light_onoff_hw(true); + + // cw_tr.end must be unchanged + assert_eq!(manager.cw_tr.end, Instant::from_ticks(100_000_000)); + // brightness_tr should now target 500 + assert_eq!(manager.brightness_tr.to, I16F16::from_num(500)); + } + + #[test] + #[mry::lock( + read_reg_tmr_ctrl, + write_reg_tmr1_tick, + write_reg_tmr_ctrl, + pwm_set_cmp, + clock_time64, + clock_time + )] + fn test_colour_command_does_not_reset_brightness_transition() { + // Verifies that a colour-only change via begin_transition does not reset + // an in-progress brightness transition (same-target guard fires for brightness). + + let mut manager = create_test_light_manager(); + + // In-progress brightness transition to 1000 + manager.brightness_tr.to = I16F16::from_num(1000); + manager.brightness_tr.from = I16F16::from_num(0); + manager.brightness_tr.current = I16F16::from_num(500); + manager.brightness_tr.start = Instant::from_ticks(0); + manager.brightness_tr.end = Instant::from_ticks(100_000_000); + + manager.brightness = 1000; + + mock_clock_time64().returns(50); + mock_clock_time().returns(0); + mock_read_reg_tmr_ctrl().returns(0x00); + mock_write_reg_tmr1_tick(0).returns(()); + mock_write_reg_tmr_ctrl(Any).returns(()); + mock_pwm_set_cmp(Any, Any).returns(()); + + // New colour, same brightness — guard fires for brightness + manager.begin_transition(100, 500, 1000); + + // brightness_tr.end unchanged (guard fired) + assert_eq!(manager.brightness_tr.end, Instant::from_ticks(100_000_000)); + // Colour was updated + assert_eq!(manager.cw_tr.to, I16F16::from_num(100)); + assert_eq!(manager.ww_tr.to, I16F16::from_num(500)); + } } diff --git a/rust/src/version.rs b/rust/src/version.rs index 49b20f9..7a7a9f4 100644 --- a/rust/src/version.rs +++ b/rust/src/version.rs @@ -1 +1 @@ -pub static BUILD_VERSION: u32 = 3537; +pub static BUILD_VERSION: u32 = 3538; diff --git a/sdk/version.in b/sdk/version.in index 5fec22f..194fcd9 100644 --- a/sdk/version.in +++ b/sdk/version.in @@ -1,2 +1,2 @@ -.equ BUILD_VERSION,3537 +.equ BUILD_VERSION,3538 .equ XTAL_16MHZ,0 From efb76663d7fb35721349892e5d1c94cd4646f9d4 Mon Sep 17 00:00:00 2001 From: Lewis Lakerink Date: Mon, 23 Mar 2026 13:11:47 +1100 Subject: [PATCH 2/2] refactor: capture Instant::now() once per transition call site - Rename begin() to begin_at(start, to) and step() to step_at(now) so callers own the timestamp - begin_transition() captures a single Instant before calling begin_at on all three channels, ensuring brightness/CW/WW share an identical start and end time rather than diverging by the cost of repeated now() calls - transition_step() captures now once and passes it to each step_at, removing per-channel skew on every timer tick and reducing syscall overhead inside the IRQ handler --- rust/src/light_manager.rs | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/rust/src/light_manager.rs b/rust/src/light_manager.rs index 12a8e07..dfc0a3b 100644 --- a/rust/src/light_manager.rs +++ b/rust/src/light_manager.rs @@ -104,19 +104,18 @@ impl Transition { } } - fn begin(&mut self, to: I16F16) { + fn begin_at(&mut self, start: Instant, to: I16F16) { // Same-target guard: if already heading to this target, don't restart the transition. if to == self.to { return; } self.from = self.current; - self.start = Instant::now(); - self.end = self.start + Duration::from_millis(TRANSITION_TIME_MS); + self.start = start; + self.end = start + Duration::from_millis(TRANSITION_TIME_MS); self.to = to; } - fn step(&mut self) -> bool { - let now = Instant::now(); + fn step_at(&mut self, now: Instant) -> bool { if now >= self.end { self.current = self.to; return false; @@ -262,9 +261,11 @@ impl LightManager { self.last_transition_time = clock_time(); } - self.brightness_tr.begin(I16F16::from_num(brightness)); - self.cw_tr.begin(I16F16::from_num(cw)); - self.ww_tr.begin(I16F16::from_num(ww)); + let now = Instant::now(); + self.brightness_tr + .begin_at(now, I16F16::from_num(brightness)); + self.cw_tr.begin_at(now, I16F16::from_num(cw)); + self.ww_tr.begin_at(now, I16F16::from_num(ww)); // Enable timer1 write_reg_tmr1_tick(0); @@ -276,9 +277,10 @@ impl LightManager { } pub fn transition_step(&mut self) { - let b = self.brightness_tr.step(); - let c = self.cw_tr.step(); - let w = self.ww_tr.step(); + let now = Instant::now(); + let b = self.brightness_tr.step_at(now); + let c = self.cw_tr.step_at(now); + let w = self.ww_tr.step_at(now); if !b && !c && !w { write_reg_tmr_ctrl(read_reg_tmr_ctrl() & !FLD_TMR::TMR1_EN.bits()); @@ -463,7 +465,8 @@ impl LightManager { pub fn light_onoff_hw(&mut self, on: bool) { let target = if on { self.brightness } else { 0 }; critical_section::with(|_| { - self.brightness_tr.begin(I16F16::from_num(target)); + self.brightness_tr + .begin_at(Instant::now(), I16F16::from_num(target)); write_reg_tmr1_tick(0); write_reg_tmr_ctrl(read_reg_tmr_ctrl() | FLD_TMR::TMR1_EN.bits()); self.transition_step();