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

CI LAB "PDUMsgID" checking depends on SB macro #1

Closed
skliper opened this issue Jun 24, 2019 · 3 comments · Fixed by #35
Closed

CI LAB "PDUMsgID" checking depends on SB macro #1

skliper opened this issue Jun 24, 2019 · 3 comments · Fixed by #35
Labels
enhancement New feature or request
Milestone

Comments

@skliper
Copy link
Contributor

skliper commented Jun 24, 2019

Originated by jphickey (22 on babelfish):

The CI LAB application attempts to verify that the PDUMsgId is within range by comparing it to the CFE_SB_HIGHEST_VALID_MSGID macro.
Unfortunately, this is a configuration macro that is specific to the SB implementation and may not be available in future builds. It makes assumptions about the way SB dispatches/handles messages that CI really should not be making.
This check also does not provide any real benefit - if the MsgID is out of range there is no major problem, it simply will not match any packets.
To improve compatibility with future SB improvements this extra check should be removed. Calling this a "defect" as it actually does break the build with the EDS branch.

@skliper skliper added the enhancement New feature or request label Jun 24, 2019
@skliper
Copy link
Contributor Author

skliper commented Jun 24, 2019

Last status - CCB 5/8/2019 - Quick discussion

  1. Reiterated option to just remove the check
  2. Proposed removing the associated logic (was used to test CF 4 or so years ago)
  3. Could use CFE_SB_IsValidMsgId() and simply send a warning event (notify user but still do what's been commanded)

Preference is 3.

@jphickey
Copy link
Contributor

jphickey commented Feb 7, 2020

Pull request #33 removes this problematic feature, which should resolve this issue.

@skliper
Copy link
Contributor Author

skliper commented Aug 21, 2020

Resolved by #35 (removed offending code).

@skliper skliper linked a pull request Aug 21, 2020 that will close this issue
@skliper skliper closed this as completed Aug 21, 2020
@astrogeco astrogeco added this to the 2.4.0 milestone Oct 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants