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.

Sign up for GitHub

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

Jump to bottom

ES CDS defines two nearly identically-named constants with different values #370

Closed
jphickey opened this issue Oct 8, 2019 · 1 comment · Fixed by #964 or #991
Closed

ES CDS defines two nearly identically-named constants with different values #370

jphickey opened this issue Oct 8, 2019 · 1 comment · Fixed by #964 or #991
Labels
enhancement
Milestone
Caelum

Comments

@jphickey
Copy link
Contributor

jphickey commented Oct 8, 2019

Describe the bug
The CDS code uses two mission-scope configuration values:
CFE_MISSION_ES_CDS_MAX_NAME_LENGTH
CFE_MISSION_ES_CDS_MAX_NAME_LEN

The values are actually different; the "LENGTH" version is just the CDS base name, where as the "LEN" is the size of the qualified name (APP.CDSName form).

To Reproduce
No runtime issue, this is a style/maintenance issue.

Expected behavior
The names should be more different and distinctive.

Suggest that CFE_MISSION_ES_CDS_MAX_NAME_LENGTH should be renamed to indicate that it reflects the size of the base name only. This value appears to primarily be used internally to CDS, and does not get directly used in telemetry packets, so it would be easier to rename, with less impact on existing code.

Reporter Info
Joseph Hickey, Vantage Systems, Inc.
@jphickey
Copy link
Contributor Author

Noticed this again when fixing #909 -- very confusing -- we should really fix this.

jphickey added a commit to jphickey/cFE that referenced this issue Oct 27, 2020
@jphickey
Fix nasa#370, rename MAX CDS name macro to FULL_NAME_LEN
f9f18a7 
Simple replacement of CFE_MISSION_ES_CDS_MAX_NAME_LEN with
CFE_MISSION_ES_CDS_MAX_FULL_NAME_LEN, to differentiate it
from CFE_MISSION_ES_CDS_MAX_NAME_LENGTH - which is the CDS
name without the app name.
@jphickey jphickey linked a pull request Oct 27, 2020 that will close this issue
Fix #909, reorganize ES public API + msg definitions #964
Merged
@jphickey jphickey mentioned this issue Oct 27, 2020
Merged
@astrogeco astrogeco mentioned this issue Nov 2, 2020
Merged
@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
Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
Caelum
2 participants
@jphickey @skliper