Add prototype Event Management calls and Event Proxy impl files #1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
warning: fails to compile with the lib
and add some other TODOs while I was going over code with Dylan
The default array size was 9, when the max (set from CFE_PLATFORM_EVS_MAX_EVENT_FILTERS) is 8. These two EIDs weren't being referenced anywhere anyway, so I just removed them from the list.
Now that they aren't listed in the Event IDs filter, there's no refs to them.
keegan-moore
commented
Mar 20, 2024
Comment on lines
+129
to
+131
| /* TODO: We'll probably want to remove this, or wrap it behind an "if debug" compiler flag. */ | ||
| OS_printf("BPNODE_EVP_SendEvent_Impl(%u, %s, %s)\n", | ||
| EventID, BPL_EVM_EventTypeToString(EventType), ExpandedEventText); |
There was a problem hiding this comment.
Should I simply remove this, or are there debug print flags in BP that we should use?
keegan-moore
commented
Mar 20, 2024
Comment on lines
+109
to
+127
| /* | ||
| ** Due to how we truncate the message if its too long (as seen in code, below), | ||
| ** we need to ensure that this buffer is at least 2 characters long. | ||
| */ | ||
| assert(BPNODE_EVP_MAX_MESSAGE_LENGTH >= 2); | ||
| assert(BPNODE_EVP_MAX_MESSAGE_LENGTH <= CFE_MISSION_EVS_MAX_MESSAGE_LENGTH); | ||
|
|
||
| memset(&ExpandedEventText, 0, sizeof(ExpandedEventText)); | ||
| ExpandedLength = vsnprintf((char *)ExpandedEventText, sizeof(ExpandedEventText), | ||
| EventText, EventTextArgPtr); | ||
| if (ExpandedLength >= (int)sizeof(ExpandedEventText)) | ||
| { | ||
| /* Mark character before zero terminator to indicate truncation */ | ||
| ExpandedEventText[sizeof(ExpandedEventText) - 2u] = BPNODE_EVP_MSG_TRUNCATED; | ||
| /* | ||
| ** TODO: should we return an error here? | ||
| ** Note: In the cFE implementation, they don't treat message truncation as an error. | ||
| */ | ||
| } |
There was a problem hiding this comment.
This error handling response should be reviewed by the group.
keegan-moore
commented
Mar 20, 2024
Comment on lines
+137
to
+138
| OS_printf("BPNODE_EVP_SendEvent_Impl CFE_EVS_SendEvent returned error status: 0x%08X!\n", | ||
| ProxyStatus); |
There was a problem hiding this comment.
Here and other places, I've put OS_printf debug print calls in to help catch errors. Is there a better strategy for this?
keegan-moore
commented
Mar 20, 2024
Comment on lines
167
to
169
| /* Application startup event message */ | ||
| CFE_EVS_SendEvent(BP_INIT_INF_EID, CFE_EVS_EventType_INFORMATION, "BP App Version %d.%d.%d.%d: Initialized", | ||
| (void) BPL_EVM_SendEvent(BP_INIT_INF_EID, CFE_EVS_EventType_INFORMATION, "BP App Version %d.%d.%d.%d: Initialized", | ||
| BP_MAJOR_VERSION, BP_MINOR_VERSION, BP_REVISION, BP_MISSION_REV); |
There was a problem hiding this comment.
The previous call to CFE_EVS_SendEvent ignored its CFE_Status_t return status, so I kept that going here.
However, if we wanted to check the return status, it would look something like this:
/* Application startup event message */
BPL_Status_t BPL_EVM_SendEventStatus;
(void) BPL_EVM_SendEvent(BP_INIT_INF_EID, CFE_EVS_EventType_INFORMATION,
"BP App Version %d.%d.%d.%d: Initialized",
BP_MAJOR_VERSION, BP_MINOR_VERSION, BP_REVISION, BP_MISSION_REV);
if (BPL_EVM_SendEventStatus.ReturnValue != BPL_STATUS_SUCCESS)
{
fprintf(stderr, "%s(): First BPL_EVM_SendEvent call failed\n", __func__);
return BPL_EVM_SendEventStatus.ReturnValue;
}I find this to be a little too distracting for such a benign activity, but I'm happy to go with the group's decision on whether to continue ignoring the status return from Send Event calls.
keegan-moore
changed the title
|
Before this change set is complete, we'd need to scrape through BP and look for all calls to |
This reverts commit d6b3f0a.
This was referenced Apr 5, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Describe the contribution
This PR add prototype implementations for the cFS Event Proxy design.
This design intends to abstract the cFS EVS API calls through the Event Management core in BPLib.
A related PR exists for BPLib, here: keegan-moore/bplib#1 .
This design change causes the following in the BP app:
BPL_EVM_Initializeinstead ofCFE_EVS_RegisterBPL_EVM_Initializetakes a struct of function pointers to the cFS Event Proxy implementationBPL_EVM_SendEventTesting performed
Steps taken to test the contribution:
make native.distclean2.
make ENABLE_TESTS=false native.installcd build-native-9.4.0/exe/cpu1./core-cpu1Expected behavior changes
EVS's
CFE_EVS_Registershould no longer generate the following warning in the ES System Log, since the event filter array is now appropriately sized:A clear and concise description of how this contribution will change behavior and level of impact.
BPL_EVM_Initializeinstead ofCFE_EVS_RegisterBPL_EVM_SendEventCFE_EVS_Registershould no longer generate a warning in the ES System Log during BP app initSystem(s) tested on
Additional context
N/A
Third party code
N/A
Contributor Info - All information REQUIRED for consideration of pull request
Keegan Moore, NASA/GSFC Code 582 (Flight Software Systems)