-
Notifications
You must be signed in to change notification settings - Fork 204
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
Update API/error code documentation relative to OSReturnCode cases #1599
Comments
Whenever there is a "domain transition" like this in error handling, a preferable approach would be for the function that gets the OSAL error to handle it to the best of its ability. If it needs to fail the overall operation, then report it (via syslog or event), along with other relevant detail of what went wrong. It should then return a CFE-defined error code up to the caller. In this pattern the error is "handled" by reporting all the details and aborting the request. As such, once an error has been logically "handled" - either by reporting or otherwise - it is no longer pending, and shouldn't be passed up the stack to the caller anymore, as the issue has already been handled. It should only pass back a code that says the requested operation was not performed, that's all the caller should need to about (it should treat the function as a black box, it doesn't know/care if OSAL functions are used underneath most of the time). |
In short, I'm actually advocating for the "alternative" in the original summary, to convert all return codes to the CFE set.
In my experience, domain-specific errors which are passed too far back up the call stack are actually more obscure, as it often results either in the same error getting reported multiple times, OR getting reported at a point where the details of what actually went wrong no longer exist. Again, in a proper API abstraction, an application caller shouldn't know or care if some API calls an OSAL function as part of its work, and shouldn't be expected to handle an OSAL error if it were to happen. While there are always exceptions and cases where it might make sense and it can be documented that the function calls OSAL and might pass back and OSAL return code that the application should handle, most of the time it doesn't. |
Sounds good to me as a more thorough approach to improving the current implementation. Might still be worth documenting the current behavior since a full scrub is probably further off (although Draco build plans haven't solidified yet to my knowledge.) Maybe resolving this issue as a first step (document/flag all the APIs where OSAL/OS error codes fall through) with a follow on to fix them would be a manageable approach. |
CFE_ES_GetTaskName() is an example where converting from OSAL goes wrong. It returns CFE_ES_ERR_RESOURCEID_NOT_VALID even when the osal error is OS_ERR_NAME_TOO_LONG cFE/modules/es/fsw/src/cfe_es_api.c Lines 988 to 993 in a16c78e
|
Should provide a more useful CFE return code for parameter errors vs ID errors. |
After reviewing all the areas where OSAL and CFE status codes are mixed, most are OK in that they do not actually return the OSAL status code to the caller - it is rewritten to a CFE status code most of the time. Here is a complete list of functions where the OSAL status is returned to the caller, without converting it to a CFE code, which I found by reviewing through the existing code: CFE_ES_BackgroundInit Of these, most are internal functions (i.e. the "Init" stuff, command processors, etc). The only ones that are actually part of the public API are the FS ones. |
Some FS API calls will pass through failure/status codes directly from OSAL without remapping to CFE Status code values. Note this behavior in the documentation and that it will likely change in a future version of CFE.
Fix #1599, documentation for FS APIs that return OSAL codes
#1599 (comment) |
Some FS API calls will pass through failure/status codes directly from OSAL without remapping to CFE Status code values. Note this behavior in the documentation and that it will likely change in a future version of CFE.
Is your feature request related to a problem? Please describe.
There are multiple cFE API's that can return OSReturnCodes (CFE_FS_ReadHeader, CFE_FS_WriteHeader, etc). This isn't explicitly documented in the API or as part of CFEReturnCodes.
Describe the solution you'd like
Add documentation. They don't conflict due to the severity bits/service bits.
Describe alternatives you've considered
Convert all return codes to the CFE set, but probably not worth it and could obscure source of error.
Additional context
Spawned from requested change in #1598
Requester Info
Jacob Hageman - NASA/GSFC
The text was updated successfully, but these errors were encountered: