From 6af96c3d5a2cf28dd31dfdc4278ecfa93a4d78ce Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Thu, 9 Mar 2023 13:01:40 -0500 Subject: [PATCH 1/4] Fix #2253, update naming convention document Updates to cover event ID recommendations --- docs/cFS_IdentifierNamingConvention.md | 97 +++++++++++++++++--------- 1 file changed, 65 insertions(+), 32 deletions(-) diff --git a/docs/cFS_IdentifierNamingConvention.md b/docs/cFS_IdentifierNamingConvention.md index 79ca8c6f6..c64493bcb 100644 --- a/docs/cFS_IdentifierNamingConvention.md +++ b/docs/cFS_IdentifierNamingConvention.md @@ -22,9 +22,9 @@ four goals: The methodology sections below include the process in which the convention was selected along with some examples and proposed names. -## Background +## Background -The following are inputs from Database Managers and users from TIRS, TIRS2, +The following are inputs from Database Managers and users from TIRS, TIRS2, MMS, ATLAS, DISCOVER that serve as the basis for the naming convention proposal. ### Commands @@ -61,23 +61,23 @@ Some general rules/recommendations to follow for cFS identifiers: 1. Use command and parameter names that convey the intent of the corresponding command or parameter - -2. Use the simplest format possible for command names and parameters. - + +2. Use the simplest format possible for command names and parameters. + * Avoid abbreviation unless it is common usage/knowledge and/or - required to avoid character limit. + required to avoid character limit. -3. Remove words that are inherent in that type of the mnemonic. +3. Remove words that are inherent in that type of the mnemonic. - * `Noop` instead of `NoopCmd` + * `Noop` instead of `NoopCmd` * `HK` instead of `HK_Pkt_Tlm` if it is always a telemetry packet - * `Name` instead of `FileName` or `Path` instead of `FilePath` if the + * `Name` instead of `FileName` or `Path` instead of `FilePath` if the command only deals with files - + 4. Individual identifier names should consist of only alphanumeric characters - and underscores, and must _not_ begin with a digit as some languages disallow + and underscores, and must _not_ begin with a digit as some languages disallow identifiers that begin with digits (i.e. `ONEHZ` instead of `1HZ`). - + CamelCase is preferred for terms where possible, but in cases where existing terms are in common use the existing case should be preserved. @@ -90,25 +90,25 @@ Some general rules/recommendations to follow for cFS identifiers: * Application Name (aka Namespace, e.g. `CFE_ES`, `SCH`, `TO_LAB`, etc.) * Interface Name (aka Topic, e.g. `CMD`, `HK`, etc.) * Subcommand Name (if applicable, e.g. `Noop`, `Restart`, etc.) - + Example: `SPACECRAFT/cpu1/CFE_ES/CMD/Noop` - + 6. A consistent character should be used as a separator between terms, but the - specific separator character depends on what is most appropriate for the system, - tool, or language in use. Tools may use a forward slash (`/`), an + specific separator character depends on what is most appropriate for the system, + tool, or language in use. Tools may use a forward slash (`/`), an underscore (`_`), a period (`.`), or a double-colon (`::`) as necessary. - + Note that underscores are allowed characters within terms, so while it is acceptable to concatenate using underscores, it may not be possible to perform the reverse operation and determine the original terms in this case. -7. Terms at the far left or far right of a multi-part identifier may be omitted where +7. Terms at the far left or far right of a multi-part identifier may be omitted where they are not known or not necessary based on the context. * Modular software components should omit the Spacecraft and Processor names from the left hand side of identifiers used in flight software. - - * When referring to an entire application, such as in a test script, one may omit the + + * When referring to an entire application, such as in a test script, one may omit the Interface and Subcommand terms from the right hand side of identifiers. __NOTE__: a sufficient number of terms must always be used in order to ensure uniqueness @@ -118,10 +118,10 @@ Some general rules/recommendations to follow for cFS identifiers: ## Applicability to Actions / Commands This section specifically concerns the last term within a fully qualified command name, which -indicates the specific command to perform. As mentioned previously, command names should clearly -indicate the action being taken as well as the subject of the action. +indicates the specific command to perform. As mentioned previously, command names should clearly +indicate the action being taken as well as the subject of the action. -The recommended convention for command names is a CamelCase identifier consisting of the +The recommended convention for command names is a CamelCase identifier consisting of the action first, followed by the subject. Example action terms: @@ -147,22 +147,22 @@ Example subjects: __NOTE__: Clarity of the command name should supersede this recommendation where relevant. In cases where strictly following this convention would lead to command names that are less clear or do not -adequately convey intent, it is acceptable to not follow this convention. This recommendation is +adequately convey intent, it is acceptable to not follow this convention. This recommendation is mainly to avoid command identifiers which are a mixture of `CounterReset` and `ResetCounter`, etc. ## Applicability to Flight Software -This section documents some additional rules/recommendations to follow specifically for flight +This section documents some additional rules/recommendations to follow specifically for flight software code implemented in C or C++. In general, to avoid name conflicts, all global identifiers in software code must employ some level of namespace protection. -In C, any global software identifiers or terms should begin with the application name followed by -an underscore, e.g. `CFE_FS_FILE_CONTENT_ID`. +In C, any global software identifiers or terms should begin with the application name followed by +an underscore, e.g. `CFE_FS_FILE_CONTENT_ID`. -In C++, where applicable, the `namespace` keyword with the application name may be used here instead +In C++, where applicable, the `namespace` keyword with the application name may be used here instead of an underscore prefix as described below. ### Typedefs @@ -198,9 +198,9 @@ Rationale: When creating enumeration labels that exist in the global namespace (which is _always_ the case for C code), the label and type names must be constructed using a consistent convention. The labels should be prefixed -using the enumerated type name without the suffix, concatenated using an underscore. +using the enumerated type name without the suffix, concatenated using an underscore. -For example, in the CFE_EVS application, the "LogMode" parameter has two possible options, "OVERWRITE" +For example, in the CFE_EVS application, the "LogMode" parameter has two possible options, "OVERWRITE" and "DISCARD". Using this convention, the specific identifiers would be as follows: Basic type name in external tool/ground system: `CFE_EVS/LogMode` @@ -215,10 +215,44 @@ Label names in C code: Note that this recommendation is specific for C and C++ where the label names are in global scope. In other languages or systems where enumeration label is not automatically a global identifier, this prefix may not be required. +## Applicability to Events and Event Identifiers + +Most CFS applications also use event services (EVS) to send status messages, outside of the normal Telemetry stream. In +addition to the message text, events also contain a numeric event identifier (referred to as an "EventID" or "EID"). + +As these identifiers serve to associate a human-readable name with the numeric ID for event labeling purposes, an +enumeration is a natural fit for this purpose as well. The enumeration labels should follow all the same naming +conventions as any other enumeration, described above. + +For consistency, an enumeration used for this purpose should be named `EventID`, and the C typedef name for the +enumeration should follow the same pattern of `_EventID_Enum_t`. + +The labels should convey the event name as well as the type of the event, separated by an underscore, in all capital +letters. This should follow the general pattern of `_` where "EVT" is a 3-letter abbreviation. + +For example: + +| Event ID Label | Application | Event Description | Event Type | +| :------------------------: | :---------------: | :---------------: | :-----------: | +| `CFE_EVS_EventID_NOOP_INF` | CFE Event Service | NOOP Command | Informational | +| `HS_EventID_CR_PIPE_ERR` | Health & Safety | Create Pipe Error | Error | + +Common events should be named consistently across applications for ease of recognition. + +Common command/event descriptions include: + +| Event Name | Event Description | Event Type | +| :------------: | :--------------------------------------: | :-----------: | +| `MID_ERR` | Invalid Message ID Received | Error | +| `LEN_ERR` | Incorrect Message Length | Error | +| `CC_ERR` | Invalid Command Code Received | Error | +| `NOOP_INF` | No-op Command Success | Informational | +| `INIT_INF` | Applicaiton Initialization Success | Informational | +| `RESET_INF` | Reset Command Counters Command Success | Informational | # Appendix: Abbreviation Guide -In general it is preferable to use entire words unless this would create an identifier which is +In general it is preferable to use entire words unless this would create an identifier which is unacceptably long. However, in cases where words do need to be shortened, it is beneficial to be consistent in the abbreviated term. @@ -239,4 +273,3 @@ be abbreviated (e.g. use Error, not Err). Enable Ena Disable Dis Subscription Subscr - From 54019f5a7e7c7b8971b4e883a46f9979d23e8377 Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Tue, 11 Apr 2023 11:15:31 -0400 Subject: [PATCH 2/4] Fix #2287, remove return value doxygen markup The CFE_ES_TaskRecordSetFree() function is a void, it should not have any documentation about return value. Remove this line. Appears to be a cut and paste error, produces a warning in newer Doxygen versions. --- modules/es/fsw/src/cfe_es_resource.h | 1 - 1 file changed, 1 deletion(-) diff --git a/modules/es/fsw/src/cfe_es_resource.h b/modules/es/fsw/src/cfe_es_resource.h index a0ecc3eee..f37525f27 100644 --- a/modules/es/fsw/src/cfe_es_resource.h +++ b/modules/es/fsw/src/cfe_es_resource.h @@ -468,7 +468,6 @@ static inline void CFE_ES_TaskRecordSetUsed(CFE_ES_TaskRecord_t *TaskRecPtr, CFE * that are known to refer to an actual table location (i.e. non-null). * * @param[in] TaskRecPtr pointer to task table entry - * @returns true if the entry is in use/configured, or false if it is free/empty */ static inline void CFE_ES_TaskRecordSetFree(CFE_ES_TaskRecord_t *TaskRecPtr) { From 810d20375c2988a228f73081d423f2418507457f Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Thu, 13 Apr 2023 13:27:42 -0400 Subject: [PATCH 3/4] Fix #2289, implement common search routine for config files Adds a 'cfe_locate_implementation_file()' function that provides a consistent means to find a file within the ${MISSION_DEFS} subdirectory. Similar locate/search logic was necessary in a few places, for generate_config_includefile as well as add_cfe_tables, but these were not consistent in the paths or ordering used. Using this function should make them consistent. --- cmake/global_functions.cmake | 140 +++++++++++++++++++++++++---------- 1 file changed, 101 insertions(+), 39 deletions(-) diff --git a/cmake/global_functions.cmake b/cmake/global_functions.cmake index d007d614a..9a50fd94d 100644 --- a/cmake/global_functions.cmake +++ b/cmake/global_functions.cmake @@ -3,13 +3,91 @@ # cFS Mission global CMake function definitions # # This is included by the top-level script and can define -# common routines and variables that may be referenced in both +# common routines and variables that may be referenced in both # the mission (non-arch) and arch-specific builds # ################################################################## include(CMakeParseArguments) +################################################################## +# +# FUNCTION: cfe_locate_implementation_file +# +# The CFE/CFS build permits users to modify or tune system behavior by +# providing customized versions of configuration files and tables. +# +# This function locates the preferred version of a file to use, by following +# a priority-based search scheme. Users can put their preferred version of +# a file in their MISSION_DEFS directory, based on a file naming convention. +# +# If no version of the file is found there, a default/fallback version can +# be specified as well, which can be part of the module source. +# +function(cfe_locate_implementation_file OUTPUT_VAR FILE_NAME) + + cmake_parse_arguments(LOCATEIMPL_ARG "OPTIONAL;ALLOW_LIST" "FALLBACK_FILE" "PREFIX;SUBDIR" ${ARGN}) + + # Always check for filename matches directly in the MISSION_DEFS dir + set(IMPL_SEARCH_BASEDIRS "${MISSION_DEFS}/") + # The prefix could also be a subdir, but do not repeat the mission name + foreach (PREFIX ${LOCATEIMPL_ARG_PREFIX}) + if (NOT "${MISSIONCONFIG}" STREQUAL "${PREFIX}") + list(APPEND IMPL_SEARCH_BASEDIRS "${MISSION_DEFS}/${PREFIX}/") + endif() + endforeach() + set(ADD_SUBDIRS) + foreach (SUBDIR ${LOCATEIMPL_ARG_SUBDIR}) + foreach (BASEDIR ${IMPL_SEARCH_BASEDIRS}) + list(APPEND ADD_SUBDIRS "${BASEDIR}${SUBDIR}/") + endforeach() + endforeach() + list(APPEND IMPL_SEARCH_BASEDIRS ${ADD_SUBDIRS}) + + # Build the list of possible locations for this file in REVERSE priority order + set(IMPL_SEARCH_PATH) + if (LOCATEIMPL_ARG_FALLBACK_FILE) + if (IS_ABSOLUTE "${LOCATEIMPL_ARG_FALLBACK_FILE}") + list(APPEND IMPL_SEARCH_PATH "${LOCATEIMPL_ARG_FALLBACK_FILE}") + else() + list(APPEND IMPL_SEARCH_PATH "${CMAKE_CURRENT_LIST_DIR}/${LOCATEIMPL_ARG_FALLBACK_FILE}") + endif() + endif() + + # Check for the existence of the file in each of the dirs + foreach(BASEDIR ${IMPL_SEARCH_BASEDIRS}) + list(APPEND IMPL_SEARCH_PATH "${BASEDIR}${FILE_NAME}") + + # A target-specific prefixed filename gets priority over a direct filename match + # But do not include this variant if the prefix is already part of the basedir + foreach (PREFIX ${LOCATEIMPL_ARG_PREFIX}) + if (NOT "${BASEDIR}" MATCHES "/${PREFIX}/") + list(APPEND IMPL_SEARCH_PATH "${BASEDIR}${PREFIX}_${FILE_NAME}") + endif() + endforeach() + endforeach() + + set(SELECTED_FILE) + foreach (CHECK_FILE ${IMPL_SEARCH_PATH}) + if (EXISTS "${CHECK_FILE}") + list(APPEND SELECTED_FILE ${CHECK_FILE}) + endif() + endforeach() + + if (SELECTED_FILE) + message(STATUS "Using file: ${SELECTED_FILE} for ${FILE_NAME}") + elseif(NOT LOCATEIMPL_ARG_OPTIONAL) + message(FATAL_ERROR "No implementation for ${FILE_NAME} found") + endif() + + # Export the result to the caller + if (NOT LOCATEIMPL_ARG_ALLOW_LIST AND SELECTED_FILE) + list(GET SELECTED_FILE -1 SELECTED_FILE) + endif() + set(${OUTPUT_VAR} ${SELECTED_FILE} PARENT_SCOPE) + +endfunction() + ################################################################## # # FUNCTION: generate_c_headerfile @@ -78,48 +156,32 @@ function(generate_config_includefile) set(GENCONFIG_ARG_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/inc") endif (NOT GENCONFIG_ARG_OUTPUT_DIRECTORY) - set(WRAPPER_FILE_CONTENT) - set(ITEM_FOUND FALSE) + if (IS_CFS_ARCH_BUILD) + set(ARCH_PREFIXES ${BUILD_CONFIG}) + else() + set(ARCH_PREFIXES) + endif() - # Assemble a list of file names to test for - # Check for existence of a file in defs directory with an exact matching name - # Then Check for existence of file(s) with a matching prefix+suffix - set(CHECK_PATH_LIST "${MISSION_DEFS}/${GENCONFIG_ARG_FILE_NAME}") if (GENCONFIG_ARG_MATCH_SUFFIX) - foreach(PREFIX ${GENCONFIG_ARG_PREFIXES} ${GENCONFIG_ARG_UNPARSED_ARGUMENTS}) - list(APPEND CHECK_PATH_LIST "${MISSION_DEFS}/${PREFIX}_${GENCONFIG_ARG_MATCH_SUFFIX}") - endforeach() - endif(GENCONFIG_ARG_MATCH_SUFFIX) - - # Check for existence of files, and add to content if present - # Note that all files are appended/concatenated together. - foreach(SRC_LOCAL_PATH ${CHECK_PATH_LIST}) - if (EXISTS "${SRC_LOCAL_PATH}") - file(TO_NATIVE_PATH "${SRC_LOCAL_PATH}" SRC_NATIVE_PATH) - list(APPEND WRAPPER_FILE_CONTENT "#include \"${SRC_NATIVE_PATH}\"\n") - set(ITEM_FOUND TRUE) - else() - list(APPEND WRAPPER_FILE_CONTENT "/* ${SRC_LOCAL_PATH} does not exist */\n") - endif (EXISTS "${SRC_LOCAL_PATH}") - endforeach() - - # If _no_ files were found in the above loop, - # then check for and use the fallback file. - # (if specified by the caller it should always exist) - # Also produce a message on the console showing whether mission config or fallback was used - if (ITEM_FOUND) - message(STATUS "Generated ${GENCONFIG_ARG_FILE_NAME} from ${MISSION_DEFS} configuration") - elseif (GENCONFIG_ARG_FALLBACK_FILE) - file(TO_NATIVE_PATH "${GENCONFIG_ARG_FALLBACK_FILE}" SRC_NATIVE_PATH) - list(APPEND WRAPPER_FILE_CONTENT - "\n\n/* No configuration for ${GENCONFIG_ARG_FILE_NAME}, using fallback */\n" - "#include \"${GENCONFIG_ARG_FALLBACK_FILE}\"\n") - message(STATUS "Using ${GENCONFIG_ARG_FALLBACK_FILE} for ${GENCONFIG_ARG_FILE_NAME}") + set(TGTFILE ${GENCONFIG_ARG_MATCH_SUFFIX}) else() - message("ERROR: No implementation for ${GENCONFIG_ARG_FILE_NAME} found") - message(FATAL_ERROR "Tested: ${CHECK_PATH_LIST}") + set(TGTFILE ${GENCONFIG_ARG_FILE_NAME}) endif() + # Use the common search function to find the candidate(s) + cfe_locate_implementation_file(SRC_LOCAL_PATH_LIST "${TGTFILE}" + ALLOW_LIST + FALLBACK_FILE "${GENCONFIG_ARG_FALLBACK_FILE}" + PREFIX ${GENCONFIG_ARG_PREFIXES} ${ARCH_PREFIXES} + SUBDIR config + ) + + set(WRAPPER_FILE_CONTENT) + foreach(SELECTED_FILE ${SRC_LOCAL_PATH_LIST}) + file(TO_NATIVE_PATH "${SELECTED_FILE}" SRC_NATIVE_PATH) + list(APPEND WRAPPER_FILE_CONTENT "#include \"${SRC_NATIVE_PATH}\"\n") + endforeach() + # Generate a header file generate_c_headerfile("${GENCONFIG_ARG_OUTPUT_DIRECTORY}/${GENCONFIG_ARG_FILE_NAME}" ${WRAPPER_FILE_CONTENT}) @@ -139,7 +201,7 @@ endfunction(generate_config_includefile) # # This function then sets up the following variables in the global scope: # TGTSYS_LIST: list of CPU architectures used in the build. Note this -# will always contain a "native" target (for tools at least) which +# will always contain a "native" target (for tools at least) which # is forced to be last. # MISSION_APPS: list of all applications in this build # MISSION_PSPMODULES: list of all psp modules in this build From b06c3aaba50ee1d30854bff235298ade554d4867 Mon Sep 17 00:00:00 2001 From: Dylan Date: Thu, 13 Apr 2023 15:35:37 -0400 Subject: [PATCH 4/4] Bump to v7.0.0-rc4+dev268 --- CHANGELOG.md | 6 ++++++ modules/core_api/fsw/inc/cfe_version.h | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5fe316a04..3d9c1e652 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## Development Build: v7.0.0-rc4+dev268 +- update naming convention document +- remove return value doxygen markup +- implement common search routine for config files +- See , , and + ## Development Build: v7.0.0-rc4+dev260 - add more generic status codes - separate dispatcher for messages diff --git a/modules/core_api/fsw/inc/cfe_version.h b/modules/core_api/fsw/inc/cfe_version.h index a15334cdb..33491ac6e 100644 --- a/modules/core_api/fsw/inc/cfe_version.h +++ b/modules/core_api/fsw/inc/cfe_version.h @@ -26,7 +26,7 @@ #define CFE_VERSION_H /* Development Build Macro Definitions */ -#define CFE_BUILD_NUMBER 260 /**< @brief Development: Number of development git commits since CFE_BUILD_BASELINE */ +#define CFE_BUILD_NUMBER 268 /**< @brief Development: Number of development git commits since CFE_BUILD_BASELINE */ #define CFE_BUILD_BASELINE "v7.0.0-rc4" /**< @brief Development: Reference git tag for build number */ /* See \ref cfsversions for definitions */