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

CFE_SB_SND_RTG_EID and CFE_SB_SND_RTG_ERR1_EID used for all three write file cmds (route, map, pipe) #1117

Open
skliper opened this issue Jan 22, 2021 · 2 comments · May be fixed by #2241
Open

Comments

@skliper
Copy link
Contributor

skliper commented Jan 22, 2021

Is your feature request related to a problem? Please describe.
Event ID's should be unique, these aren't.

Describe the solution you'd like
Define event ID's for all three. Also worth a general scrub of event ID's to ensure they are unique.

Other considerations:

  • "SND" is a misnomer, these are write commands
  • There is no event for the file header error

Describe alternatives you've considered
None

Additional context
None.

Requester Info
Jacob Hageman - NASA/GSFC

@thnkslprpt
Copy link
Contributor

Note: these were re-arranged (put into the background) in #1148
These events are now grouped together in CFE_SB_BackgroundFileEventHandler().
Making these EIDs unique would now require refactor of CFE_SB_BackgroundFileEventHandler() making it more complex and/or longer.

Perhaps just this PR should just be closed without action, or closed with a simple renaming of CFE_SB_SND_RTG_EID and CFE_SB_SND_RTG_ERR1_EID to make their names clearer.

@skliper
Copy link
Contributor Author

skliper commented Feb 3, 2023

@thnkslprpt - Agree w/ the simple rename. Single use in code is the guideline, and it's fine in a utility function to use the same EID from different callers. My preference is the ability to trace from an event to a single line in code.

thnkslprpt added a commit to thnkslprpt/cFE that referenced this issue Feb 4, 2023
@thnkslprpt thnkslprpt linked a pull request Feb 4, 2023 that will close this issue
2 tasks
thnkslprpt added a commit to thnkslprpt/cFE that referenced this issue May 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants