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

Broken RTS message, out of sync with SC #99

Closed
2 tasks done
skliper opened this issue Nov 7, 2023 · 1 comment · Fixed by #101
Closed
2 tasks done

Broken RTS message, out of sync with SC #99

skliper opened this issue Nov 7, 2023 · 1 comment · Fixed by #101
Assignees
Labels

Comments

@skliper
Copy link
Contributor

skliper commented Nov 7, 2023

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
Critically broken. Local defn of start RTS command is out of sync with SC:

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).
*/

https://github.com/nasa/SC/blob/ab3a9a03aee8fb9826077c0492f4b7698132c057/config/default_sc_msgdefs.h#L251-L258

Really the concept of local defines for SC elements doesn't seem like a good idea. LC depends on SC, the message structure, CC, and MID should come from SC.

To Reproduce
Trigger an RTS from LC. Event received:
66/1/SC 2: Invalid msg length: ID = 0x000018A9, CC = 4, Len = 10, Expected = 12

Expected behavior
Command should work

Code snips
See above

System observed on:
On an Ubuntu 22 docker, issue is in main branches.

Additional context
Might be missing a functional test or need an update if this is currently passing.

Reporter Info
Jacob Hageman - NASA/GSFC

@skliper skliper added the bug label Nov 7, 2023
@skliper
Copy link
Contributor Author

skliper commented Nov 7, 2023

@skliper skliper self-assigned this Nov 7, 2023
dzbaker added a commit that referenced this issue Nov 13, 2023
Fix #99, Add padding in LC_RTSRequest_Payload_t to match SC
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant