Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create consistent SB transmit/receive API's, refactored to utilize the zero copy pattern #1019

Closed
3 of 5 tasks
skliper opened this issue Nov 16, 2020 · 3 comments · Fixed by #1015
Closed
3 of 5 tasks
Milestone

Comments

@skliper
Copy link
Contributor

skliper commented Nov 16, 2020

Is your feature request related to a problem? Please describe.
Duplicated logic in CFE_SB_SendMsg and CFE_SB_ZeroCopySend (and related APIs), refactor could simplify CFE_SB_SendMsgFull. Sending/receiving software bus buffers vs the less restrictive alignment message type is not clear, names don't mirror.

Describe the solution you'd like
Implement the following:

  • CFE_SB_TransmitMsg - sends the less restrictive alignment message type by coping it into a SB buffer, then transmitting buffer
  • CFE_SB_TransmitBuffer - sends a message that is already in a software bus buffer (like CFE_SB_ZeroCopySend)
  • CFE_SB_ReceiveBuffer - the old CFE_SB_RcvMsg (it's not a message, it's a buffer)
  • CFE_SB_AllocateBuffer - the old CFE_SB_ZeroCopyGetPtr
  • CFE_SB_ReleaseBuffer - the old CFE_SB_ZeroCopyReleasePtr

Possibly add flag for incrementing sequence count (instead of more API's like CFE_SB_PassMsg).

Describe alternatives you've considered
None

Additional context
Came from #1009 discussions

Requester Info
Jacob Hageman - NASA/GSFC

@jphickey
Copy link
Contributor

I like the idea of getting rid of the "Pass" variants - or at least justify why we would need to send messages that don't increment the sequence count. This seems like an odd thing to do - because the whole point of a sequence count is for a receiver to know if they missed one. If we duplicate sequence numbers then it would defeat the purpose.

@skliper
Copy link
Contributor Author

skliper commented Nov 16, 2020

SBN needs it...

skliper added a commit to skliper/cFE that referenced this issue Nov 30, 2020
- Added CFE_SB_TransmitMsg, CFE_SB_TransmitBuffer,
  CFE_SB_ReceiveBuffer (partial nasa#1019 fix)
- Replace CFE_SB_RcvMsg with CFE_SB_ReceiveBuffer
- Deprecated CFE_SB_SendMsg, CFE_SB_PassMsg, CFE_SB_RcvMsg
  CFE_SB_ZeroCopyPass, CFE_SB_ZeroCopySend
- Use CFE_SB_Buffer_t for receiving and casting to command types
- Use CFE_MSG_CommandHeader_t and CFE_MSG_TelemetryHeader_t in
  command and telemetry type definitions
- Use CFE_SB_TransmitMsg to copy the command and telemetry
  into a CFE_SB_Buffer_t and send it where needed
- Avoids need to create send buffers within the app (or union
  the packet types with CFE_SB_Buffer_t)
- Eliminates references to CFE_SB_CmdHdr_t and CFE_SB_TlmHdr_t
  that formerly enforced alignment since these had potential
  to change the actual packet sizes
- No need to cast to CFE_MSG_Message_t anywhere since it's
  available in the CFE_SB_Buffer_t union
- CFE_MSG_Size_t redefined as size_t to simplify future
  transition
- Replaced Syslog with SysLog for consistency
- Added Cmd to all command typedefs
- Replaced CFE_SB_CMD_HDR_SIZE and CFE_SB_TLM_HDR_SIZE
  with sizeof the appropriate type
skliper added a commit to skliper/cFE that referenced this issue Nov 30, 2020
- Added CFE_SB_TransmitMsg, CFE_SB_TransmitBuffer,
  CFE_SB_ReceiveBuffer (partial nasa#1019 fix)
- Replace CFE_SB_RcvMsg with CFE_SB_ReceiveBuffer
- Deprecated CFE_SB_SendMsg, CFE_SB_PassMsg, CFE_SB_RcvMsg
  CFE_SB_ZeroCopyPass, CFE_SB_ZeroCopySend
- Use CFE_SB_Buffer_t for receiving and casting to command types
- Use CFE_MSG_CommandHeader_t and CFE_MSG_TelemetryHeader_t in
  command and telemetry type definitions
- Use CFE_SB_TransmitMsg to copy the command and telemetry
  into a CFE_SB_Buffer_t and send it where needed
- Avoids need to create send buffers within the app (or union
  the packet types with CFE_SB_Buffer_t)
- Eliminates references to CFE_SB_CmdHdr_t and CFE_SB_TlmHdr_t
  that formerly enforced alignment since these had potential
  to change the actual packet sizes
- No need to cast to CFE_MSG_Message_t anywhere since it's
  available in the CFE_SB_Buffer_t union
- CFE_MSG_Size_t redefined as size_t to simplify future
  transition
- Replaced Syslog with SysLog for consistency
- Added Cmd to all command typedefs
- Replaced CFE_SB_CMD_HDR_SIZE and CFE_SB_TLM_HDR_SIZE
  with sizeof the appropriate type
skliper added a commit to skliper/cFE that referenced this issue Nov 30, 2020
- Added CFE_SB_TransmitMsg, CFE_SB_TransmitBuffer,
  CFE_SB_ReceiveBuffer (partial nasa#1019 fix)
- Replace CFE_SB_RcvMsg with CFE_SB_ReceiveBuffer
- Deprecated CFE_SB_SendMsg, CFE_SB_PassMsg, CFE_SB_RcvMsg
  CFE_SB_ZeroCopyPass, CFE_SB_ZeroCopySend
- Use CFE_SB_Buffer_t for receiving and casting to command types
- Use CFE_MSG_CommandHeader_t and CFE_MSG_TelemetryHeader_t in
  command and telemetry type definitions
- Use CFE_SB_TransmitMsg to copy the command and telemetry
  into a CFE_SB_Buffer_t and send it where needed
- Avoids need to create send buffers within the app (or union
  the packet types with CFE_SB_Buffer_t)
- Eliminates references to CFE_SB_CmdHdr_t and CFE_SB_TlmHdr_t
  that formerly enforced alignment since these had potential
  to change the actual packet sizes
- No need to cast to CFE_MSG_Message_t anywhere since it's
  available in the CFE_SB_Buffer_t union
- CFE_MSG_Size_t redefined as size_t to simplify future
  transition
- Replaced Syslog with SysLog for consistency
- Added Cmd to all command typedefs
- Replaced CFE_SB_CMD_HDR_SIZE and CFE_SB_TLM_HDR_SIZE
  with sizeof the appropriate type
skliper added a commit to skliper/cFE that referenced this issue Dec 2, 2020
- Added CFE_SB_TransmitMsg, CFE_SB_TransmitBuffer,
  CFE_SB_ReceiveBuffer (partial nasa#1019 fix)
- Replace CFE_SB_RcvMsg with CFE_SB_ReceiveBuffer
- Deprecated CFE_SB_SendMsg, CFE_SB_PassMsg, CFE_SB_RcvMsg
  CFE_SB_ZeroCopyPass, CFE_SB_ZeroCopySend
- Use CFE_SB_Buffer_t for receiving and casting to command types
- Use CFE_MSG_CommandHeader_t and CFE_MSG_TelemetryHeader_t in
  command and telemetry type definitions
- Use CFE_SB_TransmitMsg to copy the command and telemetry
  into a CFE_SB_Buffer_t and send it where needed
- Avoids need to create send buffers within the app (or union
  the packet types with CFE_SB_Buffer_t)
- Eliminates references to CFE_SB_CmdHdr_t and CFE_SB_TlmHdr_t
  that formerly enforced alignment since these had potential
  to change the actual packet sizes
- No need to cast to CFE_MSG_Message_t anywhere since it's
  available in the CFE_SB_Buffer_t union
- CFE_MSG_Size_t redefined as size_t to simplify future
  transition
- Replaced Syslog with SysLog for consistency
- Added Cmd to all command typedefs
- Replaced CFE_SB_CMD_HDR_SIZE and CFE_SB_TLM_HDR_SIZE
  with sizeof the appropriate type
skliper added a commit to skliper/cFE that referenced this issue Dec 8, 2020
- Added CFE_SB_TransmitMsg, CFE_SB_TransmitBuffer,
  CFE_SB_ReceiveBuffer (partial nasa#1019 fix)
- Replace CFE_SB_RcvMsg with CFE_SB_ReceiveBuffer
- Deprecated CFE_SB_SendMsg, CFE_SB_PassMsg, CFE_SB_RcvMsg
  CFE_SB_ZeroCopyPass, CFE_SB_ZeroCopySend
- Use CFE_SB_Buffer_t for receiving and casting to command types
- Use CFE_MSG_CommandHeader_t and CFE_MSG_TelemetryHeader_t in
  command and telemetry type definitions
- Use CFE_SB_TransmitMsg to copy the command and telemetry
  into a CFE_SB_Buffer_t and send it where needed
- Avoids need to create send buffers within the app (or union
  the packet types with CFE_SB_Buffer_t)
- Eliminates references to CFE_SB_CmdHdr_t and CFE_SB_TlmHdr_t
  that formerly enforced alignment since these had potential
  to change the actual packet sizes
- No need to cast to CFE_MSG_Message_t anywhere since it's
  available in the CFE_SB_Buffer_t union
- CFE_MSG_Size_t redefined as size_t to simplify future
  transition
- Replaced Syslog with SysLog for consistency
- Added Cmd to all command typedefs
- Replaced CFE_SB_CMD_HDR_SIZE and CFE_SB_TLM_HDR_SIZE
  with sizeof the appropriate type
@skliper
Copy link
Contributor Author

skliper commented Jan 4, 2021

Fixed in #1015, just didn't rename CFE_SB_ZeroCopyGetPtr and CFE_SB_ZeroCopyReleasePtr. Open a new issue if renaming these is required.

@skliper skliper closed this as completed Jan 4, 2021
@skliper skliper linked a pull request Jan 4, 2021 that will close this issue
@skliper skliper added this to the 7.0.0 milestone Sep 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants