eive/fsfw
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 0 Commits 5 Files Changed 5 +156 -81
muellerr commented 2023-03-07 16:11:29 +01:00
Owner
Copy Link
  • 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
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
muellerr added 1 commit 2023-03-07 16:31:32 +01:00
gaisser requested changes 2023-03-07 17:05:59 +01:00
gaisser left a comment
Member
Copy Link

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

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

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

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
gaisser approved these changes 2023-03-07 17:19:36 +01:00
gaisser left a comment
Member
Copy Link

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 Label
api change
breaking api change
bug
documentation
duplicate
feature
help wanted
invalid
question
wontfix
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: eive/fsfw#130
No description provided.
Delete Branch "feature_allow_submodes_mode_list_entry"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?