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

Documentation/usage mismatch in CFE ES "Start App" command #1004

Closed
jphickey opened this issue Nov 5, 2020 · 1 comment · Fixed by #1022 or #1027
Closed

Documentation/usage mismatch in CFE ES "Start App" command #1004

jphickey opened this issue Nov 5, 2020 · 1 comment · Fixed by #1022 or #1027
Labels
Milestone

Comments

@jphickey
Copy link
Contributor

jphickey commented Nov 5, 2020

Describe the bug
The CFE_PLATFORM_ES_DEFAULT_STACK_SIZE is documented as being a default stack size, not a minimum stack size. But the CFE ES "Start App" command enforces it as a minimum value here:

else if (cmd->StackSize < CFE_PLATFORM_ES_DEFAULT_STACK_SIZE)
{
CFE_ES_TaskData.CommandErrorCounter++;
CFE_EVS_SendEvent(CFE_ES_START_STACK_ERR_EID, CFE_EVS_EventType_ERROR,
"CFE_ES_StartAppCmd: Stack size is less than system Minimum: %d.",
CFE_PLATFORM_ES_DEFAULT_STACK_SIZE);
}

But this is not in agreement with how it is documented:

/**
** \cfeescfg Define Default Stack Size for an Application
**
** \par Description:
** This parameter defines a default stack size. This parameter is used by the
** cFE Core Applications.
**
** \par Limits
** There is a lower limit of 2048. There are no restrictions on the upper limit
** however, the maximum stack size size is system dependent and should be verified.
** Most operating systems provide tools for measuring the amount of stack used by a
** task during operation. It is always a good idea to verify that no more than 1/2
** of the stack is used.
*/
#define CFE_PLATFORM_ES_DEFAULT_STACK_SIZE 8192

To Reproduce
N/A

Expected behavior
Should not enforce the default as a minimum.

I don't see any CFE platform definition for an enforced minimum stack size. If I remember correctly this was discussed once or twice and the agreement was that this is an operational issue - stack size requirements depend on the app stack usage and the memory constraints of the platform - so CFE cannot (and should not) impose some random limitations on it - it should attempt to do what the user requested.

So recommendation would be to remove this check.
One valid possibility is that if the stack size is specified as 0 (which is definitely not valid), to use the default value instead.

System observed on:
Ubuntu 20.04

Reporter Info
Joseph Hickey, Vantage Systems, Inc.

@skliper
Copy link
Contributor

skliper commented Nov 5, 2020

One valid possibility is that if the stack size is specified as 0 (which is definitely not valid), to use the default value instead.

I like this, agreed on don't force as min.

jphickey added a commit to jphickey/cFE that referenced this issue Nov 17, 2020
Do not enforce CFE_PLATFORM_ES_DEFAULT_STACK_SIZE as a minimum,
it should be a default.
jphickey added a commit to jphickey/cFE that referenced this issue Nov 17, 2020
Do not enforce CFE_PLATFORM_ES_DEFAULT_STACK_SIZE as a minimum,
it should be a default.
@astrogeco astrogeco added this to the 7.0.0 milestone Dec 1, 2020
@astrogeco astrogeco added the bug label Dec 1, 2020
astrogeco added a commit that referenced this issue Dec 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants