Health Service Bugfix #617

Merged
gaisser merged 4 commits from mueller/health-srv-bugfix into development 2022-05-09 16:08:29 +02:00
Owner
  • Found this bug using clang-tidy. There was an infinite recursion in the prepareHealthSetReply function
    which is currently unused. Also includes all other clang-tidy suggestions
    which made sense.
- Found this bug using `clang-tidy`. There was an infinite recursion in the `prepareHealthSetReply` function which is currently unused. Also includes all other `clang-tidy` suggestions which made sense.
muellerr added 2 commits 2022-05-09 10:49:15 +02:00
mark unused function
Some checks are pending
fsfw/fsfw/pipeline/head Build started...
fsfw/fsfw/pipeline/pr-development This commit looks good
e5e163bdbf
muellerr added 1 commit 2022-05-09 10:50:36 +02:00
update changelog
Some checks failed
fsfw/fsfw/pipeline/pr-development There was a failure building this commit
79f17843d8
muellerr added the
bug
label 2022-05-09 10:50:49 +02:00
muellerr added this to the v5.0.0 milestone 2022-05-09 11:01:51 +02:00
gaisser reviewed 2022-05-09 13:29:16 +02:00
@ -45,2 +42,3 @@
const object_id_t *objectId);
ReturnValue_t prepareHealthSetReply(const CommandMessage *reply);
[[maybe_unused]] ReturnValue_t prepareHealthSetReply(const CommandMessage *reply);
Owner

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

``[[maybe_unused]]`` is a c++17 feature
Author
Owner

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.
gaisser marked this conversation as resolved
gaisser reviewed 2022-05-09 15:36:29 +02:00
@ -14,3 +14,3 @@
}
CService201HealthCommanding::~CService201HealthCommanding() {}
CService201HealthCommanding::~CService201HealthCommanding() = default;
Owner

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.
Author
Owner

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.
Author
Owner

okay, moved to header

okay, moved to header
gaisser marked this conversation as resolved
muellerr added 1 commit 2022-05-09 15:41:13 +02:00
Merge remote-tracking branch 'origin/development' into mueller/health-srv-bugfix
All checks were successful
fsfw/fsfw/pipeline/pr-development This commit looks good
281f91ec5d
gaisser approved these changes 2022-05-09 16:06:52 +02:00
gaisser left a comment
Owner

LGTM

LGTM
gaisser merged commit 46cfe65321 into development 2022-05-09 16:08:29 +02:00
gaisser deleted branch mueller/health-srv-bugfix 2022-05-09 16:08:30 +02:00
Sign in to join this conversation.
No description provided.