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 +23 -24
2 changed files with 23 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 070b48ada2 - Show all commits

38
src/fsfw/subsystem/SubsystemBase.cpp
View File

@ -24,25 +24,6 @@ ReturnValue_t SubsystemBase::checkStateAgainstTable(HybridIterator<ModeListEntry
using namespace mode;
std::map<object_id_t, ChildInfo>::iterator childIter;
auto checkSubmode = [&]() {
uint8_t mask;
bool submodesAllowedMask = tableIter.value->submodesAllowed(&mask);
uint8_t submodeToCheckAgainst = tableIter.value->getSubmode();
if (tableIter.value->inheritSubmode()) {
submodeToCheckAgainst = targetSubmode;
}
if (not submodesAllowedMask) {
if (childIter->second.submode != submodeToCheckAgainst) {
return returnvalue::FAILED;
}
}
if (submodesAllowedMask) {
if ((childIter->second.submode | mask) != mask) {
return returnvalue::FAILED;
}
}
return returnvalue::OK;
};
for (; tableIter.value != NULL; ++tableIter) {
gaisser marked this conversation as resolved Outdated
gaisser commented 2023-03-07 16:44:14 +01:00
Outdated
Review
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.
object_id_t object = tableIter.value->getObject();
@ -53,9 +34,22 @@ ReturnValue_t SubsystemBase::checkStateAgainstTable(HybridIterator<ModeListEntry
if (childIter->second.mode != tableIter.value->getMode()) {
return returnvalue::FAILED;
}
ReturnValue_t result = checkSubmode();
if (result != returnvalue::OK) {
return result;
// Check submodes here.
uint8_t mask;
gaisser marked this conversation as resolved Outdated
gaisser commented 2023-03-07 16:42:23 +01:00
Outdated
Review
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; ```
bool submodesAllowedMask = tableIter.value->submodesAllowed(&mask);
uint8_t submodeToCheckAgainst = tableIter.value->getSubmode();
if (tableIter.value->inheritSubmode()) {
submodeToCheckAgainst = targetSubmode;
}
if (submodesAllowedMask) {
if ((childIter->second.submode | mask) != mask) {
return returnvalue::FAILED;
}
} else {
if (childIter->second.submode != submodeToCheckAgainst) {
return returnvalue::FAILED;
}
}
}
return returnvalue::OK;

9
src/fsfw/subsystem/modes/ModeDefinitions.h
View File

@ -13,6 +13,8 @@ enum SpecialSubmodeFlags : uint8_t { INHERIT = 1 << 0, ALLOWED_MASK = 1 << 1 };
class ModeListEntry : public SerialLinkedListAdapter<SerializeIF>,
public LinkedElement<ModeListEntry> {
public:
static constexpr uint8_t ALL_SUBMODES_ALLOWED_MASK = 0xff;
ModeListEntry() : SerialLinkedListAdapter(), LinkedElement<ModeListEntry>(this) { setLinks(); }
SerializeElement<uint32_t> value1 = 0;
@ -101,10 +103,13 @@ class ModeListEntry : public SerialLinkedListAdapter<SerializeIF>,
/**
* Specialization of @enableSubmodeAllowed which allows all submodes.
gaisser marked this conversation as resolved
gaisser commented 2023-03-07 16:47:27 +01:00
Review
Copy Link

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

0xFF should be defined somewhere visible as a restricted mask.
*/
void allowAllSubmodes() { enableSubmodeAllowed(0xff); }
void allowAllSubmodes() { enableSubmodeAllowed(ALL_SUBMODES_ALLOWED_MASK); }
/**
* Enable an allowed submode mask for mode checks.
* Enable an allowed submode mask for mode checks. Any submode which contains bits
* outside of the mask will be declined.
*
* For example, for a mask of 0b11, only the modes 0b00, 0b01 and 0b11 will be accepted.
*/
void enableSubmodeAllowed(uint8_t mask) {
value4.entry |= mode::SpecialSubmodeFlags::ALLOWED_MASK;