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

Reconsider LC/SC decoupling #100

Open
3 tasks done
skliper opened this issue Nov 7, 2023 · 1 comment
Open
3 tasks done

Reconsider LC/SC decoupling #100

skliper opened this issue Nov 7, 2023 · 1 comment

Comments

@skliper
Copy link
Contributor

skliper commented Nov 7, 2023

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.
Split off from #99 to better focus bug fix vs higher level design concepts.

There's local dfns of SC owned elements, which is a maintenance issue.

/**
* \brief RTS Request Message ID
*
* \par Description:
* Message ID that will be used by the #LC_ExecuteRTS function
* to construct the RTS request message.
*
* We define this here to allow the LC application to be built
* without including headers from the application (e.g. SC) that
* will receive the command. Obviously, this definition must
* match the message ID expectyed by the receiving application.
*
* A mission may choose to modify the #LC_ExecuteRTS function not
* to use this or define it using a message ID constant that is
* defined numerically elsewhere.
*
* \par Limits:
* This parameter shouldn't be larger than the value set for
* #CFE_PLATFORM_SB_HIGHEST_VALID_MSGID.
*/
#define LC_RTS_REQ_MID 0x18A9

/**
* \brief RTS Request Command Code
*
* \par Description:
* Command code that will be used by the #LC_ExecuteRTS function
* to construct the RTS request message.
*
* We define this here to allow the LC application to be built
* without including headers from the application (e.g. SC) that
* will receive the command. Obviously, this definition must
* match the command code expected by the receiving application.
*
* \par Limits:
* This parameter can't be larger than 127 (the maximum value of
* 7-bit number).
*/
#define LC_RTS_REQ_CC 4

/**
* \brief Payload to Start a Stored Command RTS
*/
typedef struct
{
uint16 RTSId; /**< \brief RTS Id to start */
} LC_RTSRequest_Payload_t;
/**
* \brief Send Command to Start a Stored Command RTS
*
* This is a local declaration of the command message structure
* to initiate an RTS and has been placed here to allow the
* the LC application to be built without including headers from
* any other applications (like Stored Commanding).
* A mission may choose to remove this and use a message
* structure declared elsewhere instead.
*
* This also applies to the LC_RTS_REQ_MID and LC_RTS_REQ_CC
* constants (see lc_mission_cfg.h).
*/
typedef struct
{
CFE_MSG_CommandHeader_t CommandHeader; /**< \brief Command Header */
LC_RTSRequest_Payload_t Payload;
} LC_RTSRequestCmd_t;

Describe the solution you'd like
LC depends on SC as designed, recommend single source of truth.

Describe alternatives you've considered
None

Additional context
Review functional testing to confirm this dependency is sufficiently tested.

Requester Info
Jacob Hageman - NASA/GSFC

@jphickey
Copy link
Contributor

jphickey commented Nov 7, 2023

Definitely agree, LC should not be redefining these structs, it should be using the definitions from SC if it needs them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants