Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Event ID name doesn't match use #262

Closed
3 tasks done
skliper opened this issue Jun 16, 2022 · 4 comments · Fixed by #335
Closed
3 tasks done

Event ID name doesn't match use #262

skliper opened this issue Jun 16, 2022 · 4 comments · Fixed by #335

Comments

@skliper
Copy link
Contributor

skliper commented Jun 16, 2022

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I reviewed the CF README.md file to see if the feature is in the major future work.
  • I performed a cursory search to see if the feature request is relevant, not redundant, nor in conflict with other tickets.

Is your feature request related to a problem? Please describe.
CF_EID_ERR_INIT_CMD_LENGTH is used for reporting invalid MID received, note CF_EID_ERR_CMD_GCMD_LEN and CF_EID_ERR_CMD_GCMD_CC are used for actual ground command processing:

CF/fsw/src/cf_app.c

Lines 321 to 322 in 593d61a

CFE_EVS_SendEvent(CF_EID_ERR_INIT_CMD_LENGTH, CFE_EVS_EventType_ERROR,
"CF: invalid command packet id=0x%lx", (unsigned long)CFE_SB_MsgIdToValue(msg_id));

Describe the solution you'd like
Event ID names should make sense, maybe CF_EID_ERR_MID or similar.

Describe alternatives you've considered
None

Additional context
None

Requester Info
Jacob Hageman - NASA/GSFC

@thnkslprpt
Copy link
Contributor

Maybe CF_MID_ERR_EID?
This seems like the most common format in other apps.

Having said that, there is a large variance in formats and order-of-components in names for similar events in other components/apps, so getting to a consistent format at some point in the future would make things easier.

At a glance, there is:
CF_EID_ERR_INIT_CMD_LENGTH
CFE_ES_MID_ERR_EID
CFE_EVS_ERR_MSGID_EID
CFE_SB_BAD_MSGID_EID
CFE_TBL_MID_ERR_EID
CFE_TIME_ID_ERR_EID
CS_MID_ERR_EID
HS_MID_ERR_EID
MM_MID_ERR_EID
SC_MID_ERR_EID
SCH_MD_ERR_EID

@thnkslprpt
Copy link
Contributor

ping @havencarlson @skliper

@skliper
Copy link
Contributor Author

skliper commented Oct 17, 2022

ping @dmknutsen @dzbaker - they are the deciderers now :)

@dmknutsen
Copy link
Contributor

Seems very reasonable to go with the most common format - CF_MID_ERR_EID looks good to me...

thnkslprpt added a commit to thnkslprpt/CF that referenced this issue Oct 18, 2022
thnkslprpt added a commit to thnkslprpt/CF that referenced this issue Oct 18, 2022
thnkslprpt added a commit to thnkslprpt/CF that referenced this issue Mar 12, 2023
thnkslprpt added a commit to thnkslprpt/CF that referenced this issue Mar 14, 2023
thnkslprpt added a commit to thnkslprpt/CF that referenced this issue May 19, 2023
thnkslprpt added a commit to thnkslprpt/CF that referenced this issue May 19, 2023
thnkslprpt added a commit to thnkslprpt/CF that referenced this issue Jul 15, 2023
thnkslprpt added a commit to thnkslprpt/CF that referenced this issue Dec 14, 2023
thnkslprpt added a commit to thnkslprpt/CF that referenced this issue Jan 14, 2024
thnkslprpt added a commit to thnkslprpt/CF that referenced this issue Jan 14, 2024
dzbaker added a commit that referenced this issue Feb 22, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants