Giter Site home page Giter Site logo

hs's Introduction

core Flight System (cFS) Health and Safety Application (HS)

Introduction

The Health and Safety application (HS) is a core Flight System (cFS) application that is a plug in to the Core Flight Executive (cFE) component of the cFS.

The HS application provides functionality for Application Monitoring, Event Monitoring, Hardware Watchdog Servicing, Execution Counter Reporting (optional), and CPU Aliveness Indication (via UART).

The HS application is written in C and depends on the cFS Operating System Abstraction Layer (OSAL) and cFE components.
There is additional HS application-specific configuration information contained in the application user's guide.

User's guide information can be generated using Doxygen (from top mission directory):

  make prep
  make -C build/docs/hs-usersguide hs-usersguide

Software Required

cFS Framework (cFE, OSAL, PSP)

An integrated bundle including the cFE, OSAL, and PSP can be obtained at https://github.com/nasa/cfs

About cFS

The cFS is a platform and project independent reusable software framework and set of reusable applications developed by NASA Goddard Space Flight Center. This framework is used as the basis for the flight software for satellite data systems and instruments, but can be used on other embedded systems. More information on the cFS can be found at http://cfs.gsfc.nasa.gov

hs's People

Contributors

astrogeco avatar chillfig avatar dmknutsen avatar dzbaker avatar ejtimmon avatar havencarlson avatar jasonduley avatar jphickey avatar lucienmorey avatar paulober avatar skliper avatar the-other-james avatar thnkslprpt avatar

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

hs's Issues

Update CLA Information

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I reviewed the cFS README.md file to see if the feature is in the major future work.
  • I performed a cursory search to see if the feature request is relevant, not redundant, nor in conflict with other tickets.

Is your feature request related to a problem? Please describe.
Have new CLAs given the change in nasa/cFS#491 with the combined CLA,

Describe the solution you'd like

  • Update the instructions in each app's Contributing.md
  • Delete old CLA pdfs
  • Update PR and Issue templates as needed

Describe alternatives you've considered

None

Additional context
None

Requester Info
Gerardo E. Cruz-Ortiz

Extraneous assignment to CFE_SUCCESS

Checklist

  • I reviewed the Contributing Guide.
  • I performed a cursory search to see if the bug report is relevant, not redundant, nor in conflict with other tickets.

Describe the bug
Couple more extraneous assignments as in #10

Code snips

HS/fsw/src/hs_app.c

Lines 54 to 68 in a99b890

