fsfw/fsfw
fsfw/fsfw
10
3
Fork 6
Code Issues 31 Pull Requests 4 Releases 10 Wiki Activity

Health Service Bugfix #617

Merged
gaisser merged 4 commits from mueller/health-srv-bugfix into development 2022-05-09 16:08:29 +02:00
Conversation 0 Commits 4 Files Changed 4 +14 -13
3 changed files with 14 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 4e4820af05 - Show all commits

15
src/fsfw/pus/CService201HealthCommanding.cpp
View File

@ -13,7 +13,7 @@ CService201HealthCommanding::CService201HealthCommanding(object_id_t objectId, u
: CommandingServiceBase(objectId, apid, serviceId, numParallelCommands, commandTimeoutSeconds) { : CommandingServiceBase(objectId, apid, serviceId, numParallelCommands, commandTimeoutSeconds) {
} }
CService201HealthCommanding::~CService201HealthCommanding() {} CService201HealthCommanding::~CService201HealthCommanding() = default;
gaisser marked this conversation as resolved Outdated
gaisser commented 2022-05-09 15:36:29 +02:00
Outdated
Review
Copy Link

I would move the default to the header and remove the line here but I don't know if this is good practice.

I would move the default to the header and remove the line here but I don't know if this is good practice.
muellerr commented 2022-05-09 15:39:23 +02:00
Outdated
Review
Copy Link

Yeah, I can do that too and I have no idea what the best practice is. I think there is none. If i rewrote a header, I would do it there in the future.

Yeah, I can do that too and I have no idea what the best practice is. I think there is none. If i rewrote a header, I would do it there in the future.
muellerr commented 2022-05-09 15:41:24 +02:00
Outdated
Review
Copy Link

okay, moved to header

okay, moved to header
ReturnValue_t CService201HealthCommanding::isValidSubservice(uint8_t subservice) { ReturnValue_t CService201HealthCommanding::isValidSubservice(uint8_t subservice) {
switch (subservice) { switch (subservice) {
@ -43,8 +43,8 @@ ReturnValue_t CService201HealthCommanding::getMessageQueueAndObject(uint8_t subs
} }
ReturnValue_t CService201HealthCommanding::checkInterfaceAndAcquireMessageQueue( ReturnValue_t CService201HealthCommanding::checkInterfaceAndAcquireMessageQueue(
MessageQueueId_t *messageQueueToSet, object_id_t *objectId) { MessageQueueId_t *messageQueueToSet, const object_id_t *objectId) {
HasHealthIF *destination = ObjectManager::instance()->get<HasHealthIF>(*objectId); auto *destination = ObjectManager::instance()->get<HasHealthIF>(*objectId);
if (destination == nullptr) { if (destination == nullptr) {
return CommandingServiceBase::INVALID_OBJECT; return CommandingServiceBase::INVALID_OBJECT;
} }
@ -77,6 +77,10 @@ ReturnValue_t CService201HealthCommanding::prepareCommand(CommandMessage *messag
HealthMessage::setHealthMessage(message, HealthMessage::HEALTH_ANNOUNCE_ALL); HealthMessage::setHealthMessage(message, HealthMessage::HEALTH_ANNOUNCE_ALL);
break; break;
} }
default: {
// Should never happen, subservice was already checked
result = RETURN_FAILED;
}
} }
return result; return result;
} }
@ -96,9 +100,8 @@ ReturnValue_t CService201HealthCommanding::handleReply(const CommandMessage *rep
// Not used for now, health state already reported by event // Not used for now, health state already reported by event
ReturnValue_t CService201HealthCommanding::prepareHealthSetReply(const CommandMessage *reply) { ReturnValue_t CService201HealthCommanding::prepareHealthSetReply(const CommandMessage *reply) {
prepareHealthSetReply(reply); auto health = static_cast<uint8_t>(HealthMessage::getHealth(reply));
uint8_t health = static_cast<uint8_t>(HealthMessage::getHealth(reply)); auto oldHealth = static_cast<uint8_t>(HealthMessage::getOldHealth(reply));
uint8_t oldHealth = static_cast<uint8_t>(HealthMessage::getOldHealth(reply));
HealthSetReply healthSetReply(health, oldHealth); HealthSetReply healthSetReply(health, oldHealth);
return sendTmPacket(Subservice::REPLY_HEALTH_SET, &healthSetReply); return sendTmPacket(Subservice::REPLY_HEALTH_SET, &healthSetReply);
} }

10
src/fsfw/pus/CService201HealthCommanding.h
View File

@ -1,7 +1,7 @@
#ifndef FSFW_PUS_CSERVICE201HEALTHCOMMANDING_H_ #ifndef FSFW_PUS_CSERVICE201HEALTHCOMMANDING_H_
#define FSFW_PUS_CSERVICE201HEALTHCOMMANDING_H_ #define FSFW_PUS_CSERVICE201HEALTHCOMMANDING_H_
#include "../tmtcservices/CommandingServiceBase.h" #include "fsfw/tmtcservices/CommandingServiceBase.h"
/** /**
* @brief Custom PUS service to set health of all objects * @brief Custom PUS service to set health of all objects
@ -21,7 +21,7 @@ class CService201HealthCommanding : public CommandingServiceBase {
public: public:
CService201HealthCommanding(object_id_t objectId, uint16_t apid, uint8_t serviceId, CService201HealthCommanding(object_id_t objectId, uint16_t apid, uint8_t serviceId,
uint8_t numParallelCommands = 4, uint16_t commandTimeoutSeconds = 60); uint8_t numParallelCommands = 4, uint16_t commandTimeoutSeconds = 60);
virtual ~CService201HealthCommanding(); ~CService201HealthCommanding() override;
protected: protected:
/* CSB abstract function implementations */ /* CSB abstract function implementations */
@ -38,10 +38,8 @@ class CService201HealthCommanding : public CommandingServiceBase {
bool *isStep) override; bool *isStep) override;
private: private:
ReturnValue_t checkAndAcquireTargetID(object_id_t *objectIdToSet, const uint8_t *tcData, static ReturnValue_t checkInterfaceAndAcquireMessageQueue(MessageQueueId_t *MessageQueueToSet,
size_t tcDataLen); const object_id_t *objectId);
ReturnValue_t checkInterfaceAndAcquireMessageQueue(MessageQueueId_t *MessageQueueToSet,
object_id_t *objectId);
ReturnValue_t prepareHealthSetReply(const CommandMessage *reply); ReturnValue_t prepareHealthSetReply(const CommandMessage *reply);
gaisser marked this conversation as resolved
gaisser commented 2022-05-09 13:29:16 +02:00
Review
Copy Link

[[maybe_unused]] is a c++17 feature

``[[maybe_unused]]`` is a c++17 feature
muellerr commented 2022-05-09 13:50:05 +02:00
Review
Copy Link

This is unfortunate. I can't check this if I am using a more recent compiler. WE should consider upgrading the required version to C++17. Every project using the FSFW so far supports it anyway.

This is unfortunate. I can't check this if I am using a more recent compiler. WE should consider upgrading the required version to C++17. Every project using the FSFW so far supports it anyway.

2
src/fsfw/tmtcservices/CommandingServiceBase.h
View File

@ -166,7 +166,7 @@ class CommandingServiceBase : public SystemObject,
* @param objectId Target object ID * @param objectId Target object ID
* @return * @return
* - @c RETURN_OK to generate a verification start message * - @c RETURN_OK to generate a verification start message
* - @c EXECUTION_COMPELTE Fire-and-forget command. Generate a completion * - @c EXECUTION_COMPLETE Fire-and-forget command. Generate a completion
* verification message. * verification message.
* - @c Anything else rejects the packets and generates a start failure * - @c Anything else rejects the packets and generates a start failure
* verification. * verification.