From 4bb078c451ddf543e15e27073f5c8edf1642d38e Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Mon, 12 Apr 2021 12:40:59 +0200 Subject: [PATCH 01/17] thermal sensor update --- thermal/TemperatureSensor.h | 54 +++++++++++++++++++------------------ 1 file changed, 28 insertions(+), 26 deletions(-) diff --git a/thermal/TemperatureSensor.h b/thermal/TemperatureSensor.h index 2b1fb1f07..ceb8a8617 100644 --- a/thermal/TemperatureSensor.h +++ b/thermal/TemperatureSensor.h @@ -1,11 +1,14 @@ #ifndef TEMPERATURESENSOR_H_ #define TEMPERATURESENSOR_H_ -#include "../thermal/AbstractTemperatureSensor.h" -#include "../datapoolglob/GlobalDataSet.h" -#include "../datapoolglob/GlobalPoolVariable.h" +#include "tcsDefinitions.h" +#include "AbstractTemperatureSensor.h" + +#include "../datapoollocal/LocalPoolDataSetBase.h" +#include "../datapoollocal/LocalPoolVariable.h" #include "../monitoring/LimitMonitor.h" + /** * @brief This building block handles non-linear value conversion and * range checks for analog temperature sensors. @@ -57,27 +60,26 @@ public: /** * Instantiate Temperature Sensor Object. - * @param setObjectid objectId of the sensor object - * @param inputValue Input value which is converted to a temperature - * @param poolVariable Pool Variable to store the temperature value - * @param vectorIndex Vector Index for the sensor monitor - * @param parameters Calculation parameters, temperature limits, gradient limit - * @param datapoolId Datapool ID of the output temperature - * @param outputSet Output dataset for the output temperature to fetch it with read() + * @param setObjectid objectId of the sensor object + * @param inputValue Pointer to input value which is converted to a temperature + * @param variableGpid Global Pool ID of the output value + * @param inputVariable Input variable handle + * @param vectorIndex Vector Index for the sensor monitor + * @param parameters Calculation parameters, temperature limits, gradient limit + * @param outputSet Output dataset for the output temperature to fetch it with read() * @param thermalModule respective thermal module, if it has one */ TemperatureSensor(object_id_t setObjectid, - inputType *inputValue, PoolVariableIF *poolVariable, - uint8_t vectorIndex, uint32_t datapoolId, Parameters parameters = {0, 0, 0, 0, 0, 0}, - GlobDataSet *outputSet = NULL, ThermalModuleIF *thermalModule = NULL) : + inputType *inputValue, gp_id_t variableGpid, PoolVariableIF* inputVariable, + uint8_t vectorIndex, Parameters parameters = {0, 0, 0, 0, 0, 0}, + LocalPoolDataSetBase *outputSet = NULL, ThermalModuleIF *thermalModule = NULL) : AbstractTemperatureSensor(setObjectid, thermalModule), parameters(parameters), - inputValue(inputValue), poolVariable(poolVariable), - outputTemperature(datapoolId, outputSet, PoolVariableIF::VAR_WRITE), - sensorMonitor(setObjectid, DOMAIN_ID_SENSOR, - GlobalDataPool::poolIdAndPositionToPid(poolVariable->getDataPoolId(), vectorIndex), + inputValue(inputValue), poolVariable(inputVariable), + outputTemperature(variableGpid, outputSet, PoolVariableIF::VAR_WRITE), + sensorMonitor(setObjectid, DOMAIN_ID_SENSOR, poolVariable, DEFAULT_CONFIRMATION_COUNT, parameters.lowerLimit, parameters.upperLimit, TEMP_SENSOR_LOW, TEMP_SENSOR_HIGH), - oldTemperature(20), uptimeOfOldTemperature( { INVALID_TEMPERATURE, 0 }) { + oldTemperature(20), uptimeOfOldTemperature({ thermal::INVALID_TEMPERATURE, 0 }) { } @@ -98,7 +100,7 @@ protected: private: void setInvalid() { - outputTemperature = INVALID_TEMPERATURE; + outputTemperature = thermal::INVALID_TEMPERATURE; outputTemperature.setValid(false); uptimeOfOldTemperature.tv_sec = INVALID_UPTIME; sensorMonitor.setToInvalid(); @@ -108,11 +110,11 @@ protected: UsedParameters parameters; - inputType * inputValue; + inputType* inputValue; - PoolVariableIF *poolVariable; + PoolVariableIF* poolVariable; - gp_float_t outputTemperature; + lp_var_t outputTemperature; LimitMonitor sensorMonitor; @@ -120,8 +122,8 @@ protected: timeval uptimeOfOldTemperature; void doChildOperation() { - if (!poolVariable->isValid() - || !healthHelper.healthTable->isHealthy(getObjectId())) { + if ((not poolVariable->isValid()) or + (not healthHelper.healthTable->isHealthy(getObjectId()))) { setInvalid(); return; } @@ -152,13 +154,13 @@ protected: } } - //Check is done against raw limits. SHOULDDO: Why? Using �C would be more easy to handle. + //Check is done against raw limits. SHOULDDO: Why? Using C would be more easy to handle. sensorMonitor.doCheck(outputTemperature.value); if (sensorMonitor.isOutOfLimits()) { uptimeOfOldTemperature.tv_sec = INVALID_UPTIME; outputTemperature.setValid(PoolVariableIF::INVALID); - outputTemperature = INVALID_TEMPERATURE; + outputTemperature = thermal::INVALID_TEMPERATURE; } else { oldTemperature = outputTemperature; uptimeOfOldTemperature = uptime; From 6873d2b8473469290a268149cb939a745e92e67f Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 20 Apr 2021 14:46:59 +0200 Subject: [PATCH 02/17] temp sensor update --- thermal/TemperatureSensor.h | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/thermal/TemperatureSensor.h b/thermal/TemperatureSensor.h index ceb8a8617..f1e17c559 100644 --- a/thermal/TemperatureSensor.h +++ b/thermal/TemperatureSensor.h @@ -122,6 +122,11 @@ protected: timeval uptimeOfOldTemperature; void doChildOperation() { + ReturnValue_t result = poolVariable->read(MutexIF::TimeoutType::WAITING, 20); + if(result != HasReturnvaluesIF::RETURN_OK) { + return; + } + if ((not poolVariable->isValid()) or (not healthHelper.healthTable->isHealthy(getObjectId()))) { setInvalid(); From 64efb8ec7f2cd64c188791043289e4a45d79ad87 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 20 Apr 2021 14:58:35 +0200 Subject: [PATCH 03/17] temperature sensor update --- thermal/TemperatureSensor.h | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/thermal/TemperatureSensor.h b/thermal/TemperatureSensor.h index f1e17c559..abe446c0b 100644 --- a/thermal/TemperatureSensor.h +++ b/thermal/TemperatureSensor.h @@ -69,14 +69,14 @@ public: * @param outputSet Output dataset for the output temperature to fetch it with read() * @param thermalModule respective thermal module, if it has one */ - TemperatureSensor(object_id_t setObjectid, - inputType *inputValue, gp_id_t variableGpid, PoolVariableIF* inputVariable, + TemperatureSensor(object_id_t setObjectid,lp_var_t* inputTemperature, + gp_id_t variableGpid, PoolVariableIF* inputVariable, uint8_t vectorIndex, Parameters parameters = {0, 0, 0, 0, 0, 0}, - LocalPoolDataSetBase *outputSet = NULL, ThermalModuleIF *thermalModule = NULL) : + LocalPoolDataSetBase *outputSet = nullptr, ThermalModuleIF *thermalModule = nullptr) : AbstractTemperatureSensor(setObjectid, thermalModule), parameters(parameters), - inputValue(inputValue), poolVariable(inputVariable), + inputTemperature(inputTemperature), outputTemperature(variableGpid, outputSet, PoolVariableIF::VAR_WRITE), - sensorMonitor(setObjectid, DOMAIN_ID_SENSOR, poolVariable, + sensorMonitor(setObjectid, DOMAIN_ID_SENSOR, variableGpid, DEFAULT_CONFIRMATION_COUNT, parameters.lowerLimit, parameters.upperLimit, TEMP_SENSOR_LOW, TEMP_SENSOR_HIGH), oldTemperature(20), uptimeOfOldTemperature({ thermal::INVALID_TEMPERATURE, 0 }) { @@ -110,10 +110,7 @@ protected: UsedParameters parameters; - inputType* inputValue; - - PoolVariableIF* poolVariable; - + lp_var_t* inputTemperature; lp_var_t outputTemperature; LimitMonitor sensorMonitor; @@ -122,18 +119,18 @@ protected: timeval uptimeOfOldTemperature; void doChildOperation() { - ReturnValue_t result = poolVariable->read(MutexIF::TimeoutType::WAITING, 20); + ReturnValue_t result = inputTemperature->read(MutexIF::TimeoutType::WAITING, 20); if(result != HasReturnvaluesIF::RETURN_OK) { return; } - if ((not poolVariable->isValid()) or + if ((not inputTemperature->isValid()) or (not healthHelper.healthTable->isHealthy(getObjectId()))) { setInvalid(); return; } - outputTemperature = calculateOutputTemperature(*inputValue); + outputTemperature = calculateOutputTemperature(inputTemperature->value); outputTemperature.setValid(PoolVariableIF::VALID); timeval uptime; From e6a8371d4a6a54ad4091eb3bf48da6aea96d522e Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 20 Apr 2021 15:00:59 +0200 Subject: [PATCH 04/17] temperature sensor update --- thermal/TemperatureSensor.h | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/thermal/TemperatureSensor.h b/thermal/TemperatureSensor.h index abe446c0b..55138cc3b 100644 --- a/thermal/TemperatureSensor.h +++ b/thermal/TemperatureSensor.h @@ -137,9 +137,9 @@ protected: Clock::getUptime(&uptime); if (uptimeOfOldTemperature.tv_sec != INVALID_UPTIME) { - //In theory, we could use an AbsValueMonitor to monitor the gradient. - //But this would require storing the maxGradient in DP and quite some overhead. - //The concept of delta limits is a bit strange anyway. + // In theory, we could use an AbsValueMonitor to monitor the gradient. + // But this would require storing the maxGradient in DP and quite some overhead. + // The concept of delta limits is a bit strange anyway. float deltaTime; float deltaTemp; @@ -152,11 +152,10 @@ protected: } if (parameters.gradient < deltaTemp / deltaTime) { triggerEvent(TEMP_SENSOR_GRADIENT); - //Don't set invalid, as we did not recognize it as invalid with full authority, let FDIR handle it + // Don't set invalid, as we did not recognize it as invalid with full authority, let FDIR handle it } } - //Check is done against raw limits. SHOULDDO: Why? Using C would be more easy to handle. sensorMonitor.doCheck(outputTemperature.value); if (sensorMonitor.isOutOfLimits()) { From 7b29583f8f34e96b23fd6202252f58146e17d978 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 20 Apr 2021 15:32:12 +0200 Subject: [PATCH 05/17] small improvements --- defaultcfg/fsfwconfig/FSFWConfig.h | 13 +++++++------ osal/linux/PosixThread.cpp | 10 +++++++++- osal/linux/tcpipHelpers.cpp | 1 + 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/defaultcfg/fsfwconfig/FSFWConfig.h b/defaultcfg/fsfwconfig/FSFWConfig.h index e1e0ec644..fe18a2f43 100644 --- a/defaultcfg/fsfwconfig/FSFWConfig.h +++ b/defaultcfg/fsfwconfig/FSFWConfig.h @@ -40,6 +40,13 @@ //! Specify whether a special mode store is used for Subsystem components. #define FSFW_USE_MODESTORE 0 +//! Defines if the real time scheduler for linux should be used. +//! If set to 0, this will also disable priority settings for linux +//! as most systems will not allow to set nice values without privileges +//! For embedded linux system set this to 1. +//! If set to 1 the binary needs "cap_sys_nice=eip" privileges to run +#define FSFW_USE_REALTIME_FOR_LINUX 1 + namespace fsfwconfig { //! Default timestamp size. The default timestamp will be an eight byte CDC //! short timestamp. @@ -58,12 +65,6 @@ static constexpr uint8_t FSFW_CSB_FIFO_DEPTH = 6; static constexpr size_t FSFW_PRINT_BUFFER_SIZE = 124; -//! Defines if the real time scheduler for linux should be used. -//! If set to 0, this will also disable priority settings for linux -//! as most systems will not allow to set nice values without privileges -//! For embedded linux system set this to 1. -//! If set to 1 the binary needs "cap_sys_nice=eip" privileges to run -#define FSFW_USE_REALTIME_FOR_LINUX 1 } #endif /* CONFIG_FSFWCONFIG_H_ */ diff --git a/osal/linux/PosixThread.cpp b/osal/linux/PosixThread.cpp index f1cff9925..72adfb140 100644 --- a/osal/linux/PosixThread.cpp +++ b/osal/linux/PosixThread.cpp @@ -223,8 +223,16 @@ void PosixThread::createTask(void* (*fnc_)(void*), void* arg_) { status = pthread_create(&thread,&attributes,fnc_,arg_); if(status != 0){ #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "Posix Thread create failed with: " << + sif::error << "PosixThread::createTask: Failed with: " << strerror(status) << std::endl; + sif::error << "For FSFW_USE_REALTIME_FOR_LINUX == 1 make sure to call " << + "\"all sudo setcap 'cap_sys_nice=eip'\" on the application or set " + "/etc/security/limit.conf" << std::endl; +#else + sif::printError("PosixThread::createTask: Create failed with: %s\n", strerror(status)); + sif::printError("For FSFW_USE_REALTIME_FOR_LINUX == 1 make sure to call " + "\"all sudo setcap 'cap_sys_nice=eip'\" on the application or set " + "/etc/security/limit.conf\n"); #endif } diff --git a/osal/linux/tcpipHelpers.cpp b/osal/linux/tcpipHelpers.cpp index 4c1b9a780..3e8f60092 100644 --- a/osal/linux/tcpipHelpers.cpp +++ b/osal/linux/tcpipHelpers.cpp @@ -1,5 +1,6 @@ #include "../common/tcpipHelpers.h" +#include "../../serviceinterface/ServiceInterface.h" #include "../../tasks/TaskFactory.h" #include From 524e50a6dd5649b9748c677b72e00bdb9e9a725e Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 20 Apr 2021 15:54:44 +0200 Subject: [PATCH 06/17] comment block corrected --- thermal/TemperatureSensor.h | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/thermal/TemperatureSensor.h b/thermal/TemperatureSensor.h index 55138cc3b..b4ea45049 100644 --- a/thermal/TemperatureSensor.h +++ b/thermal/TemperatureSensor.h @@ -60,18 +60,17 @@ public: /** * Instantiate Temperature Sensor Object. - * @param setObjectid objectId of the sensor object - * @param inputValue Pointer to input value which is converted to a temperature - * @param variableGpid Global Pool ID of the output value - * @param inputVariable Input variable handle - * @param vectorIndex Vector Index for the sensor monitor - * @param parameters Calculation parameters, temperature limits, gradient limit - * @param outputSet Output dataset for the output temperature to fetch it with read() - * @param thermalModule respective thermal module, if it has one + * @param setObjectid objectId of the sensor object + * @param inputTemperature Pointer to a raw input value which is converted to an floating + * point C output temperature + * @param variableGpid Global Pool ID of the output value + * @param vectorIndex Vector Index for the sensor monitor + * @param parameters Calculation parameters, temperature limits, gradient limit + * @param outputSet Output dataset for the output temperature to fetch it with read() + * @param thermalModule Respective thermal module, if it has one */ TemperatureSensor(object_id_t setObjectid,lp_var_t* inputTemperature, - gp_id_t variableGpid, PoolVariableIF* inputVariable, - uint8_t vectorIndex, Parameters parameters = {0, 0, 0, 0, 0, 0}, + gp_id_t variableGpid, uint8_t vectorIndex, Parameters parameters = {0, 0, 0, 0, 0, 0}, LocalPoolDataSetBase *outputSet = nullptr, ThermalModuleIF *thermalModule = nullptr) : AbstractTemperatureSensor(setObjectid, thermalModule), parameters(parameters), inputTemperature(inputTemperature), From 0c342ad7fcc715266a2562f65f2cec034d40e919 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 20 Apr 2021 15:57:11 +0200 Subject: [PATCH 07/17] minor changes --- thermal/TemperatureSensor.h | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/thermal/TemperatureSensor.h b/thermal/TemperatureSensor.h index b4ea45049..69c16c7f6 100644 --- a/thermal/TemperatureSensor.h +++ b/thermal/TemperatureSensor.h @@ -1,5 +1,5 @@ -#ifndef TEMPERATURESENSOR_H_ -#define TEMPERATURESENSOR_H_ +#ifndef FSFW_THERMAL_TEMPERATURESENSOR_H_ +#define FSFW_THERMAL_TEMPERATURESENSOR_H_ #include "tcsDefinitions.h" #include "AbstractTemperatureSensor.h" @@ -151,7 +151,8 @@ protected: } if (parameters.gradient < deltaTemp / deltaTime) { triggerEvent(TEMP_SENSOR_GRADIENT); - // Don't set invalid, as we did not recognize it as invalid with full authority, let FDIR handle it + // Don't set invalid, as we did not recognize it as invalid with full authority, + // let FDIR handle it } } @@ -181,7 +182,10 @@ public: static const uint16_t ADDRESS_C = 2; static const uint16_t ADDRESS_GRADIENT = 3; - static const uint16_t DEFAULT_CONFIRMATION_COUNT = 1; //!< Changed due to issue with later temperature checking even tough the sensor monitor was confirming already (Was 10 before with comment = Correlates to a 10s confirmation time. Chosen rather large, should not be so bad for components and helps survive glitches.) + //! Changed due to issue with later temperature checking even tough the sensor monitor was + //! confirming already (Was 10 before with comment = Correlates to a 10s confirmation time. + //! Chosen rather large, should not be so bad for components and helps survive glitches.) + static const uint16_t DEFAULT_CONFIRMATION_COUNT = 1; static const uint8_t DOMAIN_ID_SENSOR = 1; @@ -221,4 +225,4 @@ public: }; -#endif /* TEMPERATURESENSOR_H_ */ +#endif /* FSFW_THERMAL_TEMPERATURESENSOR_H_ */ From 38a5e7e618ebb7a9190b894b2ba6d302049aa042 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 20 Apr 2021 16:00:01 +0200 Subject: [PATCH 08/17] ctor variable better name --- thermal/TemperatureSensor.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/thermal/TemperatureSensor.h b/thermal/TemperatureSensor.h index 69c16c7f6..c7b9d771b 100644 --- a/thermal/TemperatureSensor.h +++ b/thermal/TemperatureSensor.h @@ -63,19 +63,19 @@ public: * @param setObjectid objectId of the sensor object * @param inputTemperature Pointer to a raw input value which is converted to an floating * point C output temperature - * @param variableGpid Global Pool ID of the output value + * @param outputGpid Global Pool ID of the output value * @param vectorIndex Vector Index for the sensor monitor * @param parameters Calculation parameters, temperature limits, gradient limit * @param outputSet Output dataset for the output temperature to fetch it with read() * @param thermalModule Respective thermal module, if it has one */ TemperatureSensor(object_id_t setObjectid,lp_var_t* inputTemperature, - gp_id_t variableGpid, uint8_t vectorIndex, Parameters parameters = {0, 0, 0, 0, 0, 0}, + gp_id_t outputGpid, uint8_t vectorIndex, Parameters parameters = {0, 0, 0, 0, 0, 0}, LocalPoolDataSetBase *outputSet = nullptr, ThermalModuleIF *thermalModule = nullptr) : AbstractTemperatureSensor(setObjectid, thermalModule), parameters(parameters), inputTemperature(inputTemperature), - outputTemperature(variableGpid, outputSet, PoolVariableIF::VAR_WRITE), - sensorMonitor(setObjectid, DOMAIN_ID_SENSOR, variableGpid, + outputTemperature(outputGpid, outputSet, PoolVariableIF::VAR_WRITE), + sensorMonitor(setObjectid, DOMAIN_ID_SENSOR, outputGpid, DEFAULT_CONFIRMATION_COUNT, parameters.lowerLimit, parameters.upperLimit, TEMP_SENSOR_LOW, TEMP_SENSOR_HIGH), oldTemperature(20), uptimeOfOldTemperature({ thermal::INVALID_TEMPERATURE, 0 }) { From d792679d49d6e3cc1cf323e046abc1baf7d11c1a Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 20 Apr 2021 16:09:50 +0200 Subject: [PATCH 09/17] calling empty ctor now (coverity) --- serialize/SerializeElement.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/serialize/SerializeElement.h b/serialize/SerializeElement.h index 470802927..db66f9cc2 100644 --- a/serialize/SerializeElement.h +++ b/serialize/SerializeElement.h @@ -25,7 +25,7 @@ public: } SerializeElement() : - LinkedElement(this) { + LinkedElement(this), entry() { } ReturnValue_t serialize(uint8_t **buffer, size_t *size, size_t maxSize, From 54ff8f9341c4d7ea16296b9bd5c4167ddb19a2ee Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 20 Apr 2021 16:12:50 +0200 Subject: [PATCH 10/17] heater tweaks + coverity fix --- thermal/Heater.cpp | 596 +++++++++++++++++++++++---------------------- thermal/Heater.h | 115 ++++----- 2 files changed, 359 insertions(+), 352 deletions(-) diff --git a/thermal/Heater.cpp b/thermal/Heater.cpp index 782ce2963..8bfa030b9 100644 --- a/thermal/Heater.cpp +++ b/thermal/Heater.cpp @@ -1,350 +1,354 @@ -#include "../devicehandlers/DeviceHandlerFailureIsolation.h" #include "Heater.h" +#include "../devicehandlers/DeviceHandlerFailureIsolation.h" #include "../power/Fuse.h" #include "../ipc/QueueFactory.h" Heater::Heater(uint32_t objectId, uint8_t switch0, uint8_t switch1) : - HealthDevice(objectId, 0), internalState(STATE_OFF), powerSwitcher( - NULL), pcduQueueId(0), switch0(switch0), switch1(switch1), wasOn( - false), timedOut(false), reactedToBeingFaulty(false), passive( - false), eventQueue(NULL), heaterOnCountdown(10800000)/*about two orbits*/, parameterHelper( - this), lastAction(CLEAR) { - eventQueue = QueueFactory::instance()->createMessageQueue(); +HealthDevice(objectId, 0), internalState(STATE_OFF), powerSwitcher( + NULL), pcduQueueId(0), switch0(switch0), switch1(switch1), wasOn(false), timedOut(false), + reactedToBeingFaulty(false), passive(false), eventQueue(NULL), + heaterOnCountdown(10800000)/*about two orbits*/, parameterHelper(this), lastAction(CLEAR) { + eventQueue = QueueFactory::instance()->createMessageQueue(); } Heater::~Heater() { - QueueFactory::instance()->deleteMessageQueue(eventQueue); + QueueFactory::instance()->deleteMessageQueue(eventQueue); } ReturnValue_t Heater::set() { - passive = false; - //wait for clear before doing anything - if (internalState == STATE_WAIT) { - return HasReturnvaluesIF::RETURN_OK; - } - if (healthHelper.healthTable->isHealthy(getObjectId())) { - doAction(SET); - if ((internalState == STATE_OFF) || (internalState == STATE_PASSIVE)){ - return HasReturnvaluesIF::RETURN_FAILED; - } else { - return HasReturnvaluesIF::RETURN_OK; - } - } else { - if (healthHelper.healthTable->isFaulty(getObjectId())) { - if (!reactedToBeingFaulty) { - reactedToBeingFaulty = true; - doAction(CLEAR); - } - } - return HasReturnvaluesIF::RETURN_FAILED; - } + passive = false; + //wait for clear before doing anything + if (internalState == STATE_WAIT) { + return HasReturnvaluesIF::RETURN_OK; + } + if (healthHelper.healthTable->isHealthy(getObjectId())) { + doAction(SET); + if ((internalState == STATE_OFF) || (internalState == STATE_PASSIVE)){ + return HasReturnvaluesIF::RETURN_FAILED; + } else { + return HasReturnvaluesIF::RETURN_OK; + } + } else { + if (healthHelper.healthTable->isFaulty(getObjectId())) { + if (!reactedToBeingFaulty) { + reactedToBeingFaulty = true; + doAction(CLEAR); + } + } + return HasReturnvaluesIF::RETURN_FAILED; + } } void Heater::clear(bool passive) { - this->passive = passive; - //Force switching off - if (internalState == STATE_WAIT) { - internalState = STATE_ON; - } - if (healthHelper.healthTable->isHealthy(getObjectId())) { - doAction(CLEAR); - } else if (healthHelper.healthTable->isFaulty(getObjectId())) { - if (!reactedToBeingFaulty) { - reactedToBeingFaulty = true; - doAction(CLEAR); - } - } + this->passive = passive; + //Force switching off + if (internalState == STATE_WAIT) { + internalState = STATE_ON; + } + if (healthHelper.healthTable->isHealthy(getObjectId())) { + doAction(CLEAR); + } else if (healthHelper.healthTable->isFaulty(getObjectId())) { + if (!reactedToBeingFaulty) { + reactedToBeingFaulty = true; + doAction(CLEAR); + } + } } void Heater::doAction(Action action) { - //only act if we are not in the right state or in a transition - if (action == SET) { - if ((internalState == STATE_OFF) || (internalState == STATE_PASSIVE) - || (internalState == STATE_EXTERNAL_CONTROL)) { - switchCountdown.setTimeout(powerSwitcher->getSwitchDelayMs()); - internalState = STATE_WAIT_FOR_SWITCHES_ON; - powerSwitcher->sendSwitchCommand(switch0, PowerSwitchIF::SWITCH_ON); - powerSwitcher->sendSwitchCommand(switch1, PowerSwitchIF::SWITCH_ON); - } - } else { //clear - if ((internalState == STATE_ON) || (internalState == STATE_FAULTY) - || (internalState == STATE_EXTERNAL_CONTROL)) { - internalState = STATE_WAIT_FOR_SWITCHES_OFF; - switchCountdown.setTimeout(powerSwitcher->getSwitchDelayMs()); - powerSwitcher->sendSwitchCommand(switch0, - PowerSwitchIF::SWITCH_OFF); - powerSwitcher->sendSwitchCommand(switch1, - PowerSwitchIF::SWITCH_OFF); - } - } + //only act if we are not in the right state or in a transition + if (action == SET) { + if ((internalState == STATE_OFF) || (internalState == STATE_PASSIVE) + || (internalState == STATE_EXTERNAL_CONTROL)) { + switchCountdown.setTimeout(powerSwitcher->getSwitchDelayMs()); + internalState = STATE_WAIT_FOR_SWITCHES_ON; + powerSwitcher->sendSwitchCommand(switch0, PowerSwitchIF::SWITCH_ON); + powerSwitcher->sendSwitchCommand(switch1, PowerSwitchIF::SWITCH_ON); + } + } else { //clear + if ((internalState == STATE_ON) || (internalState == STATE_FAULTY) + || (internalState == STATE_EXTERNAL_CONTROL)) { + internalState = STATE_WAIT_FOR_SWITCHES_OFF; + switchCountdown.setTimeout(powerSwitcher->getSwitchDelayMs()); + powerSwitcher->sendSwitchCommand(switch0, + PowerSwitchIF::SWITCH_OFF); + powerSwitcher->sendSwitchCommand(switch1, + PowerSwitchIF::SWITCH_OFF); + } + } } void Heater::setPowerSwitcher(PowerSwitchIF* powerSwitch) { - this->powerSwitcher = powerSwitch; + this->powerSwitcher = powerSwitch; } ReturnValue_t Heater::performOperation(uint8_t opCode) { - handleQueue(); - handleEventQueue(); + handleQueue(); + handleEventQueue(); - if (!healthHelper.healthTable->isFaulty(getObjectId())) { - reactedToBeingFaulty = false; - } + if (!healthHelper.healthTable->isFaulty(getObjectId())) { + reactedToBeingFaulty = false; + } - switch (internalState) { - case STATE_ON: - if ((powerSwitcher->getSwitchState(switch0) == PowerSwitchIF::SWITCH_OFF) - || (powerSwitcher->getSwitchState(switch1) - == PowerSwitchIF::SWITCH_OFF)) { - //switch went off on its own - //trigger event. FDIR can confirm if it is caused by MniOps and decide on the action - //do not trigger FD events when under external control - if (healthHelper.getHealth() != EXTERNAL_CONTROL) { - triggerEvent(PowerSwitchIF::SWITCH_WENT_OFF); - } else { - internalState = STATE_EXTERNAL_CONTROL; - } - } - break; - case STATE_OFF: - //check if heater is on, ie both switches are on - //if so, just command it to off, to resolve the situation or force a switch stayed on event - //But, only do anything if not already faulty (state off is the stable point for being faulty) - if ((!healthHelper.healthTable->isFaulty(getObjectId())) - && (powerSwitcher->getSwitchState(switch0) - == PowerSwitchIF::SWITCH_ON) - && (powerSwitcher->getSwitchState(switch1) - == PowerSwitchIF::SWITCH_ON)) { - //do not trigger FD events when under external control - if (healthHelper.getHealth() != EXTERNAL_CONTROL) { - internalState = STATE_WAIT_FOR_SWITCHES_OFF; - switchCountdown.setTimeout(powerSwitcher->getSwitchDelayMs()); - powerSwitcher->sendSwitchCommand(switch0, - PowerSwitchIF::SWITCH_OFF); - powerSwitcher->sendSwitchCommand(switch1, - PowerSwitchIF::SWITCH_OFF); - } else { - internalState = STATE_EXTERNAL_CONTROL; - } - } - break; - case STATE_PASSIVE: - break; - case STATE_WAIT_FOR_SWITCHES_ON: - if (switchCountdown.hasTimedOut()) { - if ((powerSwitcher->getSwitchState(switch0) - == PowerSwitchIF::SWITCH_OFF) - || (powerSwitcher->getSwitchState(switch1) - == PowerSwitchIF::SWITCH_OFF)) { - triggerEvent(HEATER_STAYED_OFF); - internalState = STATE_WAIT_FOR_FDIR; //wait before retrying or anything - } else { - triggerEvent(HEATER_ON); - internalState = STATE_ON; - } - } - break; - case STATE_WAIT_FOR_SWITCHES_OFF: - if (switchCountdown.hasTimedOut()) { - //only check for both being on (ie heater still on) - if ((powerSwitcher->getSwitchState(switch0) - == PowerSwitchIF::SWITCH_ON) - && (powerSwitcher->getSwitchState(switch1) - == PowerSwitchIF::SWITCH_ON)) { - if (healthHelper.healthTable->isFaulty(getObjectId())) { - if (passive) { - internalState = STATE_PASSIVE; - } else { - internalState = STATE_OFF; //just accept it - } - triggerEvent(HEATER_ON); //but throw an event to make it more visible - break; - } - triggerEvent(HEATER_STAYED_ON); - internalState = STATE_WAIT_FOR_FDIR; //wait before retrying or anything - } else { - triggerEvent(HEATER_OFF); - if (passive) { - internalState = STATE_PASSIVE; - } else { - internalState = STATE_OFF; - } - } - } - break; - default: - break; - } + switch (internalState) { + case STATE_ON: + if ((powerSwitcher->getSwitchState(switch0) == PowerSwitchIF::SWITCH_OFF) + || (powerSwitcher->getSwitchState(switch1) + == PowerSwitchIF::SWITCH_OFF)) { + //switch went off on its own + //trigger event. FDIR can confirm if it is caused by MniOps and decide on the action + //do not trigger FD events when under external control + if (healthHelper.getHealth() != EXTERNAL_CONTROL) { + triggerEvent(PowerSwitchIF::SWITCH_WENT_OFF); + } else { + internalState = STATE_EXTERNAL_CONTROL; + } + } + break; + case STATE_OFF: + //check if heater is on, ie both switches are on + //if so, just command it to off, to resolve the situation or force a switch stayed on event + //But, only do anything if not already faulty (state off is the stable point for being faulty) + if ((!healthHelper.healthTable->isFaulty(getObjectId())) + && (powerSwitcher->getSwitchState(switch0) + == PowerSwitchIF::SWITCH_ON) + && (powerSwitcher->getSwitchState(switch1) + == PowerSwitchIF::SWITCH_ON)) { + //do not trigger FD events when under external control + if (healthHelper.getHealth() != EXTERNAL_CONTROL) { + internalState = STATE_WAIT_FOR_SWITCHES_OFF; + switchCountdown.setTimeout(powerSwitcher->getSwitchDelayMs()); + powerSwitcher->sendSwitchCommand(switch0, + PowerSwitchIF::SWITCH_OFF); + powerSwitcher->sendSwitchCommand(switch1, + PowerSwitchIF::SWITCH_OFF); + } else { + internalState = STATE_EXTERNAL_CONTROL; + } + } + break; + case STATE_PASSIVE: + break; + case STATE_WAIT_FOR_SWITCHES_ON: + if (switchCountdown.hasTimedOut()) { + if ((powerSwitcher->getSwitchState(switch0) + == PowerSwitchIF::SWITCH_OFF) + || (powerSwitcher->getSwitchState(switch1) + == PowerSwitchIF::SWITCH_OFF)) { + triggerEvent(HEATER_STAYED_OFF); + internalState = STATE_WAIT_FOR_FDIR; //wait before retrying or anything + } else { + triggerEvent(HEATER_ON); + internalState = STATE_ON; + } + } + break; + case STATE_WAIT_FOR_SWITCHES_OFF: + if (switchCountdown.hasTimedOut()) { + //only check for both being on (ie heater still on) + if ((powerSwitcher->getSwitchState(switch0) + == PowerSwitchIF::SWITCH_ON) + && (powerSwitcher->getSwitchState(switch1) + == PowerSwitchIF::SWITCH_ON)) { + if (healthHelper.healthTable->isFaulty(getObjectId())) { + if (passive) { + internalState = STATE_PASSIVE; + } else { + internalState = STATE_OFF; //just accept it + } + triggerEvent(HEATER_ON); //but throw an event to make it more visible + break; + } + triggerEvent(HEATER_STAYED_ON); + internalState = STATE_WAIT_FOR_FDIR; //wait before retrying or anything + } else { + triggerEvent(HEATER_OFF); + if (passive) { + internalState = STATE_PASSIVE; + } else { + internalState = STATE_OFF; + } + } + } + break; + default: + break; + } - if ((powerSwitcher->getSwitchState(switch0) == PowerSwitchIF::SWITCH_ON) - && (powerSwitcher->getSwitchState(switch1) - == PowerSwitchIF::SWITCH_ON)) { - if (wasOn) { - if (heaterOnCountdown.hasTimedOut()) { - //SHOULDDO this means if a heater fails in single mode, the timeout will start again - //I am not sure if this is a bug, but atm I have no idea how to fix this and think - //it will be ok. whatcouldpossiblygowrong™ - if (!timedOut) { - triggerEvent(HEATER_TIMEOUT); - timedOut = true; - } - } - } else { - wasOn = true; - heaterOnCountdown.resetTimer(); - timedOut = false; - } - } else { - wasOn = false; - } + if ((powerSwitcher->getSwitchState(switch0) == PowerSwitchIF::SWITCH_ON) + && (powerSwitcher->getSwitchState(switch1) + == PowerSwitchIF::SWITCH_ON)) { + if (wasOn) { + if (heaterOnCountdown.hasTimedOut()) { + //SHOULDDO this means if a heater fails in single mode, the timeout will start again + //I am not sure if this is a bug, but atm I have no idea how to fix this and think + //it will be ok. whatcouldpossiblygowrong™ + if (!timedOut) { + triggerEvent(HEATER_TIMEOUT); + timedOut = true; + } + } + } else { + wasOn = true; + heaterOnCountdown.resetTimer(); + timedOut = false; + } + } else { + wasOn = false; + } - return HasReturnvaluesIF::RETURN_OK; + return HasReturnvaluesIF::RETURN_OK; } void Heater::setSwitch(uint8_t number, ReturnValue_t state, - uint32_t* uptimeOfSwitching) { - if (powerSwitcher == NULL) { - return; - } - if (powerSwitcher->getSwitchState(number) == state) { - *uptimeOfSwitching = INVALID_UPTIME; - } else { - if ((*uptimeOfSwitching == INVALID_UPTIME)) { - powerSwitcher->sendSwitchCommand(number, state); - Clock::getUptime(uptimeOfSwitching); - } else { - uint32_t currentUptime; - Clock::getUptime(¤tUptime); - if (currentUptime - *uptimeOfSwitching - > powerSwitcher->getSwitchDelayMs()) { - *uptimeOfSwitching = INVALID_UPTIME; - if (healthHelper.healthTable->isHealthy(getObjectId())) { - if (state == PowerSwitchIF::SWITCH_ON) { - triggerEvent(HEATER_STAYED_OFF); - } else { - triggerEvent(HEATER_STAYED_ON); - } - } - //SHOULDDO MiniOps during switch timeout leads to a faulty switch - } - } - } + uint32_t* uptimeOfSwitching) { + if (powerSwitcher == NULL) { + return; + } + if (powerSwitcher->getSwitchState(number) == state) { + *uptimeOfSwitching = INVALID_UPTIME; + } else { + if ((*uptimeOfSwitching == INVALID_UPTIME)) { + powerSwitcher->sendSwitchCommand(number, state); + Clock::getUptime(uptimeOfSwitching); + } else { + uint32_t currentUptime; + Clock::getUptime(¤tUptime); + if (currentUptime - *uptimeOfSwitching + > powerSwitcher->getSwitchDelayMs()) { + *uptimeOfSwitching = INVALID_UPTIME; + if (healthHelper.healthTable->isHealthy(getObjectId())) { + if (state == PowerSwitchIF::SWITCH_ON) { + triggerEvent(HEATER_STAYED_OFF); + } else { + triggerEvent(HEATER_STAYED_ON); + } + } + //SHOULDDO MiniOps during switch timeout leads to a faulty switch + } + } + } } MessageQueueId_t Heater::getCommandQueue() const { - return commandQueue->getId(); + return commandQueue->getId(); } ReturnValue_t Heater::initialize() { - ReturnValue_t result = SystemObject::initialize(); - if (result != HasReturnvaluesIF::RETURN_OK) { - return result; - } + ReturnValue_t result = SystemObject::initialize(); + if (result != HasReturnvaluesIF::RETURN_OK) { + return result; + } - EventManagerIF* manager = objectManager->get( - objects::EVENT_MANAGER); - if (manager == NULL) { - return HasReturnvaluesIF::RETURN_FAILED; - } - result = manager->registerListener(eventQueue->getId()); - if (result != HasReturnvaluesIF::RETURN_OK) { - return result; - } + EventManagerIF* manager = objectManager->get( + objects::EVENT_MANAGER); + if (manager == NULL) { + return HasReturnvaluesIF::RETURN_FAILED; + } + result = manager->registerListener(eventQueue->getId()); + if (result != HasReturnvaluesIF::RETURN_OK) { + return result; + } - ConfirmsFailuresIF* pcdu = objectManager->get( - DeviceHandlerFailureIsolation::powerConfirmationId); - if (pcdu == NULL) { - return HasReturnvaluesIF::RETURN_FAILED; - } - pcduQueueId = pcdu->getEventReceptionQueue(); + ConfirmsFailuresIF* pcdu = objectManager->get( + DeviceHandlerFailureIsolation::powerConfirmationId); + if (pcdu == NULL) { + return HasReturnvaluesIF::RETURN_FAILED; + } + pcduQueueId = pcdu->getEventReceptionQueue(); - result = manager->subscribeToAllEventsFrom(eventQueue->getId(), - getObjectId()); - if (result != HasReturnvaluesIF::RETURN_OK) { - return result; - } + result = manager->subscribeToAllEventsFrom(eventQueue->getId(), + getObjectId()); + if (result != HasReturnvaluesIF::RETURN_OK) { + return result; + } - result = parameterHelper.initialize(); - if (result != HasReturnvaluesIF::RETURN_OK) { - return result; - } + result = parameterHelper.initialize(); + if (result != HasReturnvaluesIF::RETURN_OK) { + return result; + } - result = healthHelper.initialize(); - if (result != HasReturnvaluesIF::RETURN_OK) { - return result; - } + result = healthHelper.initialize(); + if (result != HasReturnvaluesIF::RETURN_OK) { + return result; + } - return HasReturnvaluesIF::RETURN_OK; + return HasReturnvaluesIF::RETURN_OK; } void Heater::handleQueue() { - CommandMessage command; - ReturnValue_t result = commandQueue->receiveMessage(&command); - if (result == HasReturnvaluesIF::RETURN_OK) { - result = healthHelper.handleHealthCommand(&command); - if (result == HasReturnvaluesIF::RETURN_OK) { - return; - } - parameterHelper.handleParameterMessage(&command); - } + CommandMessage command; + ReturnValue_t result = commandQueue->receiveMessage(&command); + if (result == HasReturnvaluesIF::RETURN_OK) { + result = healthHelper.handleHealthCommand(&command); + if (result == HasReturnvaluesIF::RETURN_OK) { + return; + } + result = parameterHelper.handleParameterMessage(&command); + if (result == HasReturnvaluesIF::RETURN_OK) { + return; + } + } } ReturnValue_t Heater::getParameter(uint8_t domainId, uint8_t uniqueId, ParameterWrapper* parameterWrapper, const ParameterWrapper* newValues, - uint16_t startAtIndex) { - if (domainId != DOMAIN_ID_BASE) { - return INVALID_DOMAIN_ID; - } - switch (uniqueId) { - case 0: - parameterWrapper->set(heaterOnCountdown.timeout); - break; - default: - return INVALID_IDENTIFIER_ID; - } - return HasReturnvaluesIF::RETURN_OK; + uint16_t startAtIndex) { + if (domainId != DOMAIN_ID_BASE) { + return INVALID_DOMAIN_ID; + } + switch (uniqueId) { + case 0: + parameterWrapper->set(heaterOnCountdown.timeout); + break; + default: + return INVALID_IDENTIFIER_ID; + } + return HasReturnvaluesIF::RETURN_OK; } void Heater::handleEventQueue() { - EventMessage event; - for (ReturnValue_t result = eventQueue->receiveMessage(&event); - result == HasReturnvaluesIF::RETURN_OK; - result = eventQueue->receiveMessage(&event)) { - switch (event.getMessageId()) { - case EventMessage::EVENT_MESSAGE: - switch (event.getEvent()) { - case Fuse::FUSE_WENT_OFF: - case HEATER_STAYED_OFF: - case HEATER_STAYED_ON://Setting it faulty does not help, but we need to reach a stable state and can check for being faulty before throwing this event again. - if (healthHelper.healthTable->isCommandable(getObjectId())) { - healthHelper.setHealth(HasHealthIF::FAULTY); - internalState = STATE_FAULTY; - } - break; - case PowerSwitchIF::SWITCH_WENT_OFF: - internalState = STATE_WAIT; - event.setMessageId(EventMessage::CONFIRMATION_REQUEST); - if (pcduQueueId != 0) { - eventQueue->sendMessage(pcduQueueId, &event); - } else { - healthHelper.setHealth(HasHealthIF::FAULTY); - internalState = STATE_FAULTY; - } - break; - default: - return; - } - break; - case EventMessage::YOUR_FAULT: - healthHelper.setHealth(HasHealthIF::FAULTY); - internalState = STATE_FAULTY; - break; - case EventMessage::MY_FAULT: - //do nothing, we are already in STATE_WAIT and wait for a clear() - break; - default: - return; - } - } + EventMessage event; + for (ReturnValue_t result = eventQueue->receiveMessage(&event); + result == HasReturnvaluesIF::RETURN_OK; + result = eventQueue->receiveMessage(&event)) { + switch (event.getMessageId()) { + case EventMessage::EVENT_MESSAGE: + switch (event.getEvent()) { + case Fuse::FUSE_WENT_OFF: + case HEATER_STAYED_OFF: + // Setting it faulty does not help, but we need to reach a stable state and can check + // for being faulty before throwing this event again. + case HEATER_STAYED_ON: + if (healthHelper.healthTable->isCommandable(getObjectId())) { + healthHelper.setHealth(HasHealthIF::FAULTY); + internalState = STATE_FAULTY; + } + break; + case PowerSwitchIF::SWITCH_WENT_OFF: + internalState = STATE_WAIT; + event.setMessageId(EventMessage::CONFIRMATION_REQUEST); + if (pcduQueueId != 0) { + eventQueue->sendMessage(pcduQueueId, &event); + } else { + healthHelper.setHealth(HasHealthIF::FAULTY); + internalState = STATE_FAULTY; + } + break; + default: + return; + } + break; + case EventMessage::YOUR_FAULT: + healthHelper.setHealth(HasHealthIF::FAULTY); + internalState = STATE_FAULTY; + break; + case EventMessage::MY_FAULT: + //do nothing, we are already in STATE_WAIT and wait for a clear() + break; + default: + return; + } + } } diff --git a/thermal/Heater.h b/thermal/Heater.h index f83957a2e..19f13c793 100644 --- a/thermal/Heater.h +++ b/thermal/Heater.h @@ -1,90 +1,93 @@ -#ifndef FRAMEWORK_THERMAL_HEATER_H_ -#define FRAMEWORK_THERMAL_HEATER_H_ +#ifndef FSFW_THERMAL_HEATER_H_ +#define FSFW_THERMAL_HEATER_H_ #include "../devicehandlers/HealthDevice.h" #include "../parameters/ParameterHelper.h" #include "../power/PowerSwitchIF.h" #include "../returnvalues/HasReturnvaluesIF.h" #include "../timemanager/Countdown.h" -#include -//class RedundantHeater; +#include + class Heater: public HealthDevice, public ReceivesParameterMessagesIF { - friend class RedundantHeater; + friend class RedundantHeater; public: - static const uint8_t SUBSYSTEM_ID = SUBSYSTEM_ID::HEATER; - static const Event HEATER_ON = MAKE_EVENT(0, severity::INFO); - static const Event HEATER_OFF = MAKE_EVENT(1, severity::INFO); - static const Event HEATER_TIMEOUT = MAKE_EVENT(2, severity::LOW); - static const Event HEATER_STAYED_ON = MAKE_EVENT(3, severity::LOW); - static const Event HEATER_STAYED_OFF = MAKE_EVENT(4, severity::LOW); + static const uint8_t SUBSYSTEM_ID = SUBSYSTEM_ID::HEATER; + static const Event HEATER_ON = MAKE_EVENT(0, severity::INFO); + static const Event HEATER_OFF = MAKE_EVENT(1, severity::INFO); + static const Event HEATER_TIMEOUT = MAKE_EVENT(2, severity::LOW); + static const Event HEATER_STAYED_ON = MAKE_EVENT(3, severity::LOW); + static const Event HEATER_STAYED_OFF = MAKE_EVENT(4, severity::LOW); - Heater(uint32_t objectId, uint8_t switch0, uint8_t switch1); - virtual ~Heater(); + Heater(uint32_t objectId, uint8_t switch0, uint8_t switch1); + virtual ~Heater(); - ReturnValue_t performOperation(uint8_t opCode); + ReturnValue_t performOperation(uint8_t opCode); - ReturnValue_t initialize(); + ReturnValue_t initialize(); - ReturnValue_t set(); - void clear(bool passive); + ReturnValue_t set(); + void clear(bool passive); - void setPowerSwitcher(PowerSwitchIF *powerSwitch); + void setPowerSwitcher(PowerSwitchIF *powerSwitch); - MessageQueueId_t getCommandQueue() const; + MessageQueueId_t getCommandQueue() const; - ReturnValue_t getParameter(uint8_t domainId, uint8_t uniqueId, - ParameterWrapper *parameterWrapper, - const ParameterWrapper *newValues, uint16_t startAtIndex); + ReturnValue_t getParameter(uint8_t domainId, uint8_t uniqueId, + ParameterWrapper *parameterWrapper, + const ParameterWrapper *newValues, uint16_t startAtIndex); protected: - static const uint32_t INVALID_UPTIME = 0; + static const uint32_t INVALID_UPTIME = 0; - enum InternalState { - STATE_ON, - STATE_OFF, - STATE_PASSIVE, - STATE_WAIT_FOR_SWITCHES_ON, - STATE_WAIT_FOR_SWITCHES_OFF, - STATE_WAIT_FOR_FDIR, //used to avoid doing anything until fdir decided what to do - STATE_FAULTY, - STATE_WAIT, //used when waiting for system to recover from miniops - STATE_EXTERNAL_CONTROL //entered when under external control and a fdir reaction would be triggered. This is useful when leaving external control into an unknown state - //if no fdir reaction is triggered under external control the state is still ok and no need for any special treatment is needed - } internalState; + enum InternalState { + STATE_ON, + STATE_OFF, + STATE_PASSIVE, + STATE_WAIT_FOR_SWITCHES_ON, + STATE_WAIT_FOR_SWITCHES_OFF, + STATE_WAIT_FOR_FDIR, // Used to avoid doing anything until fdir decided what to do + STATE_FAULTY, + STATE_WAIT, // Used when waiting for system to recover from miniops + // Entered when under external control and a fdir reaction would be triggered. + // This is useful when leaving external control into an unknown state + STATE_EXTERNAL_CONTROL + // If no fdir reaction is triggered under external control the state is still ok and + // no need for any special treatment is needed + } internalState; - PowerSwitchIF *powerSwitcher; - MessageQueueId_t pcduQueueId; + PowerSwitchIF *powerSwitcher; + MessageQueueId_t pcduQueueId; - uint8_t switch0; - uint8_t switch1; + uint8_t switch0; + uint8_t switch1; - bool wasOn; + bool wasOn; - bool timedOut; + bool timedOut; - bool reactedToBeingFaulty; + bool reactedToBeingFaulty; - bool passive; + bool passive; - MessageQueueIF* eventQueue; - Countdown heaterOnCountdown; - Countdown switchCountdown; - ParameterHelper parameterHelper; + MessageQueueIF* eventQueue; + Countdown heaterOnCountdown; + Countdown switchCountdown; + ParameterHelper parameterHelper; - enum Action { - SET, CLEAR - } lastAction; + enum Action { + SET, CLEAR + } lastAction; - void doAction(Action action); + void doAction(Action action); - void setSwitch(uint8_t number, ReturnValue_t state, - uint32_t *upTimeOfSwitching); + void setSwitch(uint8_t number, ReturnValue_t state, + uint32_t *upTimeOfSwitching); - void handleQueue(); + void handleQueue(); - void handleEventQueue(); + void handleEventQueue(); }; -#endif /* FRAMEWORK_THERMAL_HEATER_H_ */ +#endif /* FSFW_THERMAL_HEATER_H_ */ From 864621ee37892cef944e1384d6fde38aa5770e1d Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 20 Apr 2021 16:15:30 +0200 Subject: [PATCH 11/17] small fix for linux printout --- osal/linux/tcpipHelpers.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osal/linux/tcpipHelpers.cpp b/osal/linux/tcpipHelpers.cpp index 3e8f60092..d7c644ecb 100644 --- a/osal/linux/tcpipHelpers.cpp +++ b/osal/linux/tcpipHelpers.cpp @@ -99,8 +99,8 @@ void tcpip::handleError(Protocol protocol, ErrorSources errorSrc, dur_millis_t s sif::warning << "tcpip::handleError: " << protocolString << " | " << errorSrcString << " | " << infoString << std::endl; #else - sif::printWarning("tcpip::handleError: %s | %s | %s\n", protocolString, - errorSrcString, infoString); + sif::printWarning("tcpip::handleError: %s | %s | %s\n", protocolString.c_str(), + errorSrcString.c_str(), infoString.c_str()); #endif /* FSFW_CPP_OSTREAM_ENABLED == 1 */ if(sleepDuration > 0) { From a2ba3181b91a5f3657ee8f72b7638cadc27864a4 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 20 Apr 2021 16:16:35 +0200 Subject: [PATCH 12/17] small coverity tweak --- devicehandlers/HealthDevice.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/devicehandlers/HealthDevice.cpp b/devicehandlers/HealthDevice.cpp index 418ed2576..e23dd5b69 100644 --- a/devicehandlers/HealthDevice.cpp +++ b/devicehandlers/HealthDevice.cpp @@ -16,9 +16,9 @@ ReturnValue_t HealthDevice::performOperation(uint8_t opCode) { CommandMessage command; ReturnValue_t result = commandQueue->receiveMessage(&command); if (result == HasReturnvaluesIF::RETURN_OK) { - healthHelper.handleHealthCommand(&command); + result = healthHelper.handleHealthCommand(&command); } - return HasReturnvaluesIF::RETURN_OK; + return result; } ReturnValue_t HealthDevice::initialize() { From 0055d34d9a156f805edf63e9d5970562b372755a Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 20 Apr 2021 16:17:37 +0200 Subject: [PATCH 13/17] bugfix found by coverity --- timemanager/TimeMessage.cpp | 2 +- timemanager/TimeMessage.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/timemanager/TimeMessage.cpp b/timemanager/TimeMessage.cpp index a1042efe5..66aea0f4d 100644 --- a/timemanager/TimeMessage.cpp +++ b/timemanager/TimeMessage.cpp @@ -25,6 +25,6 @@ uint32_t TimeMessage::getCounterValue() { return temp; } -size_t TimeMessage::getMinimumMessageSize() { +size_t TimeMessage::getMinimumMessageSize() const { return this->MAX_SIZE; } diff --git a/timemanager/TimeMessage.h b/timemanager/TimeMessage.h index f5ac3e14d..00778fb7f 100644 --- a/timemanager/TimeMessage.h +++ b/timemanager/TimeMessage.h @@ -11,7 +11,7 @@ protected: * @brief This call always returns the same fixed size of the message. * @return Returns HEADER_SIZE + \c sizeof(timeval) + sizeof(uint32_t). */ - size_t getMinimumMessageSize(); + size_t getMinimumMessageSize() const override; public: /** From 4fb792447e037cabbd4800f40e4fabeb44d6c143 Mon Sep 17 00:00:00 2001 From: Steffen Gaisser Date: Tue, 20 Apr 2021 17:29:56 +0200 Subject: [PATCH 14/17] Small rearragenment in Heater.cpp --- thermal/Heater.cpp | 7 +++---- thermal/Heater.h | 16 ++++++++-------- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/thermal/Heater.cpp b/thermal/Heater.cpp index 8bfa030b9..aeade8d1d 100644 --- a/thermal/Heater.cpp +++ b/thermal/Heater.cpp @@ -5,10 +5,9 @@ #include "../ipc/QueueFactory.h" Heater::Heater(uint32_t objectId, uint8_t switch0, uint8_t switch1) : -HealthDevice(objectId, 0), internalState(STATE_OFF), powerSwitcher( - NULL), pcduQueueId(0), switch0(switch0), switch1(switch1), wasOn(false), timedOut(false), - reactedToBeingFaulty(false), passive(false), eventQueue(NULL), - heaterOnCountdown(10800000)/*about two orbits*/, parameterHelper(this), lastAction(CLEAR) { +HealthDevice(objectId, 0), internalState(STATE_OFF), switch0(switch0), switch1(switch1), + heaterOnCountdown(10800000)/*about two orbits*/, + parameterHelper(this) { eventQueue = QueueFactory::instance()->createMessageQueue(); } diff --git a/thermal/Heater.h b/thermal/Heater.h index 19f13c793..2caddd852 100644 --- a/thermal/Heater.h +++ b/thermal/Heater.h @@ -57,28 +57,28 @@ protected: // no need for any special treatment is needed } internalState; - PowerSwitchIF *powerSwitcher; - MessageQueueId_t pcduQueueId; + PowerSwitchIF *powerSwitcher = nullptr; + MessageQueueId_t pcduQueueId = MessageQueueIF::NO_QUEUE; uint8_t switch0; uint8_t switch1; - bool wasOn; + bool wasOn = false; - bool timedOut; + bool timedOut = false; - bool reactedToBeingFaulty; + bool reactedToBeingFaulty = false; - bool passive; + bool passive = false; - MessageQueueIF* eventQueue; + MessageQueueIF* eventQueue = nullptr; Countdown heaterOnCountdown; Countdown switchCountdown; ParameterHelper parameterHelper; enum Action { SET, CLEAR - } lastAction; + } lastAction = CLEAR; void doAction(Action action); From 629814bc9ba00112668d0edaa62e0690cb5ead98 Mon Sep 17 00:00:00 2001 From: Steffen Gaisser Date: Tue, 20 Apr 2021 17:35:28 +0200 Subject: [PATCH 15/17] Just comments --- thermal/Heater.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/thermal/Heater.cpp b/thermal/Heater.cpp index aeade8d1d..770494385 100644 --- a/thermal/Heater.cpp +++ b/thermal/Heater.cpp @@ -224,7 +224,6 @@ void Heater::setSwitch(uint8_t number, ReturnValue_t state, triggerEvent(HEATER_STAYED_ON); } } - //SHOULDDO MiniOps during switch timeout leads to a faulty switch } } } @@ -317,7 +316,7 @@ void Heater::handleEventQueue() { switch (event.getEvent()) { case Fuse::FUSE_WENT_OFF: case HEATER_STAYED_OFF: - // Setting it faulty does not help, but we need to reach a stable state and can check + // HEATER_STAYED_ON is a setting if faulty does not help, but we need to reach a stable state and can check // for being faulty before throwing this event again. case HEATER_STAYED_ON: if (healthHelper.healthTable->isCommandable(getObjectId())) { From 03ef63302b681651daefde2f17421dabda4c6945 Mon Sep 17 00:00:00 2001 From: Steffen Gaisser Date: Tue, 20 Apr 2021 20:45:15 +0200 Subject: [PATCH 16/17] Replaced Magic Numbers --- subsystem/SubsystemBase.cpp | 9 ++++----- subsystem/SubsystemBase.h | 4 ++-- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/subsystem/SubsystemBase.cpp b/subsystem/SubsystemBase.cpp index bcfa4b0e3..565e0712d 100644 --- a/subsystem/SubsystemBase.cpp +++ b/subsystem/SubsystemBase.cpp @@ -5,8 +5,7 @@ SubsystemBase::SubsystemBase(object_id_t setObjectId, object_id_t parent, Mode_t initialMode, uint16_t commandQueueDepth) : - SystemObject(setObjectId), mode(initialMode), submode(SUBMODE_NONE), - childrenChangedMode(false), + SystemObject(setObjectId), mode(initialMode), commandQueue(QueueFactory::instance()->createMessageQueue( commandQueueDepth, CommandMessage::MAX_MESSAGE_SIZE)), healthHelper(this, setObjectId), modeHelper(this), parentId(parent) { @@ -167,16 +166,16 @@ MessageQueueId_t SubsystemBase::getCommandQueue() const { } ReturnValue_t SubsystemBase::initialize() { - MessageQueueId_t parentQueue = 0; + MessageQueueId_t parentQueue = MessageQueueIF::NO_QUEUE; ReturnValue_t result = SystemObject::initialize(); if (result != RETURN_OK) { return result; } - if (parentId != 0) { + if (parentId != objects::NO_OBJECT) { SubsystemBase *parent = objectManager->get(parentId); - if (parent == NULL) { + if (parent == nullptr) { return RETURN_FAILED; } parentQueue = parent->getCommandQueue(); diff --git a/subsystem/SubsystemBase.h b/subsystem/SubsystemBase.h index b8e4f9029..5d107701d 100644 --- a/subsystem/SubsystemBase.h +++ b/subsystem/SubsystemBase.h @@ -56,9 +56,9 @@ protected: Mode_t mode; - Submode_t submode; + Submode_t submode = SUBMODE_NONE; - bool childrenChangedMode; + bool childrenChangedMode = false; /** * Always check this against <=0, so you are robust against too many replies From 06631d06a57c30946563d8726ea9ef9a7e6c1c84 Mon Sep 17 00:00:00 2001 From: Steffen Gaisser Date: Tue, 20 Apr 2021 22:01:56 +0200 Subject: [PATCH 17/17] Added comments in AssemblyBase and SubsystemBase --- devicehandlers/AssemblyBase.h | 22 +++++++++++++++++++--- subsystem/SubsystemBase.h | 11 +++++++++++ 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/devicehandlers/AssemblyBase.h b/devicehandlers/AssemblyBase.h index 353d5f899..6cac81b49 100644 --- a/devicehandlers/AssemblyBase.h +++ b/devicehandlers/AssemblyBase.h @@ -24,6 +24,13 @@ * 1. check logic when active-> checkChildrenStateOn * 2. transition logic to change the mode -> commandChildren * + * Important: + * + * The implementation must call registerChild(object_id_t child) + * for all commanded children during initialization. + * The implementation must call the initialization function of the base class. + * (This will call the function in SubsystemBase) + * */ class AssemblyBase: public SubsystemBase { public: @@ -41,9 +48,6 @@ public: virtual ~AssemblyBase(); protected: - - // SHOULDDO: Change that OVERWRITE_HEALTH may be returned - // (or return internalState directly?) /** * Command children to reach [mode,submode] combination * Can be done by setting #commandsOutstanding correctly, @@ -68,6 +72,18 @@ protected: virtual ReturnValue_t checkChildrenStateOn(Mode_t wantedMode, Submode_t wantedSubmode) = 0; + /** + * Check whether a combination of mode and submode is valid. + * + * Ground Controller like precise return values from HasModesIF. + * So, please return any of them. + * + * @param mode The targeted mode + * @param submode The targeted submmode + * @return Any information why this combination is invalid from HasModesIF + * like HasModesIF::INVALID_SUBMODE. + * On success return HasReturnvaluesIF::RETURN_OK + */ virtual ReturnValue_t isModeCombinationValid(Mode_t mode, Submode_t submode) = 0; diff --git a/subsystem/SubsystemBase.h b/subsystem/SubsystemBase.h index 5d107701d..6b2e9b5fd 100644 --- a/subsystem/SubsystemBase.h +++ b/subsystem/SubsystemBase.h @@ -37,6 +37,17 @@ public: virtual MessageQueueId_t getCommandQueue() const override; + /** + * Function to register the child objects. + * Performs a checks if the child does implement HasHealthIF and/or HasModesIF + * + * Also adds them to the internal childrenMap. + * + * @param objectId + * @return RETURN_OK if successful + * CHILD_DOESNT_HAVE_MODES if Child is no HasHealthIF and no HasModesIF + * COULD_NOT_INSERT_CHILD If the Child could not be added to the ChildrenMap + */ ReturnValue_t registerChild(object_id_t objectId); virtual ReturnValue_t initialize() override;