From 203c0bac5cd5a1ec3df87a569d769077efc23110 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 3 Apr 2024 12:47:11 +0200 Subject: [PATCH 1/2] re-work some FDIR logic --- .../DeviceHandlerFailureIsolation.cpp | 16 +++++++--------- .../DeviceHandlerFailureIsolation.h | 14 +++++++++++++- src/fsfw/fdir/FailureIsolationBase.cpp | 19 +++++-------------- src/fsfw/fdir/FailureIsolationBase.h | 4 ++-- 4 files changed, 27 insertions(+), 26 deletions(-) diff --git a/src/fsfw/devicehandlers/DeviceHandlerFailureIsolation.cpp b/src/fsfw/devicehandlers/DeviceHandlerFailureIsolation.cpp index d3e5753f..f27570ce 100644 --- a/src/fsfw/devicehandlers/DeviceHandlerFailureIsolation.cpp +++ b/src/fsfw/devicehandlers/DeviceHandlerFailureIsolation.cpp @@ -26,6 +26,13 @@ ReturnValue_t DeviceHandlerFailureIsolation::eventReceived(EventMessage* event) if (isFdirInActionOrAreWeFaulty(event)) { return returnvalue::OK; } + // No FDIR reactions if we are in external control by default. If the user commands the device + // manually, this might be related to debugging to testing the device in a low-level way. FDIR + // reactions might get in the way of this process by restarting the device or putting it in + // the faulty state. + if (owner->getHealth() == HasHealthIF::EXTERNAL_CONTROL) { + return returnvalue::OK; + } ReturnValue_t result = returnvalue::FAILED; switch (event->getEvent()) { case HasModesIF::MODE_TRANSITION_FAILED: @@ -186,15 +193,6 @@ void DeviceHandlerFailureIsolation::setFdirState(FDIRState state) { fdirState = state; } -void DeviceHandlerFailureIsolation::triggerEvent(Event event, uint32_t parameter1, - uint32_t parameter2) { - // Do not throw error events if fdirState != none. - // This will still forward MODE and HEALTH INFO events in any case. - if (fdirState == NONE || event::getSeverity(event) == severity::INFO) { - FailureIsolationBase::triggerEvent(event, parameter1, parameter2); - } -} - bool DeviceHandlerFailureIsolation::isFdirActionInProgress() { return (fdirState != NONE); } void DeviceHandlerFailureIsolation::startRecovery(Event reason) { diff --git a/src/fsfw/devicehandlers/DeviceHandlerFailureIsolation.h b/src/fsfw/devicehandlers/DeviceHandlerFailureIsolation.h index 4835af99..779b5385 100644 --- a/src/fsfw/devicehandlers/DeviceHandlerFailureIsolation.h +++ b/src/fsfw/devicehandlers/DeviceHandlerFailureIsolation.h @@ -17,7 +17,6 @@ class DeviceHandlerFailureIsolation : public FailureIsolationBase { uint8_t eventQueueDepth = 10); ~DeviceHandlerFailureIsolation(); ReturnValue_t initialize(); - void triggerEvent(Event event, uint32_t parameter1 = 0, uint32_t parameter2 = 0); bool isFdirActionInProgress(); virtual ReturnValue_t getParameter(uint8_t domainId, uint8_t uniqueId, ParameterWrapper* parameterWrapper, @@ -41,6 +40,19 @@ class DeviceHandlerFailureIsolation : public FailureIsolationBase { static const uint32_t DEFAULT_MAX_MISSED_REPLY_COUNT = 5; static const uint32_t DEFAULT_MISSED_REPLY_TIME_MS = 10000; + /** + * This is the default implementation of the eventReceived function. + * + * It will perform recoveries or failures on a pre-defined set of events. If the user wants + * to add handling for custom events, this function should be overriden. + * + * It should be noted that this function will not perform FDIR reactions if the handler is + * faulty or in external control by default. If the user commands the device + * manually, this might be related to debugging to testing the device in a low-level way. FDIR + * reactions might get in the way of this process by restarting the device or putting it in + * the faulty state. If the user still requires FDIR handling in the EXTERNAL_CONTROL case, + * this function should be overriden. + */ virtual ReturnValue_t eventReceived(EventMessage* event); virtual void eventConfirmed(EventMessage* event); void wasParentsFault(EventMessage* event); diff --git a/src/fsfw/fdir/FailureIsolationBase.cpp b/src/fsfw/fdir/FailureIsolationBase.cpp index b6dd3773..cbf2cc06 100644 --- a/src/fsfw/fdir/FailureIsolationBase.cpp +++ b/src/fsfw/fdir/FailureIsolationBase.cpp @@ -148,25 +148,16 @@ void FailureIsolationBase::doConfirmFault(EventMessage* event) { ReturnValue_t FailureIsolationBase::confirmFault(EventMessage* event) { return YOUR_FAULT; } void FailureIsolationBase::triggerEvent(Event event, uint32_t parameter1, uint32_t parameter2) { - // With this mechanism, all events are disabled for a certain device. - // That's not so good for visibility. - if (isFdirDisabledForSeverity(event::getSeverity(event))) { - return; - } + // By default, we trigger all events and also call the handler function to handle FDIR reactions + // which might occur due to these events. This makes all events visible. If the handling of + // FDIR reaction should be disabled, this should be done through dedicated logic inside the + // eventReceived function. EventMessage message(event, ownerId, parameter1, parameter2); EventManagerIF::triggerEvent(&message, eventQueue->getId()); eventReceived(&message); } -bool FailureIsolationBase::isFdirDisabledForSeverity(EventSeverity_t severity) { - if ((owner != NULL) && (severity != severity::INFO)) { - if (owner->getHealth() == HasHealthIF::EXTERNAL_CONTROL) { - // External control disables handling of fault messages. - return true; - } - } - return false; -} +bool FailureIsolationBase::isFdirDisabledForSeverity(EventSeverity_t severity) { return false; } void FailureIsolationBase::throwFdirEvent(Event event, uint32_t parameter1, uint32_t parameter2) { EventMessage message(event, ownerId, parameter1, parameter2); diff --git a/src/fsfw/fdir/FailureIsolationBase.h b/src/fsfw/fdir/FailureIsolationBase.h index 42d82d76..4ae7fb95 100644 --- a/src/fsfw/fdir/FailureIsolationBase.h +++ b/src/fsfw/fdir/FailureIsolationBase.h @@ -44,13 +44,13 @@ class FailureIsolationBase : public ConfirmsFailuresIF, public HasParametersIF { virtual void wasParentsFault(EventMessage* event); virtual ReturnValue_t confirmFault(EventMessage* event); virtual void decrementFaultCounters() = 0; + virtual bool isFdirDisabledForSeverity(EventSeverity_t severity); ReturnValue_t sendConfirmationRequest(EventMessage* event, MessageQueueId_t destination = MessageQueueIF::NO_QUEUE); void throwFdirEvent(Event event, uint32_t parameter1 = 0, uint32_t parameter2 = 0); private: void doConfirmFault(EventMessage* event); - bool isFdirDisabledForSeverity(EventSeverity_t severity); }; -#endif /* FRAMEWORK_FDIR_FAILUREISOLATIONBASE_H_ */ +#endif /* FRAMEWORK_FDIR From c5159fb64559a69a71470cf921b75b9a0b900ea3 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 3 Apr 2024 12:52:32 +0200 Subject: [PATCH 2/2] optimize docs --- src/fsfw/devicehandlers/DeviceHandlerFailureIsolation.cpp | 6 ++---- src/fsfw/devicehandlers/DeviceHandlerFailureIsolation.h | 4 ++-- src/fsfw/fdir/FailureIsolationBase.h | 2 +- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/fsfw/devicehandlers/DeviceHandlerFailureIsolation.cpp b/src/fsfw/devicehandlers/DeviceHandlerFailureIsolation.cpp index f27570ce..aa897769 100644 --- a/src/fsfw/devicehandlers/DeviceHandlerFailureIsolation.cpp +++ b/src/fsfw/devicehandlers/DeviceHandlerFailureIsolation.cpp @@ -26,10 +26,8 @@ ReturnValue_t DeviceHandlerFailureIsolation::eventReceived(EventMessage* event) if (isFdirInActionOrAreWeFaulty(event)) { return returnvalue::OK; } - // No FDIR reactions if we are in external control by default. If the user commands the device - // manually, this might be related to debugging to testing the device in a low-level way. FDIR - // reactions might get in the way of this process by restarting the device or putting it in - // the faulty state. + // As mentioned in the function documentation, no FDIR reaction are performed when the device + // is in external control. if (owner->getHealth() == HasHealthIF::EXTERNAL_CONTROL) { return returnvalue::OK; } diff --git a/src/fsfw/devicehandlers/DeviceHandlerFailureIsolation.h b/src/fsfw/devicehandlers/DeviceHandlerFailureIsolation.h index 779b5385..3c042caa 100644 --- a/src/fsfw/devicehandlers/DeviceHandlerFailureIsolation.h +++ b/src/fsfw/devicehandlers/DeviceHandlerFailureIsolation.h @@ -46,8 +46,8 @@ class DeviceHandlerFailureIsolation : public FailureIsolationBase { * It will perform recoveries or failures on a pre-defined set of events. If the user wants * to add handling for custom events, this function should be overriden. * - * It should be noted that this function will not perform FDIR reactions if the handler is - * faulty or in external control by default. If the user commands the device + * It should be noted that the default implementation will not perform FDIR reactions if the + * handler is faulty or in external control by default. If the user commands the device * manually, this might be related to debugging to testing the device in a low-level way. FDIR * reactions might get in the way of this process by restarting the device or putting it in * the faulty state. If the user still requires FDIR handling in the EXTERNAL_CONTROL case, diff --git a/src/fsfw/fdir/FailureIsolationBase.h b/src/fsfw/fdir/FailureIsolationBase.h index 4ae7fb95..efabf9cd 100644 --- a/src/fsfw/fdir/FailureIsolationBase.h +++ b/src/fsfw/fdir/FailureIsolationBase.h @@ -53,4 +53,4 @@ class FailureIsolationBase : public ConfirmsFailuresIF, public HasParametersIF { void doConfirmFault(EventMessage* event); }; -#endif /* FRAMEWORK_FDIR +#endif /* FRAMEWORK_FDIR */