Checklist (Please check before submitting)
Describe the bug
HS requirements HS2000.2(e) and HS5000.1(e) indicate that "Perform No Action" is one option for handling the monitor events. However, there is no specific case for this in the code.
Instead, this invokes the "Send Software Bus Message" case, which in turn will attempt to find the matching entry in the MA table. This will fail to find the entry (index out of range) and as a result no message is sent.
In the end, the functionality is correct, its just fairly obfuscated in the way its implemented.
To Reproduce
N/A
Expected behavior
A clearer (& more robust) implementation should have an explicit case for the NOACT configuration option. With it implemented the way it is, simply changing the Enum integer values could cause the MA table lookup to suddenly hit a valid entry and then SB messages would be sent even though its configured for no action.
Code snips
This "default" case will be used for no action. The comment does indicate that the logic will end up skipping the message if set to NOACT, but this relies on the enum values being set in a particular way, so its rather weak.
|
/* |
|
** Message Action types processing (invalid will be skipped) |
|
*/ |
|
default: |
|
HS_ExecuteMessageAction(AMEntryPtr->ActionType, HS_AppMonActCallback, AMEntryPtr); |
|
break; |
Additional context
Noted during requirements review in #154
Reporter Info
Joseph Hickey, Vantage Systems, Inc.
Checklist (Please check before submitting)
Describe the bug
HS requirements HS2000.2(e) and HS5000.1(e) indicate that "Perform No Action" is one option for handling the monitor events. However, there is no specific case for this in the code.
Instead, this invokes the "Send Software Bus Message" case, which in turn will attempt to find the matching entry in the MA table. This will fail to find the entry (index out of range) and as a result no message is sent.
In the end, the functionality is correct, its just fairly obfuscated in the way its implemented.
To Reproduce
N/A
Expected behavior
A clearer (& more robust) implementation should have an explicit case for the
NOACTconfiguration option. With it implemented the way it is, simply changing the Enum integer values could cause the MA table lookup to suddenly hit a valid entry and then SB messages would be sent even though its configured for no action.Code snips
This "default" case will be used for no action. The comment does indicate that the logic will end up skipping the message if set to NOACT, but this relies on the enum values being set in a particular way, so its rather weak.
HS/fsw/src/hs_monitors.c
Lines 222 to 227 in 5c91c18
Additional context
Noted during requirements review in #154
Reporter Info
Joseph Hickey, Vantage Systems, Inc.