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 #4, Add PacketNotFoundCtr to HK AppData/Tlm #54

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

thnkslprpt
Copy link
Contributor

Checklist

Describe the contribution

  • Fixes HK Count track times packet is not found #4
    • Adds a new struct member (PacketNotFoundCtr) to HK_HkTlm_Payload_t and HK_AppData_t that tracks the number of times a packet was not found during traversal of the copy table in HK_SendCombinedHkPacket().
    • Split the MissingDataCtr in half to create this counter, but could also just utilize the padding that is already there for this new member, and keep both sized at uint16 if needed

Testing performed
GitHub CI actions all passing successfully.

Expected behavior changes
Additional information tracking number of times packet not found available in tlm packets now.

Contributor Info
Avi Weiss @thnkslprpt

@thnkslprpt
Copy link
Contributor Author

Two questions:

  1. Should this actually be an ERROR event type, rather than INFORMATIONal? - the EID comments indicate the former.
  2. Leave as submitted, or use the padding to create PacketNotFoundCtr instead of splitting MissingDataCtr in half?

@skliper
Copy link
Contributor

skliper commented Apr 26, 2023

This is a thinker. TBH this doesn't seem to fall in the "run time" sort of error bucket, more a misconfiguration if something is requesting a combined HK MID that doesn't exist in the table. If it was misconfigured (either the table's got the wrong MID or the request has the wrong MID), I'd expect it to be on some sort of schedule, so would probably just flood this event and I don't see big value in tracking how many times it occurred. I probably added this issue to track code review comments without really digging into it. Since it's not the command MID you wouldn't expect it to come from the ground either way.

I recommend ERRORifying the event for sure. I'm not convinced the counter is added value.

If others want it I recommend using the old spare element to avoid breaking tlm databases. If they don't keep up they'll just see a spare increment vs getting nonsensical values for the two counters (but only in an error case, so splitting could introduce a sleeper bug).

@thnkslprpt
Copy link
Contributor Author

Cool no worries - if not enough added value to justify adding a counter can just close as won't-fix.

Either way the event should be clarified as either INFO or ERROR:

HK/fsw/inc/hk_events.h

Lines 157 to 164 in 83b7d08

* \par Type: ERROR
*
* \par Cause:
*
* This event message is issued when the HK application receives a command to
* send a combined output message that is not listed in the copy table.
*/
#define HK_UNKNOWN_COMBINED_PACKET_EID 11

CFE_EVS_SendEvent(HK_UNKNOWN_COMBINED_PACKET_EID, CFE_EVS_EventType_INFORMATION,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

HK Count track times packet is not found
2 participants