Fix #846, Deconflict CFE_ES_LIB_ALREADY_LOADED and CFE_ES_ERR_SYS_LOG_TRUNCATED

[skliper]

Describe the contribution
Fix #846 - deconflict CFE_ES_LIB_ALREADY_LOADED and CFE_ES_ERR_SYS_LOG_TRUNCATED EIDs

Testing performed
Built and ran unit tests (checks both those returns), passed.

Expected behavior changes
EIDs no longer overloaded.

System(s) tested on

• Hardware: cFS Dev Server
• OS: Ubuntu 18.04
• Versions: bundle main + this commit

Additional context
None

Third party code
None

Contributor Info - All information REQUIRED for consideration of pull request
Jacob Hageman - NASA/GSFC

[skliper]

Just realized these aren't EIDs... error codes. I'll fix commit.

[jphickey]

See also #616, seems to be a duplicate

[keegan-moore]

Should the value start with 0xC, to indicate that this return value is an error?

[skliper]

Should the value start with 0xC, to indicate that this return value is an error?

I left them with the "Informational" severity code, this change just deconflicts. I'd recommend severity code changes be a new issue if you want to re-evaluate them.

[keegan-moore]

We might want to open a new issue to discuss changing this value to be an error type (start with 0xC instead of 0x4).

[skliper]

We might want to open a new issue to discuss changing this value to be an error type (start with 0xC instead of 0x4).

See also #483 for the general ticket.

[jphickey]

My take is that the log getting truncated isn't really an error - its informational - because the message WAS logged. An error code should indicate no action was performed. That's so the caller has a recourse - they can check for the error, and do some sort of mitigation, possibly re-try the call.

In this situation - its not like anyone can take useful action on this response - code is not going to check for this condition and log a shorter message or something. (which would end up as a duplicate too).

So eventually (and in line with #483) we should get rid of CFE_ES_ERR_SYS_LOG_TRUNCATED entirely. Just return CFE_SUCCESS. If anything, ES can internally track the number of times this happens so the user knows they need to increase CFE_PLATFORM_ES_SYSTEM_LOG_SIZE ... but returning it to apps as a status code makes no sense. What is a generic app going to do with that info?

[jphickey]

FWIW - in the framework I see 278 references to CFE_ES_WriteToSysLog() and with the exception of unit test cases - none of them actually check the returned status at all or do any sort of recovery if the syslog doesn't work or is truncated or anything.

(Which makes sense when you think about it, because we wouldn't expect a CFS app to handle system-level issues like a broken syslog - it isn't its domain).

[keegan-moore]

I looked at this from a different perspective. It seems like CFE_ES_ERR_SYS_LOG_TRUNCATED was created to tell the calling app that the app was using the syslog wrong, and the function call didn't work as expected. I wouldn't consider this to mean that syslog was broken, I'd consider that to be an app error, and I'd expect CFE_ES_WriteToSysLog() to return some type of non-CFE_SUCCESS code if an app used the API incorrectly. It would be great if 0x2 was used to indicate a warning, or something like that.
Even though apps don't typically look at that return value, if it's going to return a value, I don't think it should return CFE_SUCCESS in that case.
Thinking out loud: Knowing that most apps don't check the return, I'd say it would be easier for ES to report that error somehow. I'm not sure what the best mechanism for that would be, though.

[jphickey]

I see this as a case where the app invoked CFE_ES_WriteToSysLog() perfectly well, but was limited by the internal constraints of the implementation.

At any rate, happy to discuss the philosophy around error codes further in #483 .... should probably transition discussion over there ...