Navigation Menu

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

Allow mbed_app.json to override OS_IDLE_THREAD_STACK_SIZE for Nordic targets #5801

Closed
saedelman opened this issue Jan 8, 2018 · 10 comments
Closed

Comments

@saedelman
Copy link

saedelman commented Jan 8, 2018

Description

  • Type: Bug
  • Priority: Minor

Bug

The iOS_IDLE_THREAD_STACK_SIZE define is hard coded for Nordic targets in:

mbed-os/targets/TARGET_NORDIC/mbed_rtx.h

This should be wrapped in #ifndef/#endif otherwise the user defined setting in mbed_app.json is ignored.

Target
NRF52_DK

Toolchain:
GCC_ARM

mbed-cli version:
1.1.1

Enhancement

It would be nice if the idle thread and timer threads were named so that they are easily identified when dumping threads stats (using mbed-memory-status for example).

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 8, 2018

I checked, there are at least 2 platforms that define this macro, none use ifndef.

It would be nice if the idle thread and timer threads were named so that they are easily identified when dumping threads stats (using mbed-memory-status for example).

For this, I assume instead of NULL, there could be a name:

static const osThreadAttr_t os_idle_thread_attr = {
  NULL,
  osThreadDetached,
  &os_idle_thread_cb,
  (uint32_t)sizeof(os_idle_thread_cb),
  &os_idle_thread_stack,
  (uint32_t)sizeof(os_idle_thread_stack),
  osPriorityIdle,
  0U, 0U
};

@pan- @ARMmbed/team-nordic

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 8, 2018

For the names in idle thread and timer threads, can you create an issue in CMSIS 5: https://github.com/ARM-software/CMSIS_5/ with details , to understand the use case and make this request upstream. It could be configurable (default NULL as it is).

@pan-
Copy link
Member

pan- commented Jan 8, 2018

@bulislaw Is there any plan to allows compile time configuration of the RTOS ?

@bulislaw
Copy link
Member

bulislaw commented Jan 9, 2018

I would say it makes sense to have the sizes configurable for timer and idle thread stacks. Eg translate the obscure config defines to the system names here https://github.com/ARMmbed/mbed-os/blob/master/rtos/TARGET_CORTEX/mbed_rtx_conf.h#L37. And mbed_app.json would be the right place. @saedelman would you be able to create PR for that?

@JonatanAntoni
Copy link
Member

We added some defines to add names to idle and timer threads, see aa7a332.

@saedelman
Copy link
Author

@JonatanAntoni Thank you.

@bulislaw Agreed. If the RTOS allows me to define my own idle function (e.g. Thread::attach_idle_hook()) then I should be able to define the stack for it by overriding OS_IDLE_THREAD_STACK_SIZE, or it will most certainly lead to an overflow. I'm happy to create a PR, can you clarify how I do that? Not familiar.

@bulislaw
Copy link
Member

@saedelman Hava a look how main thread stack is configured here: https://github.com/ARMmbed/mbed-os/blob/master/rtos/TARGET_CORTEX/mbed_rtx_conf.h#L31 it'll be the same for the other stacks.

@marcuschangarm
Copy link
Contributor

@saedelman

The hard coded OS_IDLE_THREAD_STACK_SIZE has been removed from mbed-os/targets/TARGET_NORDIC/mbed_rtx.h and shouldn't interfere with your application anymore: #6845

@ciarmcom
Copy link
Member

ciarmcom commented Jun 1, 2018

ARM Internal Ref: MBOTRIAGE-294

@0xc0170
Copy link
Contributor

0xc0170 commented Jun 18, 2018

The hard coded OS_IDLE_THREAD_STACK_SIZE has been removed from mbed-os/targets/TARGET_NORDIC/mbed_rtx.h and shouldn't interfere with your application anymore: #6845

This issue should be now resolved. I'll close it

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

No branches or pull requests

8 participants