diff --git a/satrs-example/src/acs/mgm.rs b/satrs-example/src/acs/mgm.rs index 7e5c391..158355f 100644 --- a/satrs-example/src/acs/mgm.rs +++ b/satrs-example/src/acs/mgm.rs @@ -1,5 +1,5 @@ use satrs::fdir::{FaultCounterStd, FaultResponse, RecoveryEvent, RecoveryFdir}; -use satrs::health::{HealthState, HealthTableMapSync}; +use satrs::health::HealthTableMapSync; use satrs::spacepackets::CcsdsPacketIdAndPsc; use satrs_example::{HkHelperSingleSet, TimestampHelper, TmtcQueues}; use satrs_minisim::acs::MgmRequestLis3Mdl; @@ -258,7 +258,9 @@ impl MgmHandlerLis3Mdl { } // The mode did not change for other components, so there is nothing to report. ModeTransitionEvent::PowerCycleDone => self.handle_recovery_done(), - ModeTransitionEvent::PowerCycleFailed => self.handle_recovery_failure(), + ModeTransitionEvent::PowerCycleFailed { restore_mode } => { + self.handle_recovery_failure(restore_mode) + } } } @@ -478,18 +480,22 @@ impl MgmHandlerLis3Mdl { ); self.send_event(mgm::Event::SpiFaultThresholdExceeded); self.send_event(mgm::Event::Recovery(RecoveryEvent::ThresholdExceeded)); - // Do not restart an already pending Off transition: poll_sensor still calls - // this every cycle the fault persists, and current stays Normal until the - // transition completes, so re-triggering here would keep resetting the - // transition state machine before it can ever finish. - if self.switch_and_mode_helper.target() != Some(DeviceMode::Off) { - log::warn!("{}: commanding device off due to fault", self.id.str()); - self.start_transition(DeviceMode::Off, None); - } + self.switch_off_faulty_device(); } } } + fn switch_off_faulty_device(&mut self) { + // Do not restart an already pending Off transition: poll_sensor still calls + // this every cycle the fault persists, and current stays Normal until the + // transition completes, so re-triggering here would keep resetting the + // transition state machine before it can ever finish. + if self.switch_and_mode_helper.target() != Some(DeviceMode::Off) { + log::warn!("{}: commanding device off due to fault", self.id.str()); + self.start_transition(DeviceMode::Off, None); + } + } + /// Starts a power cycle if the health is [satrs::health::HealthState::NeedsRecovery]. The health is set /// either by the FDIR or by ground. fn check_needs_recovery(&mut self) { @@ -505,10 +511,14 @@ impl MgmHandlerLis3Mdl { self.fdir.recovery_done(); return; } + self.start_recovery(self.mode()); + } + + fn start_recovery(&mut self, restore_mode: DeviceMode) { log::warn!("{}: starting power cycle recovery", self.id.str()); self.shared_mgm_set.lock().unwrap().valid = false; self.switch_and_mode_helper - .start_power_cycle(self.recovery_off_duration); + .start_power_cycle(restore_mode, self.recovery_off_duration); self.send_event(mgm::Event::Recovery(RecoveryEvent::Started)); } @@ -520,22 +530,30 @@ impl MgmHandlerLis3Mdl { self.send_event(mgm::Event::Recovery(RecoveryEvent::Done)); } - fn handle_recovery_failure(&mut self) { - log::error!( - "{}: power cycle recovery failed, marking component faulty", - self.id.str() - ); - self.fdir.recovery_failed(); + /// A failed power cycle costs a recovery attempt like any other fault. + fn handle_recovery_failure(&mut self, restore_mode: DeviceMode) { self.send_event(mgm::Event::Recovery(RecoveryEvent::Failed)); - if self.mode() == DeviceMode::Off { - // Switching back on failed. Other components still assume the mode before the - // power cycle. - self.announce_mode(); - self.report_mode_to_parent(); - } else if self.fdir.health() == Some(HealthState::Faulty) { - // Switching off failed. Retry like for any other faulty component. - log::warn!("{}: commanding device off due to fault", self.id.str()); - self.start_transition(DeviceMode::Off, None); + match self.fdir.recovery_failed() { + FaultResponse::Recover => { + log::warn!("{}: power cycle recovery failed, retrying", self.id.str()); + self.start_recovery(restore_mode); + } + FaultResponse::SetFaulty => { + log::error!( + "{}: power cycle recovery failed too often, marking component faulty", + self.id.str() + ); + self.send_event(mgm::Event::Recovery(RecoveryEvent::ThresholdExceeded)); + self.switch_off_faulty_device(); + } + FaultResponse::Ignored => { + // Ground changed the health during the recovery, so the device is left as it + // is. The power cycle is not hidden from other components anymore. + if self.mode() != restore_mode { + self.announce_mode(); + self.report_mode_to_parent(); + } + } } } @@ -626,7 +644,7 @@ mod tests { }; use arbitrary_int::u11; - use satrs::health::HealthTableProvider; + use satrs::health::{HealthState, HealthTableProvider}; use satrs::spacepackets::SpacePacketHeader; use satrs_minisim::acs::lis3mdl::MgmLis3RawValues; use types::{ @@ -1199,31 +1217,44 @@ mod tests { } #[test] - fn test_recovery_switch_on_timeout_marks_component_faulty() { + fn test_power_cycle_switch_on_failures_mark_component_faulty() { let mut testbench = MgmTestbench::new(); testbench.switch_to_normal(); - testbench.exceed_spi_fault_threshold(); - testbench.handler.periodic_operation(); - testbench.set_switch_state(SwitchState::Off); - testbench.handler.periodic_operation(); testbench.drain_events(); testbench.mode_report_rx.try_iter().for_each(drop); + testbench.exceed_spi_fault_threshold(); - // The switch never turns on again. - testbench.handler.periodic_operation(); - std::thread::sleep(Duration::from_millis(110)); - testbench.handler.periodic_operation(); - assert_eq!(testbench.handler.mode(), DeviceMode::Off); + // The switch never turns on again. Every failed power cycle costs a recovery attempt. + for _ in 0..RECOVERY_THRESHOLD { + assert_eq!(testbench.health(), Some(HealthState::NeedsRecovery)); + testbench.handler.periodic_operation(); + testbench.set_switch_state(SwitchState::Off); + testbench.handler.periodic_operation(); + testbench.handler.periodic_operation(); + std::thread::sleep(Duration::from_millis(110)); + testbench.handler.periodic_operation(); + } assert_eq!(testbench.health(), Some(HealthState::Faulty)); let events = testbench.drain_events(); + let started = events + .iter() + .filter(|e| matches!(e, mgm::Event::Recovery(RecoveryEvent::Started))) + .count(); + assert_eq!(started, RECOVERY_THRESHOLD as usize); assert!(matches!( events[..], [ + .., mgm::Event::Recovery(RecoveryEvent::Failed), - mgm::Event::ModeChanged(DeviceMode::Off) + mgm::Event::Recovery(RecoveryEvent::ThresholdExceeded) ] )); - // A failed power cycle is not hidden from the parent. + // Retries are hidden from the parent. + assert!(testbench.mode_report_rx.try_recv().is_err()); + + // The faulty device is commanded off, which is reported to the parent. + testbench.handler.periodic_operation(); + assert_eq!(testbench.handler.mode(), DeviceMode::Off); assert!(matches!( testbench.mode_report_rx.try_recv(), Ok(ModeResponse::Mode(DeviceMode::Off)) @@ -1231,34 +1262,36 @@ mod tests { } #[test] - fn test_recovery_switch_off_timeout_commands_device_off() { + fn test_power_cycle_switch_off_failures_mark_component_faulty() { let mut testbench = MgmTestbench::new(); testbench.switch_to_normal(); - testbench.exceed_spi_fault_threshold(); - testbench.handler.periodic_operation(); testbench.drain_events(); - testbench.drain_switch_requests(); testbench.mode_report_rx.try_iter().for_each(drop); + testbench.exceed_spi_fault_threshold(); + testbench.test_spi_interface().next_mgm_data = MgmLis3RawValues::default(); - // The switch never turns off. - std::thread::sleep(Duration::from_millis(110)); - testbench.handler.periodic_operation(); - assert_eq!(testbench.handler.mode(), DeviceMode::Normal); + // The switch never turns off. Every failed power cycle costs a recovery attempt. + for _ in 0..RECOVERY_THRESHOLD { + assert_eq!(testbench.health(), Some(HealthState::NeedsRecovery)); + testbench.handler.periodic_operation(); + std::thread::sleep(Duration::from_millis(110)); + testbench.handler.periodic_operation(); + } assert_eq!(testbench.health(), Some(HealthState::Faulty)); - // The mode did not change, so it is not announced or reported. - let events = testbench.drain_events(); - assert!(matches!( - events[..], - [mgm::Event::Recovery(RecoveryEvent::Failed)] - )); - assert!(testbench.mode_report_rx.try_recv().is_err()); + assert_eq!(testbench.handler.mode(), DeviceMode::Normal); assert_eq!( testbench.handler.switch_and_mode_helper.target(), Some(DeviceMode::Off) ); + // The mode never changed, so it was not announced or reported. + let events = testbench.drain_events(); + assert!( + !events + .iter() + .any(|e| matches!(e, mgm::Event::ModeChanged(_))) + ); + assert!(testbench.mode_report_rx.try_recv().is_err()); - testbench.handler.periodic_operation(); - assert_eq!(testbench.drain_switch_requests(), [SwitchStateBinary::Off]); testbench.set_switch_state(SwitchState::Off); testbench.handler.periodic_operation(); assert_eq!(testbench.handler.mode(), DeviceMode::Off); diff --git a/satrs-example/src/device_mode.rs b/satrs-example/src/device_mode.rs index 0c069b2..c6842f4 100644 --- a/satrs-example/src/device_mode.rs +++ b/satrs-example/src/device_mode.rs @@ -48,7 +48,7 @@ enum PowerCycleState { /// has driven it to completion. Carries back whichever TC commanded the transition, if any, so /// the caller can reply to it -- what that reply looks like is handler-specific, so this stays /// out of the helper. -pub enum ModeTransitionEvent { +pub enum ModeTransitionEvent { /// The target mode was reached. Reached(Option), /// The target mode could not be reached. @@ -56,8 +56,9 @@ pub enum ModeTransitionEvent { /// The power cycle completed and the mode before the power cycle was restored. PowerCycleDone, /// Power switching failed during the power cycle. The power cycle is not hidden anymore, - /// so [SwitchAndModeHelper::reported_mode] returns the actual mode again. - PowerCycleFailed, + /// so [SwitchAndModeHelper::reported_mode] returns the actual mode again. `restore_mode` is + /// the mode the power cycle should have restored, which can be used to retry it. + PowerCycleFailed { restore_mode: Mode }, } /// Drives the on/off power-switch commanding state machine (Idle -> PowerSwitching -> Done) @@ -126,12 +127,10 @@ impl SwitchAndModeHelper { self.start_transition_internal(target_mode, tc_commander); } - /// Switches the device off, keeps it off for `off_duration` and then restores the current - /// mode. Reaching the intermediate off mode does not generate an event. - pub fn start_power_cycle(&mut self, off_duration: Duration) { - self.power_cycle_state = PowerCycleState::SwitchingOff { - restore_mode: self.mode(), - }; + /// Switches the device off, keeps it off for `off_duration` and then switches it to + /// `restore_mode`. Reaching the intermediate off mode does not generate an event. + pub fn start_power_cycle(&mut self, restore_mode: Mode, off_duration: Duration) { + self.power_cycle_state = PowerCycleState::SwitchingOff { restore_mode }; self.power_cycle_off_duration = off_duration; self.start_transition_internal(Mode::OFF, None); } @@ -148,7 +147,7 @@ impl SwitchAndModeHelper { /// This is the main API that the periodic handler of a device handler should call. /// /// It handles the switch commanding and returns relevant events. - pub fn handle_mode_transition(&mut self) -> Option { + pub fn handle_mode_transition(&mut self) -> Option> { // The most probable case: Nothing to do. if self.target().is_none() && !self.power_cycle_active() { return None; @@ -168,8 +167,9 @@ impl SwitchAndModeHelper { // Power cycling, where a bit more logic is required. // Handle the error case first. if let SwitchOutcome::Failed(_) = outcome { + let restore_mode = self.reported_mode(); self.power_cycle_state = PowerCycleState::Idle; - return Some(ModeTransitionEvent::PowerCycleFailed); + return Some(ModeTransitionEvent::PowerCycleFailed { restore_mode }); } // At this point: The switching was succesfull, so we only match on the // power cycle state. @@ -306,7 +306,8 @@ mod tests { /// Starts a power cycle from `Normal` and drives it until the device is off. fn power_cycle_until_off(&mut self, off_duration: Duration) { self.switch_to_normal(); - self.helper.start_power_cycle(off_duration); + self.helper + .start_power_cycle(DeviceMode::Normal, off_duration); assert!(self.helper.handle_mode_transition().is_none()); assert_eq!(self.switch_requests(), [SwitchStateBinary::Off]); self.set_switch_state(SwitchState::Off); @@ -417,7 +418,8 @@ mod tests { fn test_power_cycle_reports_restored_mode() { let mut tb = Testbench::new(); tb.switch_to_normal(); - tb.helper.start_power_cycle(Duration::from_secs(60)); + tb.helper + .start_power_cycle(DeviceMode::Normal, Duration::from_secs(60)); assert_eq!(tb.helper.reported_mode(), DeviceMode::Normal); tb.helper.handle_mode_transition(); tb.set_switch_state(SwitchState::Off); @@ -452,12 +454,15 @@ mod tests { fn test_power_cycle_switch_off_timeout() { let mut tb = Testbench::new(); tb.switch_to_normal(); - tb.helper.start_power_cycle(Duration::ZERO); + tb.helper + .start_power_cycle(DeviceMode::Normal, Duration::ZERO); assert!(tb.helper.handle_mode_transition().is_none()); std::thread::sleep(TIMEOUT); assert!(matches!( tb.helper.handle_mode_transition(), - Some(ModeTransitionEvent::PowerCycleFailed) + Some(ModeTransitionEvent::PowerCycleFailed { + restore_mode: DeviceMode::Normal + }) )); assert_eq!(tb.helper.mode(), DeviceMode::Normal); assert!(!tb.helper.power_cycle_active()); @@ -471,7 +476,9 @@ mod tests { std::thread::sleep(TIMEOUT); assert!(matches!( tb.helper.handle_mode_transition(), - Some(ModeTransitionEvent::PowerCycleFailed) + Some(ModeTransitionEvent::PowerCycleFailed { + restore_mode: DeviceMode::Normal + }) )); assert_eq!(tb.helper.mode(), DeviceMode::Off); assert!(!tb.helper.power_cycle_active()); diff --git a/satrs/src/fdir.rs b/satrs/src/fdir.rs index 1b7b5b6..045e143 100644 --- a/satrs/src/fdir.rs +++ b/satrs/src/fdir.rs @@ -30,17 +30,19 @@ use crate::health::{HealthState, HealthTableProvider}; #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] #[cfg_attr(feature = "defmt", derive(defmt::Format))] pub enum RecoveryEvent { - /// The component health is [crate::health::HealthState::NeedsRecovery] and it is being power cycled. + /// The component health is [crate::health::HealthState::NeedsRecovery] and it is being + /// power cycled. Started, /// The power cycle completed and the component is healthy again. Done, - /// The power cycle failed, the component was marked faulty. + /// The power cycle failed. This costs a recovery attempt like any other fault, so it is + /// followed by either a new recovery or [RecoveryEvent::ThresholdExceeded]. Failed, /// The component was recovered too often, it was marked faulty. ThresholdExceeded, } -/// Outcome of [RecoveryFdir::handle_fault]. +/// Outcome of [RecoveryFdir::handle_fault] and [RecoveryFdir::recovery_failed]. #[derive(Debug, Copy, Clone, PartialEq, Eq)] pub enum FaultResponse { /// The component is already faulty, recovering or externally controlled, so nothing was @@ -49,8 +51,8 @@ pub enum FaultResponse { /// The health was set to [crate::health::HealthState::NeedsRecovery]. The component should /// be power cycled. Recover, - /// The component was recovered too often and its health was set to [crate::health::HealthState::Faulty]. - /// The component should be switched off. + /// The component was recovered too often and its health was set to + /// [crate::health::HealthState::Faulty]. The component should be switched off. SetFaulty, } @@ -330,6 +332,12 @@ impl RecoveryFdir { ) { return FaultResponse::Ignored; } + self.escalate() + } + + /// Every recovery attempt counts. If there were too many attempts, the component is marked + /// faulty. Otherwise, the component should be recovered (again). + fn escalate(&mut self) -> FaultResponse { if self.recovery_counter.increment_and_check() { self.set_health(HealthState::Faulty); return FaultResponse::SetFaulty; @@ -351,11 +359,13 @@ impl RecoveryFdir { } } - /// The power cycle failed. Marks the component faulty. - pub fn recovery_failed(&mut self) { - if self.needs_recovery() { - self.set_health(HealthState::Faulty); + /// The power cycle failed. This costs a recovery attempt like any other fault. + pub fn recovery_failed(&mut self) -> FaultResponse { + // The health might have changed during the recovery. + if !self.needs_recovery() { + return FaultResponse::Ignored; } + self.escalate() } } @@ -411,10 +421,16 @@ mod tests { } fn recovery_fdir() -> RecoveryFdir { + recovery_fdir_with_threshold(1) + } + + fn recovery_fdir_with_threshold( + recovery_threshold: u32, + ) -> RecoveryFdir { RecoveryFdir::new( 1, crate::health::HealthTableMapSync::default(), - 1, + recovery_threshold, Duration::from_secs(60), ) } @@ -449,10 +465,12 @@ mod tests { } #[test] - fn failed_recovery_sets_faulty() { - let mut fdir = recovery_fdir(); + fn failed_recovery_is_retried() { + let mut fdir = recovery_fdir_with_threshold(2); fdir.handle_fault(); - fdir.recovery_failed(); + assert_eq!(fdir.recovery_failed(), FaultResponse::Recover); + assert!(fdir.needs_recovery()); + assert_eq!(fdir.recovery_failed(), FaultResponse::SetFaulty); assert_eq!(fdir.health(), Some(HealthState::Faulty)); } @@ -478,7 +496,7 @@ mod tests { fdir.set_health(HealthState::ExternalControl); fdir.recovery_done(); assert_eq!(fdir.health(), Some(HealthState::ExternalControl)); - fdir.recovery_failed(); + assert_eq!(fdir.recovery_failed(), FaultResponse::Ignored); assert_eq!(fdir.health(), Some(HealthState::ExternalControl)); }