Summary
The initial state validation in LC_SampleAPs prevents sampling any action points whenever the first (start) AP is in PERMOFF or NOT_USED state. Since LC_SampleSingleAP() already skips non‑operational APs, this extra check is unnecessary and leads to unexpected behavior when sampling a range that happens to begin with a disabled AP.
Background
-
Function: LC_SampleAPs(uint16 StartIndex, uint16 EndIndex)
-
Current behavior:
CurrentAPState = LC_OperData.ARTPtr[StartIndex].CurrentState;
if ((CurrentAPState != LC_ACTION_NOT_USED) && (CurrentAPState != LC_APSTATE_PERMOFF))
{
for (TableIndex = StartIndex; TableIndex <= EndIndex; TableIndex++)
{
LC_SampleSingleAP(TableIndex);
}
}
else
{
CFE_EVS_SendEvent(..., "Sample AP error, invalid current AP state: AP = %d, State = %d", StartIndex, CurrentAPState);
}
return;
-
Redundancy: Inside LC_SampleSingleAP() there is already a check for PERMOFF/invalid states and it simply returns without sampling non‑operational APs (see LC_SampleSingleAP at line 94–95`).
Problem
- Unintended all‑or‑nothing behavior
If StartIndex refers to an AP that is permanently off or unused, none of the APs in the [StartIndex…EndIndex] range will be sampled—even if the rest are active.
- Duplicate logic
The per‑AP validity check is already handled in LC_SampleSingleAP(), so rejecting the entire batch upfront is unnecessary and inconsistent.
Proposed Change
- Remove the initial state check at the top of
LC_SampleAPs.
- Always iterate from
StartIndex to EndIndex, calling LC_SampleSingleAP(TableIndex) for each.
- Rely on the existing checks inside
LC_SampleSingleAP() to skip invalid APs and emit error events on a per‑AP basis.
Before:
if ((CurrentAPState != LC_ACTION_NOT_USED) && (CurrentAPState != LC_APSTATE_PERMOFF))
{
for (TableIndex = StartIndex; TableIndex <= EndIndex; TableIndex++)
{
LC_SampleSingleAP(TableIndex);
}
}
else
{
CFE_EVS_SendEvent(…);
}
After:
for (TableIndex = StartIndex; TableIndex <= EndIndex; TableIndex++)
{
LC_SampleSingleAP(TableIndex);
}
Impact
- Positive:
- Ensures that all valid APs in the requested range are sampled, regardless of the state of the first AP.
- Simplifies the code by removing redundant validation logic.
- Neutral:
- Error reporting remains the same—
LC_SampleSingleAP() will emit an event for each invalid AP.
References
- Initial check in
LC_SampleAPs:
|
if ((CurrentAPState != LC_ACTION_NOT_USED) && (CurrentAPState != LC_APSTATE_PERMOFF)) |
- Per‑AP validation in
LC_SampleSingleAP:
|
if ((CurrentAPState == LC_APSTATE_ACTIVE) || (CurrentAPState == LC_APSTATE_PASSIVE)) |
Summary
The initial state validation in
LC_SampleAPsprevents sampling any action points whenever the first (start) AP is inPERMOFForNOT_USEDstate. SinceLC_SampleSingleAP()already skips non‑operational APs, this extra check is unnecessary and leads to unexpected behavior when sampling a range that happens to begin with a disabled AP.Background
Function:
LC_SampleAPs(uint16 StartIndex, uint16 EndIndex)Current behavior:
Redundancy: Inside
LC_SampleSingleAP()there is already a check forPERMOFF/invalid states and it simply returns without sampling non‑operational APs (seeLC_SampleSingleAPat line 94–95`).Problem
If
StartIndexrefers to an AP that is permanently off or unused, none of the APs in the[StartIndex…EndIndex]range will be sampled—even if the rest are active.The per‑AP validity check is already handled in
LC_SampleSingleAP(), so rejecting the entire batch upfront is unnecessary and inconsistent.Proposed Change
LC_SampleAPs.StartIndextoEndIndex, callingLC_SampleSingleAP(TableIndex)for each.LC_SampleSingleAP()to skip invalid APs and emit error events on a per‑AP basis.Before:
After:
Impact
LC_SampleSingleAP()will emit an event for each invalid AP.References
LC_SampleAPs:LC/fsw/src/lc_action.c
Line 53 in d6f56c3
LC_SampleSingleAP:LC/fsw/src/lc_action.c
Line 94 in d6f56c3