From f0570041229e5535dabf9f32830adfdae6318463 Mon Sep 17 00:00:00 2001 From: Phil Date: Sat, 22 Aug 2026 18:46:33 +0200 Subject: [PATCH 1/2] Fix #114: remove redundant initial state check in LC_SampleAPs The guard on the starting AP's state prevented sampling the entire range when the first AP happened to be PERMOFF or NOT_USED. LC_SampleSingleAP already checks each AP's state individually, making this pre-check redundant and harmful. --- fsw/src/lc_action.c | 33 ++++++--------------------------- 1 file changed, 6 insertions(+), 27 deletions(-) diff --git a/fsw/src/lc_action.c b/fsw/src/lc_action.c index 92c1d35..16ee7cb 100644 --- a/fsw/src/lc_action.c +++ b/fsw/src/lc_action.c @@ -41,37 +41,16 @@ void LC_SampleAPs(uint16 StartIndex, uint16 EndIndex) { uint16 TableIndex; - uint8 CurrentAPState; /* - ** Make sure the current state of the starting actionpoint - ** in the sample is valid for a sample request - */ - CurrentAPState = LC_OperData.ARTPtr[StartIndex].CurrentState; - - if ((CurrentAPState != LC_ACTION_NOT_USED) && (CurrentAPState != LC_APSTATE_PERMOFF)) + ** Sample selected actionpoints. + ** LC_SampleSingleAP handles per-AP state checks internally, + ** so no pre-check on the start index is needed. + */ + for (TableIndex = StartIndex; TableIndex <= EndIndex; TableIndex++) { - /* - ** Sample selected actionpoints - */ - for (TableIndex = StartIndex; TableIndex <= EndIndex; TableIndex++) - { - LC_SampleSingleAP(TableIndex); - } + LC_SampleSingleAP(TableIndex); } - else - { - /* - ** Actionpoint isn't currently operational - */ - CFE_EVS_SendEvent(LC_APSAMPLE_CURR_ERR_EID, - CFE_EVS_EventType_ERROR, - "Sample AP error, invalid current AP state: AP = %d, State = %d", - StartIndex, - CurrentAPState); - } - - return; } /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */ From 51a1d61f8556058ffa37d63bba41a867c6bbd758 Mon Sep 17 00:00:00 2001 From: philphauler Date: Thu, 17 Sep 2026 18:02:13 +0200 Subject: [PATCH 2/2] Fix #114, update LC_SampleAPs unit tests for removed error-state check LC_SampleAPs no longer sends LC_APSAMPLE_CURR_ERR_EID when the starting actionpoint is NOT_USED or PERMOFF; LC_SampleSingleAP already ignores those states internally (only ACTIVE/PASSIVE are sampled), so the loop just skips them silently. Update LC_SampleAPs_Test_SingleActionPointError and LC_SampleAPs_Test_SingleActionPointPermOff to assert zero events sent instead of the removed error event, matching current behavior. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01HDRvC9ePDP3i6xpxFAECFg --- unit-test/lc_action_tests.c | 38 +++++++++---------------------------- 1 file changed, 9 insertions(+), 29 deletions(-) diff --git a/unit-test/lc_action_tests.c b/unit-test/lc_action_tests.c index 8aba324..882defc 100644 --- a/unit-test/lc_action_tests.c +++ b/unit-test/lc_action_tests.c @@ -65,26 +65,16 @@ void LC_SampleAPs_Test_SingleActionPointError(void) { uint16 StartIndex = 0; uint16 EndIndex = 0; - int32 strCmpResult; - char ExpectedEventString[CFE_MISSION_EVS_MAX_MESSAGE_LENGTH]; - - snprintf(ExpectedEventString, - CFE_MISSION_EVS_MAX_MESSAGE_LENGTH, - "Sample AP error, invalid current AP state: AP = %%d, State = %%d"); LC_OperData.ARTPtr[StartIndex].CurrentState = LC_APSTATE_NOT_USED; /* Execute the function being tested */ LC_SampleAPs(StartIndex, EndIndex); - /* Verify results */ - UtAssert_INT32_EQ(UT_GetStubCount(UT_KEY(CFE_EVS_SendEvent)), 1); - UtAssert_INT32_EQ(context_CFE_EVS_SendEvent[0].EventID, LC_APSAMPLE_CURR_ERR_EID); - UtAssert_INT32_EQ(context_CFE_EVS_SendEvent[0].EventType, CFE_EVS_EventType_ERROR); - - strCmpResult = strncmp(ExpectedEventString, context_CFE_EVS_SendEvent[0].Spec, CFE_MISSION_EVS_MAX_MESSAGE_LENGTH); - - UtAssert_True(strCmpResult == 0, "Event string matched expected result, '%s'", context_CFE_EVS_SendEvent[0].Spec); + /* Verify results: LC_SampleSingleAP silently skips actionpoints that + * are not ACTIVE or PASSIVE, so no event should be issued for an + * invalid starting actionpoint state */ + UtAssert_INT32_EQ(UT_GetStubCount(UT_KEY(CFE_EVS_SendEvent)), 0); } void LC_SampleAPs_Test_MultiActionPointNominal(void) @@ -103,26 +93,16 @@ void LC_SampleAPs_Test_SingleActionPointPermOff(void) { uint16 StartIndex = 0; uint16 EndIndex = 0; - int32 strCmpResult; - char ExpectedEventString[CFE_MISSION_EVS_MAX_MESSAGE_LENGTH]; - - snprintf(ExpectedEventString, - CFE_MISSION_EVS_MAX_MESSAGE_LENGTH, - "Sample AP error, invalid current AP state: AP = %%d, State = %%d"); LC_OperData.ARTPtr[StartIndex].CurrentState = LC_APSTATE_PERMOFF; /* Execute the function being tested */ LC_SampleAPs(StartIndex, EndIndex); - /* Verify results */ - UtAssert_INT32_EQ(UT_GetStubCount(UT_KEY(CFE_EVS_SendEvent)), 1); - UtAssert_INT32_EQ(context_CFE_EVS_SendEvent[0].EventID, LC_APSAMPLE_CURR_ERR_EID); - UtAssert_INT32_EQ(context_CFE_EVS_SendEvent[0].EventType, CFE_EVS_EventType_ERROR); - - strCmpResult = strncmp(ExpectedEventString, context_CFE_EVS_SendEvent[0].Spec, CFE_MISSION_EVS_MAX_MESSAGE_LENGTH); - - UtAssert_True(strCmpResult == 0, "Event string matched expected result, '%s'", context_CFE_EVS_SendEvent[0].Spec); + /* Verify results: LC_SampleSingleAP silently skips actionpoints that + * are not ACTIVE or PASSIVE, so no event should be issued for an + * invalid starting actionpoint state */ + UtAssert_INT32_EQ(UT_GetStubCount(UT_KEY(CFE_EVS_SendEvent)), 0); } void LC_SampleSingleAP_Test_StateChangePassToFail(void) @@ -1919,4 +1899,4 @@ void UtTest_Setup(void) LC_Test_Setup, LC_Test_TearDown, "LC_ValidateRPN_Test_InvalidBufferItem"); -} \ No newline at end of file +}