Fix #108, dispatch pattern for SC #111
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| /* Process a command */ | ||
| /* */ | ||
| /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */ | ||
| void SC_ProcessCommand(const CFE_SB_Buffer_t *BufPtr) |
Check notice
Code scanning / CodeQL
Long function without assertion Note
| /* Process Requests */ | ||
| /* */ | ||
| /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */ | ||
| void SC_ProcessRequest(const CFE_SB_Buffer_t *BufPtr) |
Check notice
Code scanning / CodeQL
Long function without assertion Note
| /* SC Verify the length of the command */ | ||
| /* */ | ||
| /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */ | ||
| bool SC_VerifyCmdLength(const CFE_MSG_Message_t *Msg, size_t ExpectedLength) |
Check notice
Code scanning / CodeQL
Long function without assertion Note
| if (SC_AppData.NextProcNumber == SC_RTP) | ||
| { | ||
| /* set during init to power on or processor reset auto-exec RTS */ | ||
| if (SC_AppData.AutoStartRTS != 0) | ||
| { | ||
| /* make sure the selected auto-exec RTS is enabled */ | ||
| if (SC_OperData.RtsInfoTblAddr[SC_RTS_NUM_TO_INDEX(SC_AppData.AutoStartRTS)].RtsStatus == SC_LOADED) | ||
| { | ||
| SC_OperData.RtsInfoTblAddr[SC_RTS_NUM_TO_INDEX(SC_AppData.AutoStartRTS)].DisabledFlag = false; | ||
| } | ||
|
|
||
| /* send ground cmd to have SC start the RTS */ | ||
| SC_AutoStartRts(SC_AppData.AutoStartRTS); | ||
|
|
||
| /* only start it once */ | ||
| SC_AppData.AutoStartRTS = 0; | ||
| } | ||
|
|
||
| /* request from health and safety for housekeeping status */ | ||
| SC_SendHkPacket(); | ||
| SC_ProcessRtpCommand(); | ||
| } | ||
| break; | ||
| } | ||
|
|
||
| case SC_1HZ_WAKEUP_MID: | ||
| /* | ||
| ** Time to execute a command in the SC memory | ||
| */ | ||
| do | ||
| SC_UpdateNextTime(); | ||
| if ((SC_AppData.NextProcNumber == SC_NONE) || | ||
| (SC_AppData.NextCmdTime[SC_AppData.NextProcNumber] > SC_AppData.CurrentTime)) | ||
| { | ||
| SC_OperData.NumCmdsSec = 0; | ||
| IsThereAnotherCommandToExecute = false; | ||
| } | ||
| else /* Command needs to run immediately */ | ||
| { | ||
| if (SC_OperData.NumCmdsSec >= SC_MAX_CMDS_PER_SEC) | ||
| { | ||
| /* | ||
| ** Check to see if there is an ATS switch Pending, if so service it. | ||
| */ | ||
| if (SC_OperData.AtsCtrlBlckAddr->SwitchPendFlag == true) | ||
| { | ||
| SC_ServiceSwitchPend(); | ||
| } | ||
|
|
||
| if (SC_AppData.NextProcNumber == SC_ATP) | ||
| { | ||
| SC_ProcessAtpCmd(); | ||
| } | ||
| else | ||
| { | ||
| if (SC_AppData.NextProcNumber == SC_RTP) | ||
| { | ||
| SC_ProcessRtpCommand(); | ||
| } | ||
| } | ||
|
|
||
| SC_UpdateNextTime(); | ||
| if ((SC_AppData.NextProcNumber == SC_NONE) || | ||
| (SC_AppData.NextCmdTime[SC_AppData.NextProcNumber] > SC_AppData.CurrentTime)) | ||
| { | ||
| SC_OperData.NumCmdsSec = 0; | ||
| IsThereAnotherCommandToExecute = false; | ||
| } | ||
| else /* Command needs to run immediately */ | ||
| { | ||
| if (SC_OperData.NumCmdsSec >= SC_MAX_CMDS_PER_SEC) | ||
| { | ||
| SC_OperData.NumCmdsSec = 0; | ||
| IsThereAnotherCommandToExecute = false; | ||
| } | ||
| else | ||
| { | ||
| IsThereAnotherCommandToExecute = true; | ||
| } | ||
| } | ||
| } while (IsThereAnotherCommandToExecute); | ||
|
|
||
| break; | ||
|
|
||
| default: | ||
| CFE_EVS_SendEvent(SC_MID_ERR_EID, CFE_EVS_EventType_ERROR, "Invalid command pipe message ID: 0x%08lX", | ||
| (unsigned long)CFE_SB_MsgIdToValue(MessageID)); | ||
|
|
||
| SC_OperData.HkPacket.Payload.CmdErrCtr++; | ||
| break; | ||
| } /* end switch */ | ||
| SC_OperData.NumCmdsSec = 0; | ||
| IsThereAnotherCommandToExecute = false; | ||
| } | ||
| else | ||
| { | ||
| IsThereAnotherCommandToExecute = true; | ||
| } | ||
| } | ||
| } while (IsThereAnotherCommandToExecute); |
Check warning
Code scanning / CodeQL
Unbounded loop Warning
| /* Process a command */ | ||
| /* */ | ||
| /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */ | ||
| void SC_ProcessCommand(const CFE_SB_Buffer_t *BufPtr) |
Check notice
Code scanning / CodeQL
Function too long Note
| /* Process a command */ | ||
| /* */ | ||
| /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */ | ||
| void SC_ProcessCommand(const CFE_SB_Buffer_t *BufPtr) |
Check warning
Code scanning / CodeQL
Poorly documented large function Warning
|
Workflow failure currently reported in this PR should be fixed by my other pending PRs. Once reviewed and merged I this can be rebased. |
jphickey
force-pushed
the
fix-108-dispatcher
branch
from
11116a2
to
e904d4b
Compare
jphickey
force-pushed
the
fix-108-dispatcher
branch
from
e904d4b
to
5aec544
Compare
There was a problem hiding this comment.
CodeQL-coding-standard found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.
Move switch statements to a separate dispatch source file, and update all unit tests accordingly. All business logic of commands is put into separate command handler functions in sc_cmds.c. No logic is inside the switch statement, outside of message validation.
jphickey
force-pushed
the
fix-108-dispatcher
branch
from
5aec544
to
4090ed7
Compare
|
Everything rebased and tests/workflows are passing. |
Checklist (Please check before submitting)
Describe the contribution
Move switch statements to a separate dispatch source file, and update all unit tests accordingly.
All business logic of commands is put into separate command handler functions in sc_cmds.c. No logic is inside the switch statement, outside of message validation.
Fixes #108
Testing performed
Build and run SC and all tests
Expected behavior changes
No external/observable behavior change
System(s) tested on
Debian
Additional context
Intent is to improve code consistency, ease future maintenance, and to match patterns used elsewhere.
Contributor Info - All information REQUIRED for consideration of pull request
Joseph Hickey, Vantage Systems, Inc.