void HS_AppMain(void)
{
int32 Status = CFE_SUCCESS;
uint32 RunStatus = CFE_ES_RunStatus_APP_RUN;
CFE_SB_Buffer_t *BufPtr = NULL;
/*
** Performance Log, Start
*/
CFE_ES_PerfLogEntry(HS_APPMAIN_PERF_ID);
/*
** Perform application specific initialization
*/
Status = HS_AppInit();

HS/fsw/src/hs_app.c

Lines 421 to 430 in a99b890

int32 HS_TblInit(void)
{
uint32 TableSize = 0;
uint32 TableIndex = 0;
int32 Status = CFE_SUCCESS;
/* Register The HS Applications Monitor Table */
TableSize = HS_MAX_MONITORED_APPS * sizeof(HS_AMTEntry_t);
Status = CFE_TBL_Register(&HS_AppData.AMTableHandle, HS_AMT_TABLENAME, TableSize, CFE_TBL_OPT_DEFAULT,
HS_ValidateAMTable);

Expected behavior
Remove assignments - unnecessary code.

Reporter Info
Avi Weiss @thnkslprpt

Use generated stubs

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I reviewed the README file to see if the feature is in the major future work.
  • I performed a cursory search to see if the feature request is relevant, not redundant, nor in conflict with other tickets.

Is your feature request related to a problem? Please describe.
HS unit testing currently uses a set of stubs for its internal units that are not generated by the tool

Describe the solution you'd like
Use the generated stubs directly whenever possible, as this makes future maintenance easier - when an API changes, just re-run the generator tool to update the stubs.

Additional context
This requires some additional separation of items - global variable stubs should be in a separate compilation unit, as the tool does not generate these.

Requester Info
Joseph Hickey, Vantage Systems, Inc.

Make hs_custom.h commands and EIDs public

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I reviewed the README file to see if the feature is in the major future work.
  • I performed a cursory search to see if the feature request is relevant, not redundant, nor in conflict with other tickets.

Is your feature request related to a problem? Please describe.
The commands and EIDs curently in hs_customs.h need to be made public.

HS/fsw/src/hs_custom.h

Lines 33 to 243 in 7afa69a

/*************************************************************************
* Constants
************************************************************************/
#define HS_UTIL_DIAG_REPORTS 4
/**
* \ingroup cfshscmdcodes
*
* Custom command codes must not conflict with those in hs_msgdefs.h
* \{
*/
/**
* \brief Report Util Diagnostics
*
* \par Description
* Reports the Utilization Diagnostics
*
* \par Command Structure
* #HS_NoArgsCmd_t
*
* \par Command Verification
* Successful execution of this command may be verified with
* the following telemetry:
* - #HS_HkPacket_t.CmdCount will increment
* - The #HS_UTIL_DIAG_REPORT_EID informational event message will be
* generated when the command is executed
*
* \par Error Conditions
* This command may fail for the following reason(s):
* - Command packet length not as expected
*
* \par Evidence of failure may be found in the following telemetry:
* - #HS_HkPacket_t.CmdErrCount will increment
*
* \par Criticality
* None
*/
#define HS_REPORT_DIAG_CC 12
/**
* \brief Set Utilization Calibration Parameters
*
* \par Description
* Sets the Utilization Calibration Parameter
*
* \par Command Structure
* #HS_SetUtilParamsCmd_t
*
* \par Command Verification
* Successful execution of this command may be verified with
* the following telemetry:
* - #HS_HkPacket_t.CmdCount will increment
*
* \par Error Conditions
* This command may fail for the following reason(s):
* - Command packet length not as expected
* - Any parameter is set to 0.
*
* \par Evidence of failure may be found in the following telemetry:
* - #HS_HkPacket_t.CmdErrCount will increment
*
* \par Criticality
* None
*/
#define HS_SET_UTIL_PARAMS_CC 13
/**
* \brief Set Utilization Diagnostics Parameter
*
* \par Description
* Sets the Utilization Diagnostics parameter
*
* \par Command Structure
* #HS_SetUtilDiagCmd_t
*
* \par Command Verification
* Successful execution of this command may be verified with
* the following telemetry:
* - #HS_HkPacket_t.CmdCount will increment
*
* \par Error Conditions
* This command may fail for the following reason(s):
* - Command packet length not as expected
*
* \par Evidence of failure may be found in the following telemetry:
* - #HS_HkPacket_t.CmdErrCount will increment
*
* \par Criticality
* None
*/
#define HS_SET_UTIL_DIAG_CC 14
/**\}*/
/**
* \ingroup cfshsevents
*
* Custom Event IDs must not conflict with those in hs_events.h
* \{
*/
/**
* \brief HS CPU Utilization Monitoring Create Child Task Failed Event ID
*
* \par Type: ERROR
*
* \par Cause:
*
* This event message is issued when CFS Health and Safety
* is unable to create its child task via the #CFE_ES_CreateChildTask
* API
*/
#define HS_CR_CHILD_TASK_ERR_EID 101
/**
* \brief HS CPU Utilization Monitoring Register Sync Callback Failed Event ID
*
* \par Type: ERROR
*
* \par Cause:
*
* This event message is issued when CFS Health and Safety
* is unable to create its sync callback via the #CFE_TIME_RegisterSynchCallback
* API
*/
#define HS_CR_SYNC_CALLBACK_ERR_EID 102
/**
* \brief HS Report Diagnostics Command Event ID
*
* \par Type: INFORMATION
*
* \par Cause:
*
* This event message is issued when CFS Health and Safety receives the #HS_REPORT_DIAG_CC command.
*/
#define HS_UTIL_DIAG_REPORT_EID 103
/**
* \brief HS Set Utilization Paramaters Command Event ID
*
* \par Type: Debug
*
* \par Cause:
*
* This event message is issued when CFS Health and Safety successfully processes the #HS_SET_UTIL_PARAMS_CC command.
*/
#define HS_SET_UTIL_PARAMS_DBG_EID 104
/**
* \brief HS Set Utilization Parameters Zero Value Event ID
*
* \par Type: Error
*
* \par Cause:
*
* This event message is issued when CFS Health and Safety fails to processes the #HS_SET_UTIL_PARAMS_CC command.
* due to a 0 as at least one of the parameters.
*/
#define HS_SET_UTIL_PARAMS_ERR_EID 105
/**
* \brief HS Set Utilization Diagnostics Command Event ID
*
* \par Type: Debug
*
* \par Cause:
*
* This event message is issued when CFS Health and Safety successfully processes the #HS_SET_UTIL_DIAG_CC command.
*/
#define HS_SET_UTIL_DIAG_DBG_EID 106
/**\}*/
/*************************************************************************
* Command Structure Definitions
************************************************************************/
/**
* \ingroup cfshscmdstructs
* \{
*/
/**
* \brief Set Utilitiliztion Parameters Command
*
* See #HS_SET_UTIL_PARAMS_CC
*/
typedef struct
{
CFE_MSG_CommandHeader_t CmdHeader; /**< \brief Command header */
int32 Mult1; /**< \brief Multiplier 1 parameter */
int32 Div; /**< \brief Divisior parameter */
int32 Mult2; /**< \brief Multiplier 2 parameter */
} HS_SetUtilParamsCmd_t;
/**
* \brief Set Utilization Diagnostics Command
*
* See #HS_SET_UTIL_DIAG_CC
*/
typedef struct
{
CFE_MSG_CommandHeader_t CmdHeader; /**< \brief Command header */
uint32 Mask; /**< \brief Utilization Diagnostics Mask */
} HS_SetUtilDiagCmd_t;
/**\}*/

Describe the solution you'd like
Make new file hs/fsw/inc/hs_custom_internal.h and move the aforementioned definitions to it.

Describe alternatives you've considered
Simply move the aforementioned definitions to fsw/inc/hs_extern_typedefs.h

Additional context
JSC needs this change to build successfully

Requester Info
Justin Figueroa, Vantage Systems

Combine consecutive mutually exclusive status checks into an if/else

Checklist

  • I reviewed the Contributing Guide.
  • I performed a cursory search to see if the bug report is relevant, not redundant, nor in conflict with other tickets.

Describe the bug
HS_EnableEventMonCmd() and HS_DisableEventMonCmd() have consecutive, mutually exclusive status checks.

Code snips

HS/fsw/src/hs_cmds.c

Lines 409 to 416 in 76915e9

if (Status != CFE_SUCCESS)
{
CFE_EVS_SendEvent(HS_EVENTMON_LONG_SUB_EID, CFE_EVS_EventType_ERROR,
"Event Monitor Enable: Error Subscribing to long-format Events,RC=0x%08X",
(unsigned int)Status);
}
if (Status == CFE_SUCCESS)

HS/fsw/src/hs_cmds.c

Lines 465 to 472 in 76915e9

if (Status != CFE_SUCCESS)
{
CFE_EVS_SendEvent(HS_EVENTMON_LONG_UNSUB_EID, CFE_EVS_EventType_ERROR,
"Event Monitor Disable: Error Unsubscribing from long-format Events,RC=0x%08X",
(unsigned int)Status);
}
if (Status == CFE_SUCCESS)

Expected behavior
Combine into an if/else - no need to do 2 evaluations.

Reporter Info
Avi Weiss @thnkslprpt

Remove CFE_PSP_MemSet use on addresses in RAM

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I reviewed the README file to see if the feature is in the major future work.
  • I performed a cursory search to see if the feature request is relevant, not redundant, nor in conflict with other tickets.

Is your feature request related to a problem? Please describe.
Should just use memset/memcpy for addresses in RAM. The PSP functions serve no use in this context.

Describe the solution you'd like
Replace with memset/memcpy.

Describe alternatives you've considered
None

Additional context
None

Requester Info
Jacob Hageman - NASA/GSFC

HS requirements imply 2 tables/entry types where only 1 exists

There are several HS requirements that imply that there are 2 tables and/or entry types - Core/Non-Core. That is not the case. There is one app table...and there is no distinction between core/non-core app types (with the exception that HS does not support restarting a core app).

HS2000 โ€“ The term โ€˜criticalโ€™ should be removed from the application table name.
HS2000.1, HS2000.1.1, HS2000.1.2 โ€“ Requirements are duplicate and need to be removed. They are covered by HS2000.2, HS2000.2.1, HS2000.2.2.
HS2000.2 โ€“ Needs to be updated to remove verbiage specifying the type (Core/Non-Core) of entry in application table. Further, it should specify that item (a) only applies to non-core apps.

AppMonStatusRefresh Tests appear to check the wrong output

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I performed a cursory search to see if the bug report is relevant, not redundant, nor in conflict with other tickets.

Describe the bug
The HS_AppMonStatusRefresh() API call updates a field called AppMonEnables in the global data structure.

Declared here:

uint32 AppMonEnables[((HS_MAX_MONITORED_APPS - 1) / HS_BITS_PER_APPMON_ENABLE) +

Value is updated in the function here:
CFE_SET((HS_AppData.AppMonEnables[TableIndex / HS_BITS_PER_APPMON_ENABLE]),

Called from unit test here (among others):
HS_AppMonStatusRefresh();

However - the unit tests then check the similarly-named field within the HK Packet here:

UtAssert_True(HS_AppData.HkPacket.AppMonEnables[0] == 0, "HS_AppData.HkPacket.AppMonEnables[0] == 0");

Note that this is not the same member -- one is directly inside the global, the other is inside the HK Packet.

Expected behavior
Test should verify the fields it was supposed to update.

Additional context
Note the tests is passing, this suggests the test might not be sufficient, because it is checking a different value than was set. The test conditions could have simply been written to pass without actually verifying expected output.

Reporter Info
Joseph Hickey, Vantage Systems, Inc.

Replace quotes with angle brackets in #include statements within /inc

Describe the solution you'd like
Quotes should be replaced with angle brackets in #include statements that reside in the /inc location. This will ensure that the preprocessor selects the files pre-designated to override the default files contained within the open source cFS build release - as opposed to selecting those located in the same directory.

Requester Info
Dan Knutsen
NASA Goddard

All CMD/TLM message should put content in a "Payload" sub-structure

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I reviewed the README file to see if the feature is in the major future work.
  • I performed a cursory search to see if the feature request is relevant, not redundant, nor in conflict with other tickets.

Is your feature request related to a problem? Please describe.
To match the patterns used in CFE and other modules, all CMD/TLM message definitions should put the content (non-header) parts into a separate struct called "Payload".

Describe the solution you'd like
Separate message content into a sub structure called "Payload".

Additional context
This is benefit to tooling that can use the presence of this field to identify where the actual content starts (e.g. something like offsetof(MsgType, Payload) would work and be correct, as opposed to checking sizeof(CFE_MSG_CommandHeader_t) which may not actually reflect where the content starts due to possible compiler-added padding between them).

Requester Info
Joseph Hickey, Vantage Systems, Inc.

Inconsistent Event ID naming

Checklist

  • I reviewed the Contributing Guide.
  • I performed a cursory search to see if the bug report is relevant, not redundant, nor in conflict with other tickets.

Describe the bug
Copy of nasa/cFE#2175
After finding that there were 9 different Event IDs to indicate the same thing (Invalid Message ID) in nasa/CF#262, I scrubbed the other common commands (e.g. Task Initialisation [INIT], NOOP, Reset Counters etc.) and found the same issue there - almost every component/app had their own variation of the Event ID name for the exact same event.

Expected behavior
Apply consistent Event ID names to the events which are common to all/most components and apps.

Code snips
Invalid Message ID:
CFE_EVS_ERR_MSGID_EID
CFE_SB_BAD_MSGID_EID
CFE_TIME_ID_ERR_EID
CS_MID_ERR_EID
TO_LAB_MSGID_ERR_EID
SAMPLE_APP_INVALID_MSGID_ERR_EID
BP_INVALID_MID_ERR_EID
SCH_MD_ERR_EID
CI_LAB_COMMAND_ERR_EID

Initialization:
CFE_TIME_INIT_EID
CFE_TBL_INIT_INF_EID
CFE_EVS_STARTUP_EID
CF_EID_INF_INIT
BP_INIT_APP_INFO_EID
SCH_INITSTATS_INF_EID
CI_LAB_STARTUP_INF_EID

NOOP:
CFE_TIME_NOOP_EID
CFE_TBL_NOOP_INF_EID
CFE_SB_CMD0_RCVD_EID
CF_EID_INF_CMD_NOOP
FM_NOOP_CMD_EID
CI_LAB_COMMANDNOP_INF_EID

Reset Counters:
CFE_TIME_RESET_EID
CFE_TBL_RESET_INF_EID
CFE_EVS_RSTCNT_EID
CFE_SB_CMD1_RCVD_EID
CF_EID_INF_CMD_RESET
SC_RESET_DEB_EID
HS_RESET_DBG_EID
FM_RESET_CMD_EID
HK_RESET_CNTRS_CMD_EID
MD_RESET_CNTRS_DBG_EID
CI_LAB_COMMANDRST_INF_EID

etc.

Reporter Info
Avi Weiss @thnkslprpt

Add functional verification of HS8006 and HS8006.1

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I reviewed the README file to see if the feature is in the major future work.
  • I performed a cursory search to see if the feature request is relevant, not redundant, nor in conflict with other tickets.

Is your feature request related to a problem? Please describe.
HS8006: Upon any initialization, HS shall wait until the cFE startup synch has been received indicating all Applications have started.
HS8006.1: If the startup-synch is not received in <PLATFORM_DEFINED> seconds, HS shall begin processing.

These are difficult to verify in CFT, so they need to be verified in unit test by ensuring CFE_ES_WaitForStartupSync is called with a platform defined timeout.

Describe the solution you'd like
Add verification in the unit tests.

Describe alternatives you've considered
None

Additional context
Test case: https://etdjira.gsfc.nasa.gov/browse/GSFCCFS-1927

Requester Info
Jacob Hageman - NASA/GSFC

Refactor config dependent conditional in hs_custom.c

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I reviewed the README file to see if the feature is in the major future work.
  • I performed a cursory search to see if the feature request is relevant, not redundant, nor in conflict with other tickets.

Is your feature request related to a problem? Please describe.
Can't cover branch when HS_UTIL_CALLS_PER_MARK is 1 (since the counter is static local):

HS/fsw/src/hs_custom.c

Lines 180 to 195 in 2fc0dd9

void HS_UtilizationMark(void)
{
static uint32 CycleCount = 0;
CycleCount++;
if (CycleCount >= HS_UTIL_CALLS_PER_MARK)
{
HS_CustomData.LastIdleTaskInterval = HS_CustomData.ThisIdleTaskExec - HS_CustomData.LastIdleTaskExec;
HS_CustomData.LastIdleTaskExec = HS_CustomData.ThisIdleTaskExec;
CycleCount = 0;
}
return;
} /* end HS_UtilizationMark */

Describe the solution you'd like
Use the custom global to store config such that it can be set to get full coverage.

Describe alternatives you've considered
None

Additional context
None

Requester Info
Jacob Hageman - NASA/GSFC

Make hs_tbl.h public

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I reviewed the README file to see if the feature is in the major future work.
  • I performed a cursory search to see if the feature request is relevant, not redundant, nor in conflict with other tickets.

Is your feature request related to a problem? Please describe.
hs_tbl.h defines macros used in table generation and should be made public.

Requester Info
Dan Knutsen
NASA Goddard

Refactor "Custom" code (not really customizable)

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I performed a cursory search to see if the bug report is relevant, not redundant, nor in conflict with other tickets.

Describe the bug
The structure of the HS code includes hs_custom.h and hs_custom.c that imply that these contain user-defined functions. However the current code does not really allow for this. Notably -

  1. The interface between "custom" and standard routines is not well defined
  2. Unit tests directly use the hs_custom_internal.h data structures in non-custom tests (e.g. see HS_MonitorUtilization_Test_CPUHogging, among others)

Furthermore, the code is simply in the same src directory as the rest of HS, so any customizations mean creating a fork of HS and the user having to manage that fork.

To Reproduce
Attempt to change the contents of HS_CustomData_t - such as removing the UtilMult and and UtilDiv fields and replacing with some other logic. This will break seeming unrelated code that assumes these fields exist.

Expected behavior
If HS depends on a computation working a certain way, it shouldn't be labeled as custom. Conversely, if a section of code is truly intended to be customized by the user, the interface into that custom function needs to be well-defined and no other parts of the code should assume it works a certain way or has certain fields in its global data.

Code snips
Example of a place where unit test is directly accessing custom fields from a non-custom test:

HS_CustomData.LastIdleTaskInterval = 1;
HS_CustomData.UtilMult1 = -3;
HS_CustomData.UtilMult2 = 1;
HS_CustomData.UtilDiv = 1;

System observed on:
N/A

Additional context
Could this custom logic potentially be refactored into a separate support library, so the user would not have to fork HS to customize these routines?

Reporter Info
Joseph Hickey, Vantage Systems, Inc.

Static analysis issues relative to flight code

Handful of static analysis issues in the "red" identified (non-Style issues). Need to resolve these.

Filter: -file:elf -file:ut -file:cfe -file:os -file:cf_ -file:_lab_app.c !(significance:style)

should resolve and/or disposition the higher ranked ones at minimum.

Note license restricts publishing issues.

Imported from GSFCCFS-1958

Simplify message action logic to avoid unreachable branch

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I reviewed the README file to see if the feature is in the major future work.
  • I performed a cursory search to see if the feature request is relevant, not redundant, nor in conflict with other tickets.

Is your feature request related to a problem? Please describe.
Branch can't be covered since there's no way to get to this decision with a ActionType <= HS_AMT_ACT_LAST_NONMSG:

HS/fsw/src/hs_monitors.c

Lines 182 to 184 in 2fc0dd9

if ((HS_AppData.MsgActsState == HS_STATE_ENABLED) &&
(ActionType > HS_AMT_ACT_LAST_NONMSG) &&
(ActionType <= (HS_AMT_ACT_LAST_NONMSG + HS_MAX_MSG_ACT_TYPES)))

This is due to ActionType already being checked here:

if ((ActionType != HS_AMT_ACT_NOACT) && (HS_AppData.AppMonCheckInCountdown[TableIndex] != 0))

All other cases of current implementation are handled. Only way to exercise this decision as false would be to introduce a bug.

Describe the solution you'd like
Slight refactor to simplify and allow full coverage - remove the NOACT case since it can never happen:

case HS_AMT_ACT_NOACT:

Also swap the logic to calculate MsgActsIndex first and just check the result for in-range.

Describe alternatives you've considered
Could do enabled check first, but it's already 9 levels deep. Better to save that for #5 to really clean up duplicated logic.

Additional context
None

Requester Info
Jacob Hageman - NASA/GSFC

Move interface definition files to "inc" location

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I reviewed the README file to see if the feature is in the major future work.
  • I performed a cursory search to see if the feature request is relevant, not redundant, nor in conflict with other tickets.

Is your feature request related to a problem? Please describe.
The interface definition files of all open source apps currently exist in the "src" directory.

Describe the solution you'd like
Create an "inc" dir to go with the "src" dir. Move the interface definitions into this location: "_msg.h", "_msgdefs.h", "_tbldefs.h", and "_events.h". Consider moving header files in both "platform_inc" and "mission_inc" to "inc"

Describe alternatives you've considered
Leaving as is.

Additional context
N/A

Requester Info
Justin Figueroa, Vantage Systems

Use CFE_MSG_CMD_HDR_INIT macro in Message Action Table implementation

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I reviewed the README file to see if the feature is in the major future work.
  • I performed a cursory search to see if the feature request is relevant, not redundant, nor in conflict with other tickets.

Is your feature request related to a problem? Please describe.
Current raw buffer/command implementation in MAT table isn't portable across different endian systems and is somewhat challenging/messy to implement.

Describe the solution you'd like
Similar to nasa/SC#35, the message action table could use the CFE_MSG_CMD_HDR_INIT macro and real command types to simplify table implementation.

Suggestion:
Typedef a union that contains each of the message types in the table, then define the array w/ the MAT info and unioned element

typedef union {cmda, cmdb} cmdbuff;
typedef struct {mat_info, cmdbuff} mat_element;
then mat_element[X] = ... where cmdbuff.cmda can set the header w/ the macro and actual elements of command directly

Describe alternatives you've considered
None

Additional context
Similar possible approach with SCH

Requester Info
Jacob Hageman - NASA/GSFC

Split "platform_cfg" into external and internal components

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I reviewed the README file to see if the feature is in the major future work.
  • I performed a cursory search to see if the feature request is relevant, not redundant, nor in conflict with other tickets.

Is your feature request related to a problem? Please describe.
Currently, all configurable items for the app are contained in a single hs_platform_cfg.h header file. This contains macro definitions that affect both the internal operation of the application (such as operational limits) as well as the external interface in CMD/TLM and table files.

Describe the solution you'd like
Split this header into two components, one that contains only public items (i.e. those that affect CMD/TLM/Table definitions) and one that contains private/internal items that are only used within the local application code and do not affect the interface.

Additional context
Mainly a scoping concern, separate files for separate scopes. These files could also be generated in the future, but scope still needs to be consistent.

Requester Info
Joseph Hickey, Vantage Systems, Inc.

Apps should check/verify msg BEFORE calling handler

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I reviewed the README file to see if the feature is in the major future work.
  • I performed a cursory search to see if the feature request is relevant, not redundant, nor in conflict with other tickets.

Is your feature request related to a problem? Please describe.
This remains an area with coding pattern discrepancies between CFE core and CFS apps, and also different between CFS apps to some degree as well.

The CFE core and sample app (which is supposed to be the example of "best practice") do validation on the message before calling the handler. For example:

        case SAMPLE_APP_NOOP_CC:
            if (SAMPLE_APP_VerifyCmdLength(&SBBufPtr->Msg, sizeof(SAMPLE_APP_NoopCmd_t)))
            {
                SAMPLE_APP_Noop((SAMPLE_APP_NoopCmd_t *)SBBufPtr);
            }

This is different from HS, which does a similar check, but done inside each handler, for example:

HS/fsw/src/hs_cmds.c

Lines 391 to 401 in 76915e9

void HS_EnableEventMonCmd(const CFE_SB_Buffer_t *BufPtr)
{
size_t ExpectedLength = sizeof(HS_EnableEventMonCmd_t);
int32 Status = CFE_SUCCESS;
/*
** Verify message packet length
*/
if (HS_VerifyMsgLength(&BufPtr->Msg, ExpectedLength))
{
/*

Describe the solution you'd like
CFS Apps should follow the best practices/patterns set forth in the framework code. (there are reasons for the pattern being recommended practice)

Additional context
The pattern recommended in the framework (checking before calling, as done in sample_app) has several advantages:

  1. Each command handler function has a unique type-safe prototype, that cannot be interchanged with another handler without triggering a type mismatch compiler error.
  2. All typecasting/conversions are confined to one place, and it is nearby to the place that the verification is done - which eases maintainability because check and conversion are all in close proximity and any mismatches will be more visible.
  3. It spreads out the cyclomatic complexity. In the non-recommended pattern, there is a case where the length check fails, and the entire handler is essentially skipped. This adds to the cyclomatic complexity of every handler. In the recommended pattern, those checks are done prior to the invocation of the handler, so the handler can focus solely on its intended purpose - doing the command itself.

Requester Info
Joseph Hickey, Vantage Systems, Inc.

Scrub configuration file for items that don't change

Suggest removing non-project configuration items from this file. Would help to limit this list to only items that projects should expect to manage.

Examples of things I wouldn't consider for project configuration - app name, wakeup pipe depth, table names, etc.

Note open ticket on CPU utilization/idle task so avoiding comments on those parameters since I expect this implementation to change.

Imported from GSFCCFS-1014

Recommended refactoring in HS_MonitorEvent

HS_MonitorEvent recommendation:

refactor common action logic for the different monitors into one function, and pass in unique info. Would avoid repeated logic for the same action from a different trigger.

finding from code review

Imported from GSFCCFS-1003

HS missing requirements for SetUtilDiagCmd, SetUtilParamsCmd, and UtilDiagReport (all hs_custom.c)

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I reviewed the README file to see if the feature is in the major future work.
  • I performed a cursory search to see if the feature request is relevant, not redundant, nor in conflict with other tickets.

Is your feature request related to a problem? Please describe.
Although these are in the custom section and may need adjustment depending on resolution of the hogging task related issues, the current implementation is missing associated requirements for HS_REPORT_DIAG_CC, HS_SET_UTIL_PARAMS_CC, HS_SET_UTIL_DIAG_CC:

#define HS_REPORT_DIAG_CC 12

#define HS_SET_UTIL_PARAMS_CC 13

#define HS_SET_UTIL_DIAG_CC 14

Describe the solution you'd like
All commands should have associated requirements, add.

Describe alternatives you've considered
None in the short term. Longer term may update behavior based on:

Additional context
Really should be at minimum in the draco-rc2 requirements documents (as well as many historical versions).

Requester Info
Jacob Hageman - NASA/GSFC

Ping for awareness: @ArielSAdamsNASA

Static analysis workflow failures due to style warnings

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I reviewed the README file to see if the feature is in the major future work.
  • I performed a cursory search to see if the feature request is relevant, not redundant, nor in conflict with other tickets.

Is your feature request related to a problem? Please describe.
Strict cppcheck fails in static analysis workflow, see https://github.com/nasa/HS/runs/6355951822?check_suite_focus=true:
'''
[fsw/src/hs_cmds.c:686] -> [fsw/src/hs_cmds.c:701]: (style) Variable 'Status' is reassigned a value before the old one has been used.
[fsw/src/hs_custom.c:90] -> [fsw/src/hs_custom.c:95]: (style) Variable 'Status' is reassigned a value before the old one has been used.
'''

Describe the solution you'd like
Fix

Describe alternatives you've considered
None

Additional context
None

Requester Info
Jacob Hageman - NASA/GSFC

Remove use of compiler extension in table file definitions

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I performed a cursory search to see if the bug report is relevant, not redundant, nor in conflict with other tickets.

Describe the bug
The __attribute__((__used__)) is a GNU compiler extension flag and such constructs should be avoided in portable code.

Describe the solution you'd like
Remove

Additional context
This flag is only in there to avoid a warning due to the use of static - but this object should not be static to begin with, then everything works without the use of special flags.

Requester Info
Joseph Hickey, Vantage Systems, Inc.

Cppcheck unread variable errors in hs_monitor.c

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I performed a cursory search to see if the bug report is relevant, not redundant, nor in conflict with other tickets.

Describe the bug
Cppcheck is currently reporting the following style issues:

severity location error id issue
style fsw/src/hs_monitors.c:641 unreadVariable Variable 'EntryResult' is assigned a value that is never used.
style fsw/src/hs_monitors.c:643 unreadVariable Variable 'ResourceType' is assigned a value that is never used.
style fsw/src/hs_monitors.c:644 unreadVariable Variable 'NullTerm' is assigned a value that is never used.

To Reproduce
Run Cppcheck workflow

Expected behavior
Should run clean

System observed on:
Github hosted runner

Reporter Info
Joseph Hickey, Vantage Systems, Inc.

New Cppcheck errors: '[unreadVariable]'

Checklist

  • I reviewed the Contributing Guide.
  • I performed a cursory search to see if the bug report is relevant, not redundant, nor in conflict with other tickets.

Describe the bug
Latest version of Cppcheck is issuing the following failures for HS:

fsw/src/hs_cmds.c:157:23: style: Variable 'Status' is assigned a value that is never used. [unreadVariable]
    int32  Status     = CFE_SUCCESS;
                      ^
fsw/src/hs_cmds.c:158:23: style: Variable 'TableIndex' is assigned a value that is never used. [unreadVariable]
    uint32 TableIndex = 0;
                      ^
fsw/src/hs_custom.c:267:22: style: Variable 'j' is assigned a value that is never used. [unreadVariable]
    uint32 j         = 0;
                     ^
fsw/src/hs_custom.c:268:22: style: Variable 'ThisValue' is assigned a value that is never used. [unreadVariable]
    uint32 ThisValue = 0;
                     ^
fsw/src/hs_custom.c:270:28: style: Variable 'Ordinal' is assigned a value that is never used. [unreadVariable]
    uint32 Ordinal         = 0;
                           ^
fsw/src/hs_custom.c:271:28: style: Variable 'NewOrdinalIndex' is assigned a value that is never used. [unreadVariable]
    uint32 NewOrdinalIndex = 0;
                           ^
fsw/src/hs_monitors.c:48:35: style: Variable 'Status' is assigned a value that is never used. [unreadVariable]
    int32            Status       = CFE_SUCCESS;
                                  ^
fsw/src/hs_monitors.c:50:35: style: Variable 'ActionType' is assigned a value that is never used. [unreadVariable]
    uint16           ActionType   = 0;
                                  ^
fsw/src/hs_monitors.c:228:35: style: Variable 'ActionType' is assigned a value that is never used. [unreadVariable]
    uint16           ActionType   = 0;
                                  ^
fsw/src/hs_monitors.c:458:24: style: Variable 'EntryResult' is assigned a value that is never used. [unreadVariable]
    int32  EntryResult = 0;
                       ^
fsw/src/hs_monitors.c:460:23: style: Variable 'ActionType' is assigned a value that is never used. [unreadVariable]
    uint16 ActionType = 0;
                      ^
fsw/src/hs_monitors.c:461:23: style: Variable 'CycleCount' is assigned a value that is never used. [unreadVariable]
    uint16 CycleCount = 0;
                      ^
fsw/src/hs_monitors.c:462:23: style: Variable 'NullTerm' is assigned a value that is never used. [unreadVariable]
    uint16 NullTerm   = 0;
                      ^
fsw/src/hs_monitors.c:549:24: style: Variable 'EntryResult' is assigned a value that is never used. [unreadVariable]
    int32  EntryResult = 0;
                       ^
fsw/src/hs_monitors.c:551:23: style: Variable 'ActionType' is assigned a value that is never used. [unreadVariable]
    uint16 ActionType = 0;
                      ^
fsw/src/hs_monitors.c:552:23: style: Variable 'EventID' is assigned a value that is never used. [unreadVariable]
    uint16 EventID    = 0;
                      ^
fsw/src/hs_monitors.c:553:23: style: Variable 'NullTerm' is assigned a value that is never used. [unreadVariable]
    uint16 NullTerm   = 0;
                      ^
fsw/src/hs_monitors.c:734:24: style: Variable 'EnableState' is assigned a value that is never used. [unreadVariable]
    uint16 EnableState = 0;
                       ^
fsw/src/hs_monitors.c:735:24: style: Variable 'EntryResult' is assigned a value that is never used. [unreadVariable]
    int32  EntryResult = 0;
                       ^
##[error]Process completed with exit code 255.

To Reproduce
Run the current version of Cppcheck on the current main branch HS source code.

Expected behavior
Cppcheck should pass without raising any errors.

Reporter Info
Avi @thnkslprpt

Add fsw/src to app target

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I performed a cursory search to see if the bug report is relevant, not redundant, nor in conflict with other tickets.

Describe the bug
Cannot build cert_testbed. target_include_directories(xx PUBLIC fsw/src) is needed to build tables, etc. Revise CMakeLists.txt

To Reproduce
Build in cert_testbed

Expected behavior
Error-free build

Code snips
If applicable, add references to the software.

System observed on:

  • Ubuntu 20.04

Additional context
N/A

Reporter Info
Justin Figueroa, Vantage Systems

Refactor "Compiled-out" code (ifdef)

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I performed a cursory search to see if the bug report is relevant, not redundant, nor in conflict with other tickets.

Describe the bug
HS contains a significant amount of code that can be compiled-out via #if HS_MAX_EXEC_CNT_SLOTS != 0

To comply with coding standards this should be refactored so it does not need to be compiled out.

To Reproduce
N/A

Expected behavior
Should not remove entire functions via #if as this can negatively affect unit testing. Switches like this increases the testing permutations required - technically should test it both ways, but we are not currently doing so.

System observed on:
N/A

Additional context
In this case the default value of HS_MAX_EXEC_CNT_SLOTS is 32, so this is compiled-in by default, but we never actually test the compiled-out option to see if it works correctly.

Requirement for this feature should be revisited first (i.e. is there a reason why we really need to have the option to set HS_MAX_EXEC_CNT_SLOTS to 0, as other configurable parameters do not have such an option).

If it is required to compile this out entirely, then a source-selection would be preferable over chunks of #if code.

Reporter Info
Joseph Hickey, Vantage Systems, Inc.

Compiling unit tests with cFS bootes-rc2

The provided unit tests do not compile with the bootes-rc2 release of cFS.

The unit test README states that "The ut-assert framework, stubs, and hooks are located in the directory cfe/tools/ut-assert". This is no longer the case. I found an older version of cfe that had this ut-assert folder and managed to get the unit tests to compile with that, but many of the unit tests failed, likely because the functions in ut-assert are out of date.

It seems that many of the functions within the old ut-assert folder used by these unit tests have moved to various places in cfe, osal, and psp and have since been updated. I am attempting to move files around and link libraries to compile with the updated cFS, but have not had any luck so far.

Below are the versions of cfe, osal, and psp I am using:

  • cfe v6.7.0+dev298
  • osal v5.0.0+dev250
  • psp v1.4.0+dev76

HS idle task only executes once and exits... reports CPU hogging all the time

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I performed a cursory search to see if the bug report is relevant, not redundant, nor in conflict with other tickets.

Describe the bug
HS idle task used to have a while loop (from 2.3.2 tag):

HS/fsw/src/hs_custom.c

Lines 60 to 84 in 192a10f

while (HS_CustomData.IdleTaskRunStatus == CFE_SUCCESS)
{
/* Check to see if we are to mark the time. */
if(((HS_CustomData.ThisIdleTaskExec & HS_CustomData.UtilMask) == HS_CustomData.UtilMask) &&
(HS_CustomData.ThisIdleTaskExec > HS_CustomData.UtilMask))
{
/* Entry and Exit markers are for easy time marking only; not performance */
CFE_ES_PerfLogEntry(HS_IDLETASK_PERF_ID);
/* Increment the child task Execution Counter */
CFE_ES_IncrementTaskCounter();
/* update stamp and array */
CFE_PSP_GetTime(&PSPTime);
HS_CustomData.UtilArray[HS_CustomData.UtilArrayIndex & HS_CustomData.UtilArrayMask] = (uint32) PSPTime.microsecs;
HS_CustomData.UtilArrayIndex++;
CFE_ES_PerfLogExit(HS_IDLETASK_PERF_ID);
}
/* Call the Utilization Tracking function */
HS_UtilizationIncrement();
}

Now just runs once:

HS/fsw/src/hs_custom.c

Lines 51 to 80 in c5ef5ac

void HS_IdleTask(void)
{
OS_time_t PSPTime = {0};
HS_CustomData.IdleTaskRunStatus = CFE_SUCCESS;
/* Check to see if we are to mark the time. */
if (((HS_CustomData.ThisIdleTaskExec & HS_CustomData.UtilMask) == HS_CustomData.UtilMask) &&
(HS_CustomData.ThisIdleTaskExec > HS_CustomData.UtilMask))
{
/* Entry and Exit markers are for easy time marking only; not performance */
CFE_ES_PerfLogEntry(HS_IDLETASK_PERF_ID);
/* Increment the child task Execution Counter */
CFE_ES_IncrementTaskCounter();
/* update stamp and array */
CFE_PSP_GetTime(&PSPTime);
HS_CustomData.UtilArray[HS_CustomData.UtilArrayIndex & HS_CustomData.UtilArrayMask] = (uint32)PSPTime.ticks;
HS_CustomData.UtilArrayIndex++;
CFE_ES_PerfLogExit(HS_IDLETASK_PERF_ID);
}
/* Call the Utilization Tracking function */
HS_UtilizationIncrement();
return;
} /* End of HS_IdleTask() */

To Reproduce
Running cFS with HS reports hogging every time...

EVS Port1 66/1/HS 61: CPU Hogging Detected
1980-012-14:03:25.80827 HS App: CPU Hogging Detected

Expected behavior
Task should run in a loop and realistic margin should be getting reported

System observed on:

  • OS: Linux
  • Versions: 2.4.1+

Additional context
None

Reporter Info
Jacob Hageman - NASA/GSFC

Uncovered condition for while loop in hs_custom.c

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I reviewed the README file to see if the feature is in the major future work.
  • I performed a cursory search to see if the feature request is relevant, not redundant, nor in conflict with other tickets.

Is your feature request related to a problem? Please describe.
There will always be room within the DiagValue/Count array here, so it'll match before j >= the limit. The conditions are not independent:

HS/fsw/src/hs_custom.c

Lines 319 to 336 in 2fc0dd9

while ((MatchFound == false) && (j < HS_UTIL_TIME_DIAG_ARRAY_LENGTH))
{
if (ThisValue == DiagValue[j])
{
DiagCount[j]++;
MatchFound = true;
}
else if (DiagValue[j] == 0xFFFFFFFF)
{
DiagValue[j] = ThisValue;
DiagCount[j]++;
MatchFound = true;
}
else
{
j++;
}
}

Describe the solution you'd like
Refactor

Describe alternatives you've considered
None

Additional context
None

Requester Info
Jacob Hageman - NASA/GSF

Add CodeQL to repository

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I reviewed the README file to see if the feature is in the major future work.
  • I performed a cursory search to see if the feature request is relevant, not redundant, nor in conflict with other tickets.

Is your feature request related to a problem? Please describe.
Use CodeQL for continuous integration

Describe the solution you'd like
Add CodeQL workflow

Describe alternatives you've considered
None

Additional context
Add any other context about the feature request here.

Requester Info
Haven Carlson

Health and Safety has extraneous code

IVV Severity: 4

Issue Category: Code
Issue Type: Extraneous Code
Count: 1

Description:
The Health and Safety App init function includes the extraneous assignment of Status to CFE_SUCCESS[1: line 355]; however, this value is never used again before the value of Status is reassigned[1: line 381].

Recommended Actions:
The assignment statement for Status can be removed.

Impact:
Defect impacting maintainability on current mission or reuse on future missions.

Imported from GSFCCFS-1901

Revisit coverage, update to 100% code/branch or write issues where unreachable

Failure: Coverage cs lines 99.9% functions 100.0% branches 99.1%
Failure: Coverage ds lines 99.6% functions 100.0% branches 97.7%
Failure: Coverage fm lines 93.8% functions 94.0% branches 89.1%
Failure: Coverage hs lines 100.0% functions 100.0% branches 98.1%
Failure: Coverage lc lines 99.6% functions 100.0% branches 94.3%
Failure: Coverage md lines 98.4% functions 100.0% branches 96.7%
Failure: Coverage sc lines 99.9% functions 100.0% branches 99.0%

Fix where possible, elsewise Issues should document all uncovered lines/branches and disposition (why it's ok as-is)

Imported from GSFCCFS-1935

Redundant comments (/* end of function */, /* end if */ etc.) and clean up empty lines.

Checklist

  • I reviewed the Contributing Guide.
  • I performed a cursory search to see if the bug report is relevant, not redundant, nor in conflict with other tickets.

Describe the bug
Copy of nasa/to_lab#68 and nasa/sample_app#111
There are quite a few redundant comments in the code, such as:

  • /* end of function */-type comments
  • /* end if */-type comments
  • function header comments which include the function name

Another minor issue has to do with empty lines:
a) unnecessary empty lines (e.g. first line after the opening brace of a function/struct, or the last line before the closing brace - the latter apparently sometimes triggers the CI format checks).
b) missing empty lines between functions (i.e. closing brace of last function, then next function beginning on the immediately next line without an empty line in between)

The unnecessary empty lines (at the beginning or end of a function, for example) represent a low single-digit percentage of the cases (the vast majority of functions/structs do not have these extra empty lines), so there is an argument to remove them purely for consistency, not just due to them being redundant and triggering the CI format checks.

Expected behavior
Remove redundant comments to reduce clutter and inconsistency in the code, and improve readability.

Reporter Info
@thnkslprpt

Performance ID usage is inconsistent in HS app

Anytime a task goes into a system delay (for example a pend on message receipt or task delay), an app should do the following:

CFE_ES_PerfLogExit(<performance_id>);

CFE_ES_PerfLogEntry(<performance_id>);

There appear to be some cases in HS where a task delay called, but the performance monitor is not exited.
Other apps should also be checked for consistent usage.

Imported from GSFCCFS-1208

Remove stray terminators

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I reviewed the README file to see if the feature is in the major future work.
  • I performed a cursory search to see if the feature request is relevant, not redundant, nor in conflict with other tickets.

Is your feature request related to a problem? Please describe.


Describe the solution you'd like
Remove

Describe alternatives you've considered
None

Additional context
None

Requester Info
Jacob Hageman - NASA/GSFC

HS_IdleTask has no cancellation point

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I reviewed the README file to see if the feature is in the major future work.
  • I performed a cursory search to see if the feature request is relevant, not redundant, nor in conflict with other tickets.

Is your feature request related to a problem? Please describe.
The default HS_IdleTask has no cancellation point, so hangs up on Linux for Cntrl-C.

Describe the solution you'd like
Add a cancellation point

Describe alternatives you've considered
None

Additional context
None

Requester Info
Jacob Hageman - NASA/GSFC

@excaliburtb

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    ๐Ÿ–– Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. ๐Ÿ“Š๐Ÿ“ˆ๐ŸŽ‰

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google โค๏ธ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.