eive/fsfw
15
1 Fork 0
forked from fsfw/fsfw
Code Issues Pull Requests Releases 1 Wiki Activity

Feature: Allowed Submodes Mask for Mode List Entry #130

Merged
muellerr merged 5 commits from feature_allow_submodes_mode_list_entry into develop 2023-03-07 17:20:48 +01:00
Conversation 2 Commits 5 Files Changed 5
+156 -81
muellerr commented 2023-03-07 16:11:29 +01:00
Owner
Copy link
Copy source
  • Also refactor the mode list entry to use the serial linked list adapter
- Also refactor the mode list entry to use the serial linked list adapter
muellerr added 2 commits 2023-03-07 16:11:31 +01:00
allow submode mask now 2745b2080d
add copy and assignment ctor for mode definition 4c48668125
muellerr requested review from gaisser 2023-03-07 16:19:29 +01:00
muellerr added 1 commit 2023-03-07 16:29:17 +01:00
bugfix in submode check logic af58c414fc
muellerr added 1 commit 2023-03-07 16:31:32 +01:00
Merge remote-tracking branch 'origin/develop' into feature_allow_submodes_mode_list_entry d9a139e1ef
gaisser left a comment
Member
Copy link
Copy source

See comments

See comments
src/fsfw/subsystem/SubsystemBase.cpp Outdated
@@ -24,2 +24,4 @@
using namespace mode;
std::map<object_id_t, ChildInfo>::iterator childIter;
auto checkSubmode = [&]() {
gaisser commented 2023-03-07 16:44:14 +01:00
Member
Copy link
Copy source

If you have to use a lambda, which I find a nightmare to read, maybe capture &childIter explicitly.

If you have to use a lambda, which I find a nightmare to read, maybe capture &childIter explicitly.
gaisser marked this conversation as resolved
src/fsfw/subsystem/SubsystemBase.cpp Outdated
@@ -26,0 +36,4 @@
return returnvalue::FAILED;
}
}
if (submodesAllowedMask) {
gaisser commented 2023-03-07 16:42:23 +01:00
Member
Copy link
Copy source

This can be the else case of the above. Or maybe switch cases (not XX might be harder to read):

    if (submodesAllowedMask) {
      if ((childIter->second.submode | mask) != mask) {
        return returnvalue::FAILED;
      }
    }else{
      if (childIter->second.submode != submodeToCheckAgainst) {
        return returnvalue::FAILED;
      }
    }
    return returnvalue::OK;
This can be the else case of the above. Or maybe switch cases (not XX might be harder to read): ``` c++ if (submodesAllowedMask) { if ((childIter->second.submode | mask) != mask) { return returnvalue::FAILED; } }else{ if (childIter->second.submode != submodeToCheckAgainst) { return returnvalue::FAILED; } } return returnvalue::OK; ```
gaisser marked this conversation as resolved
src/fsfw/subsystem/modes/ModeDefinitions.h
@@ -109,0 +101,4 @@
/**
* Specialization of @enableSubmodeAllowed which allows all submodes.
*/
void allowAllSubmodes() { enableSubmodeAllowed(0xff); }
gaisser commented 2023-03-07 16:47:27 +01:00
Member
Copy link
Copy source

0xFF should be defined somewhere visible as a restricted mask.

0xFF should be defined somewhere visible as a restricted mask.
gaisser marked this conversation as resolved
muellerr added 1 commit 2023-03-07 17:15:41 +01:00
review improvements 070b48ada2
gaisser approved these changes 2023-03-07 17:19:36 +01:00
gaisser left a comment
Member
Copy link
Copy source

LGTM

LGTM
muellerr merged commit 7f6ba5f40b into develop 2023-03-07 17:20:48 +01:00
muellerr deleted branch feature_allow_submodes_mode_list_entry 2023-03-07 17:20:48 +01:00
Sign in to join this conversation.
Reviewers
No Reviewers
gaisser
Labels
Clear labels
api change
API change
 breaking api change
API change which can break existing code
 bug
Something is not working
 documentation
Documentation
 duplicate
This issue or pull request already exists
 feature
New feature
 help wanted
Need some help
 invalid
Something is wrong
 question
More information is needed
 wontfix
This won't be fixed
No labels
Milestone
No items
No Milestone
Assignees
Clear assignees
ahinkel (Andy Hinkel) jenkins_buildfix lkoester meggert (Marius Eggert) meierj (Jakob Meier) mkoller mkranz mohr (Ulrich Mohr) msenger (Mikael Senger) muellerr (Robin Müller) rschweigert steinertm (Michael Steinert) zinkj
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: eive/fsfw#130