From ee6c43d4d7f0d06efe95bd59d2df92d5b9055a7e Mon Sep 17 00:00:00 2001 From: Jacob Hageman Date: Mon, 24 May 2021 18:36:34 +0000 Subject: [PATCH 1/2] Fix #1569, Remove unused CFE_SB_NO_SUBSCRIBERS --- modules/core_api/fsw/inc/cfe_error.h | 9 --------- 1 file changed, 9 deletions(-) diff --git a/modules/core_api/fsw/inc/cfe_error.h b/modules/core_api/fsw/inc/cfe_error.h index 47259cc1a..c0d3187cd 100644 --- a/modules/core_api/fsw/inc/cfe_error.h +++ b/modules/core_api/fsw/inc/cfe_error.h @@ -779,15 +779,6 @@ typedef int32 CFE_Status_t; */ #define CFE_SB_MAX_DESTS_MET ((CFE_Status_t)0xca00000a) -/** - * @brief No Subscribers - * - * This error code is returned by the #CFE_SB_Unsubscribe API if there has - * not been an entry in the routing tables for the MsgId/PipeId given as - * parameters. - */ -#define CFE_SB_NO_SUBSCRIBERS ((CFE_Status_t)0xca00000b) - /** * @brief Internal Error * From d6c00911c60d57dd6b29176e7c66a874bb8934e1 Mon Sep 17 00:00:00 2001 From: Jacob Hageman Date: Mon, 24 May 2021 18:45:05 +0000 Subject: [PATCH 2/2] Fix #1452, Document cleanup from SB/MSG/SBR review --- .../core_api/fsw/inc/cfe_msg_api_typedefs.h | 2 +- modules/core_api/fsw/inc/cfe_sb.h | 19 +++++++++++-------- .../core_api/fsw/inc/cfe_sb_api_typedefs.h | 8 +++++--- modules/core_private/fsw/inc/cfe_sbr.h | 2 +- .../msg/option_inc/default_cfe_msg_hdr_pri.h | 3 ++- .../option_inc/default_cfe_msg_hdr_priext.h | 3 ++- modules/sb/fsw/src/cfe_sb_api.c | 4 ++-- modules/sbr/CMakeLists.txt | 2 +- 8 files changed, 25 insertions(+), 18 deletions(-) diff --git a/modules/core_api/fsw/inc/cfe_msg_api_typedefs.h b/modules/core_api/fsw/inc/cfe_msg_api_typedefs.h index b91001bd6..43e89510c 100644 --- a/modules/core_api/fsw/inc/cfe_msg_api_typedefs.h +++ b/modules/core_api/fsw/inc/cfe_msg_api_typedefs.h @@ -45,7 +45,7 @@ /* * Types */ -typedef size_t CFE_MSG_Size_t; /**< \brief Message size (CCSDS needs uint32 for max size) */ +typedef size_t CFE_MSG_Size_t; /**< \brief Message size, note CCSDS maximum is UINT16_MAX+7 */ typedef uint32 CFE_MSG_Checksum_t; /**< \brief Message checksum (Oversized to avoid redefine) */ typedef uint16 CFE_MSG_FcnCode_t; /**< \brief Message function code */ typedef uint16 CFE_MSG_HeaderVersion_t; /**< \brief Message header version */ diff --git a/modules/core_api/fsw/inc/cfe_sb.h b/modules/core_api/fsw/inc/cfe_sb.h index 3967688bd..0a04d7a18 100644 --- a/modules/core_api/fsw/inc/cfe_sb.h +++ b/modules/core_api/fsw/inc/cfe_sb.h @@ -154,7 +154,7 @@ CFE_Status_t CFE_SB_PipeId_ToIndex(CFE_SB_PipeId_t PipeID, uint32 *Idx); ** ** \param[in] PipeId The pipe ID of the pipe to set options on. ** -** \param[in] Opts A bit field of options. +** \param[in] Opts A bit field of options: \ref CFESBPipeOptions ** ** \return Execution status, see \ref CFEReturnCodes ** \retval #CFE_SUCCESS \copybrief CFE_SUCCESS @@ -173,7 +173,7 @@ CFE_Status_t CFE_SB_SetPipeOpts(CFE_SB_PipeId_t PipeId, uint8 Opts); ** ** \param[in] PipeId The pipe ID of the pipe to get options from. ** -** \param[out] *OptsPtr A bit field of options. +** \param[out] *OptsPtr A bit field of options: \ref CFESBPipeOptions ** ** \return Execution status, see \ref CFEReturnCodes ** \retval #CFE_SUCCESS \copybrief CFE_SUCCESS @@ -312,8 +312,9 @@ CFE_Status_t CFE_SB_Subscribe(CFE_SB_MsgId_t MsgId, CFE_SB_PipeId_t PipeId); ** the specified message ID. This is similar to #CFE_SB_SubscribeEx ** with the Quality field set to #CFE_SB_DEFAULT_QOS and MsgLim set ** to #CFE_PLATFORM_SB_DEFAULT_MSG_LIMIT, but will not report the subscription. -** Subscription Reporting is enabled for interprocessor communication -** by way of the Software Bus Network (SBN) Application. +** +** Software Bus Network (SBN) application is an example use case, +** where local subscriptions should not be reported to peers. ** ** \par Assumptions, External Events, and Notes: ** - This API is typically only used by Software Bus Network (SBN) Application @@ -346,7 +347,8 @@ CFE_Status_t CFE_SB_SubscribeLocal(CFE_SB_MsgId_t MsgId, CFE_SB_PipeId_t PipeId, ** list for the specified message ID. ** ** \par Assumptions, External Events, and Notes: -** None +** If the Pipe is not subscribed to MsgId, the CFE_SB_UNSUB_NO_SUBS_EID +** event will be generated and #CFE_SUCCESS will be returned ** ** \param[in] MsgId The message ID of the message to be unsubscribed. ** @@ -355,7 +357,6 @@ CFE_Status_t CFE_SB_SubscribeLocal(CFE_SB_MsgId_t MsgId, CFE_SB_PipeId_t PipeId, ** ** \return Execution status, see \ref CFEReturnCodes ** \retval #CFE_SUCCESS \copybrief CFE_SUCCESS -** \retval #CFE_SB_NO_SUBSCRIBERS \copybrief CFE_SB_NO_SUBSCRIBERS ** \retval #CFE_SB_INTERNAL_ERR \copybrief CFE_SB_INTERNAL_ERR ** ** \sa #CFE_SB_Subscribe, #CFE_SB_SubscribeEx, #CFE_SB_SubscribeLocal, #CFE_SB_UnsubscribeLocal @@ -371,7 +372,9 @@ CFE_Status_t CFE_SB_Unsubscribe(CFE_SB_MsgId_t MsgId, CFE_SB_PipeId_t PipeId); ** list for the specified message ID on the current CPU. ** ** \par Assumptions, External Events, and Notes: -** - This API is typically only used by Software Bus Network (SBN) Application +** This API is typically only used by Software Bus Network (SBN) Application. +** If the Pipe is not subscribed to MsgId, the CFE_SB_UNSUB_NO_SUBS_EID +** event will be generated and #CFE_SUCCESS will be returned ** ** \param[in] MsgId The message ID of the message to be unsubscribed. ** @@ -380,7 +383,6 @@ CFE_Status_t CFE_SB_Unsubscribe(CFE_SB_MsgId_t MsgId, CFE_SB_PipeId_t PipeId); ** ** \return Execution status, see \ref CFEReturnCodes ** \retval #CFE_SUCCESS \copybrief CFE_SUCCESS -** \retval #CFE_SB_NO_SUBSCRIBERS \copybrief CFE_SB_NO_SUBSCRIBERS ** \retval #CFE_SB_INTERNAL_ERR \copybrief CFE_SB_INTERNAL_ERR ** ** \sa #CFE_SB_Subscribe, #CFE_SB_SubscribeEx, #CFE_SB_SubscribeLocal, #CFE_SB_Unsubscribe @@ -489,6 +491,7 @@ CFE_Status_t CFE_SB_ReceiveBuffer(CFE_SB_Buffer_t **BufPtr, CFE_SB_PipeId_t Pipe ** It will automatically be freed by SB once all recipients have finished reading it. ** -# Applications must not de-reference the message pointer (for reading ** or writing) after the call to CFE_SB_TransmitBuffer(). +** -# If #CFE_SB_ReleaseMessageBuffer should be used only if a message is not transmitted ** ** \param[in] MsgSize The size of the SB message buffer the caller wants ** (including the SB message header). diff --git a/modules/core_api/fsw/inc/cfe_sb_api_typedefs.h b/modules/core_api/fsw/inc/cfe_sb_api_typedefs.h index 5cc2e9366..0f7873da0 100644 --- a/modules/core_api/fsw/inc/cfe_sb_api_typedefs.h +++ b/modules/core_api/fsw/inc/cfe_sb_api_typedefs.h @@ -110,11 +110,13 @@ */ #define CFE_SB_INVALID_PIPE CFE_SB_PIPEID_C(CFE_RESOURCEID_UNDEFINED) -/* -** Pipe option bit fields. -*/ +/** + * @defgroup CFESBPipeOptions cFE SB Pipe options + * @{ + */ #define CFE_SB_PIPEOPTS_IGNOREMINE \ 0x00000001 /**< \brief Messages sent by the app that owns this pipe will not be sent to this pipe. */ +/**@}*/ #define CFE_SB_DEFAULT_QOS ((CFE_SB_Qos_t) {0}) /**< \brief Default Qos macro */ diff --git a/modules/core_private/fsw/inc/cfe_sbr.h b/modules/core_private/fsw/inc/cfe_sbr.h index efee0e056..2b351b74b 100644 --- a/modules/core_private/fsw/inc/cfe_sbr.h +++ b/modules/core_private/fsw/inc/cfe_sbr.h @@ -50,7 +50,7 @@ void CFE_SBR_Init(void); /** - * \brief Add a route for the given a message id + * \brief Add a route for the given message id * * Called for the first subscription to a message ID, uses up one * element in the routing table. Assumes check for existing diff --git a/modules/msg/option_inc/default_cfe_msg_hdr_pri.h b/modules/msg/option_inc/default_cfe_msg_hdr_pri.h index ca27a0cd3..dbc7336ee 100644 --- a/modules/msg/option_inc/default_cfe_msg_hdr_pri.h +++ b/modules/msg/option_inc/default_cfe_msg_hdr_pri.h @@ -88,7 +88,8 @@ struct CFE_MSG_TelemetryHeader { CFE_MSG_Message_t Msg; /**< \brief Base message */ CFE_MSG_TelemetrySecondaryHeader_t Sec; /**< \brief Secondary header */ - uint8 Spare[4]; /**< \brief Padding to end on 64 bit boundary */ + uint8 Spare[4]; /**< \brief Pad to avoid compiler padding if payload + requires 64 bit alignment */ }; #endif /* DEFAULT_CFE_MSG_HDR_PRI_H */ diff --git a/modules/msg/option_inc/default_cfe_msg_hdr_priext.h b/modules/msg/option_inc/default_cfe_msg_hdr_priext.h index 233f9c799..3205aca87 100644 --- a/modules/msg/option_inc/default_cfe_msg_hdr_priext.h +++ b/modules/msg/option_inc/default_cfe_msg_hdr_priext.h @@ -74,7 +74,8 @@ typedef struct CFE_MSG_Message_t Msg; /**< \brief Base message */ CFE_MSG_CommandSecondaryHeader_t Sec; /**< \brief Secondary header */ - uint8 Spare[4]; /**< /brief Padding to end on 64 bit boundary */ + uint8 Spare[4]; /**< /brief Pad to avoid compiler padding if payload + requires 64 bit alignment */ } CFE_MSG_CommandHeader_t; diff --git a/modules/sb/fsw/src/cfe_sb_api.c b/modules/sb/fsw/src/cfe_sb_api.c index f991a2cd9..4cc541070 100644 --- a/modules/sb/fsw/src/cfe_sb_api.c +++ b/modules/sb/fsw/src/cfe_sb_api.c @@ -1244,11 +1244,11 @@ int32 CFE_SB_UnsubscribeFull(CFE_SB_MsgId_t MsgId, CFE_SB_PipeId_t PipeId, uint8 /* get routing id */ RouteId = CFE_SBR_GetRouteId(MsgId); - /* if there have never been subscriptions for this message id... */ + /* Status remains CFE_SUCCESS if route is valid or not */ if (!CFE_SBR_IsValidRouteId(RouteId)) { + /* If there are no subscriptions, simply report via event */ PendingEventID = CFE_SB_UNSUB_NO_SUBS_EID; - /* Status stays CFE_SUCCESS here */ } else { diff --git a/modules/sbr/CMakeLists.txt b/modules/sbr/CMakeLists.txt index a8a564a7a..e32ba5e03 100644 --- a/modules/sbr/CMakeLists.txt +++ b/modules/sbr/CMakeLists.txt @@ -30,7 +30,7 @@ elseif (MISSION_MSGMAP_IMPLEMENTATION STREQUAL "HASH") ${CMAKE_CURRENT_SOURCE_DIR}/fsw/src/cfe_sbr_map_hash.c ${CMAKE_CURRENT_SOURCE_DIR}/fsw/src/cfe_sbr_route_unsorted.c) else() - message(ERROR "Invalid software bush routing implementation selected:" MISSION_MSGMAP_IMPLEMENTATION) + message(ERROR "Invalid software bus routing implementation selected:" MISSION_MSGMAP_IMPLEMENTATION) endif() # Module library