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

Fix #1320 #1583 #508, Event ID updates #1594

Merged
merged 2 commits into from
Jun 23, 2021

Conversation

skliper
Copy link
Contributor

@skliper skliper commented Jun 1, 2021

Describe the contribution
Fix #1320 - Removed CFE_ES_MAX_EID and all the others (very fragile, especially when EIDs are out of order)
Fix #1583 - Ordered EID's and fixed duplicate assignments
Fix #508 - Cleaned up EventID documentation

  • Removed format string and replaced with brief description
  • Reduce verbiage in full descriptions, made consistent and less "bi-directional" when there are multiple sources. API/commands will reference EIDs.

Testing performed
Build an run unit tests, passed - TBD (not actually done yet)

Expected behavior changes
Deconflicted EIDs.

System(s) tested on

  • Hardware: Intel I5/Docker
  • OS: Ubuntu 18.04
  • Versions: Bundle main + this commit

Additional context
Did not address #1532, could be a big impact.
Wrote #1588 to track use of EID in multiple locations.

Third party code
None

Contributor Info - All information REQUIRED for consideration of pull request
Jacob Hageman - NASA/GSFC

@skliper skliper added the CCB:Ready Ready for discussion at the Configuration Control Board (CCB) label Jun 1, 2021
@skliper skliper added this to the 7.0.0 milestone Jun 1, 2021
@skliper
Copy link
Contributor Author

skliper commented Jun 1, 2021

Created draft and marked to review current approach.

@astrogeco
Copy link
Contributor

astrogeco commented Jun 2, 2021

CCB:2021-06-02 APPROVED

  • Did this renumber Event IDs? Only 3 cases were changed.

@astrogeco astrogeco added CCB:Approved Indicates code review and approval by community CCB and removed CCB:Ready Ready for discussion at the Configuration Control Board (CCB) labels Jun 2, 2021
@skliper
Copy link
Contributor Author

skliper commented Jun 2, 2021

Note a future improvement could be to change how commands are documented, use grouping instead of xrefitem (cross reference) to create the group and use a \brief to describe the command. Then everywhere the command brief is useful could just \copybrief #WHATEVER_CC, would eliminate at least two copies of the brief command description for every event ID. The current references/links help to prevent confusion, but the less opportunities for typos in repeated text the better.

@skliper skliper marked this pull request as ready for review June 22, 2021 14:15
@skliper skliper added the CCB:Ready Ready for discussion at the Configuration Control Board (CCB) label Jun 22, 2021
@astrogeco
Copy link
Contributor

Can I remove the WIP prefix?

@skliper skliper changed the title WIP #1320 #1583 #508, Event ID updates Fix #1320 #1583 #508, Event ID updates Jun 22, 2021
@skliper
Copy link
Contributor Author

skliper commented Jun 22, 2021

Can I remove the WIP prefix?

Done!

@astrogeco astrogeco changed the base branch from main to integration-candidate June 23, 2021 13:13
@astrogeco astrogeco merged commit 63a5a31 into nasa:integration-candidate Jun 23, 2021
astrogeco added a commit to nasa/cFS that referenced this pull request Jun 23, 2021
nasa/cFE#1633, add test log file
nasa/cFE#1594, Fix #1320 #1583 #508, Event ID updates
@astrogeco
Copy link
Contributor

CCB:2021-06-23 APPROVED

@astrogeco astrogeco removed the CCB:Ready Ready for discussion at the Configuration Control Board (CCB) label Jun 23, 2021
astrogeco added a commit to nasa/cFS that referenced this pull request Jun 24, 2021
*Combines*

- nasa/elf2cfetbl#81
- nasa/tblCRCTool#52
- nasa/ci_lab#88
- nasa/sch_lab#83
- nasa/sample_app#150
- nasa/sample_lib#64
- nasa/to_lab#100

*Includes*

- nasa/cFE#1630, correct path to users guide warning log
- nasa/cFE#1621, add additional test cases for Child Tasks
- nasa/cFE#1608, Add cfe functional tests to CI
- nasa/cFE#1627, rename/clean CFE coverage assert macros
- nasa/cFE#1623, Added UT tests for cFE ES Api
- nasa/cFE#1634, Expand CDS Functional Tests.
- nasa/cFE#1633, add test log file
- nasa/cFE#1594, Event ID updates
- nasa/cFE#1624, scrub all UT_Report calls

- nasa/osal#1066, implement missing parameter/retcode test permutations

- nasa/cFS-GroundSystem#182, Add test start command script for cmdUtil
- nasa/tblCRCTool#51, add printf conversion casts

** Implement Coding Standard in CodeQL **

- nasa/cFS-GroundSystem#180
- nasa/elf2cfetbl#80
- nasa/tblCRCTool#49

- nasa/ci_lab#87
- nasa/sch_lab#79
- nasa/sample_app#149
- nasa/sample_lib#63
- nasa/to_lab#99
astrogeco added a commit to nasa/cFS that referenced this pull request Jun 24, 2021
*Combines*

- nasa/cFE#1632, v6.8.0-rc1+dev726
- nasa/osal#1079, v5.1.0-rc1+dev548

- nasa/ci_lab#88, v2.4.0-rc1+dev42
- nasa/sch_lab#83, v2.4.0-rc1+dev40
- nasa/sample_app#150, v1.2.0-rc1+dev66
- nasa/sample_lib#64, v1.2.0-rc1+dev38
- nasa/to_lab#100, v2.4.0-rc1+dev49

- nasa/elf2cfetbl#81, v3.2.0-rc1+dev30
- nasa/tblCRCTool#52, v1.2.0-rc1+dev33
- nasa/cFS-GroundSystem#183, v2.2.0-rc1+dev52

*Includes*

- nasa/cFE#1630, correct path to users guide warning log
- nasa/cFE#1621, add additional test cases for Child Tasks
- nasa/cFE#1608, Add cfe functional tests to CI
- nasa/cFE#1627, rename/clean CFE coverage assert macros
- nasa/cFE#1623, Added UT tests for cFE ES Api
- nasa/cFE#1634, Expand CDS Functional Tests.
- nasa/cFE#1633, add test log file
- nasa/cFE#1594, Event ID updates
- nasa/cFE#1624, scrub all UT_Report calls

- nasa/osal#1066, implement missing parameter/retcode test permutations

- nasa/cFS-GroundSystem#182, Add test start command script for cmdUtil
- nasa/tblCRCTool#51, add printf conversion casts

** Implement Coding Standard in CodeQL **

- nasa/cFS-GroundSystem#180
- nasa/elf2cfetbl#80
- nasa/tblCRCTool#49

- nasa/ci_lab#87
- nasa/sch_lab#79
- nasa/sample_app#149
- nasa/sample_lib#63
- nasa/to_lab#99

Co-authored-by: Jacob Hageman <skliper@users.noreply.github.com>
Co-authored-by: Joseph Hickey <jphickey@users.noreply.github.com>
Co-authored-by: Ariel Adams <ArielSAdamsNASA@users.noreply.github.com>
Co-authored-by: Alex Campbell <zanzaben@users.noreply.github.com>
Co-authored-by: Jose F Martinez Pedraza <pepepr08@users.noreply.github.com>
@skliper skliper deleted the fix1320-es_max_eid branch October 22, 2021 19:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CCB:Approved Indicates code review and approval by community CCB
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Order event ID's and fix duplicates CFE_ES_MAX_EID incorrect Doxygen event documentation needs scrub
2 participants