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

Add the possibility to include a custom config file in FreeRTOSConfig.h #358

Closed
wants to merge 1 commit into from

Conversation

danicampora
Copy link
Contributor

This allows to add and change config values by simply adding a definition of FREERTOSPORTCONFIG_H inside sdkconfig.h.

For instance, in sdkconfig.h:

#define FREERTOSPORTCONFIG_H         "FreeRTOSPortConfig.h"

Inside FreeRTOSPortConfig.h:

// Enable support for both dynamic and static allocation
#define configSUPPORT_STATIC_ALLOCATION       1
#define configSUPPORT_DYNAMIC_ALLOCATION      1

#undef configTIMER_TASK_STACK_DEPTH
#define configTIMER_TASK_STACK_DEPTH          4096

This is needed by the ESP32 port of MicroPython: https://github.com/micropython/micropython-esp32

…Config.h

This allows to re-define values by simply adding a definitions of FREERTOSPORTCONFIG_H inside sdkconfig.h.

For instance, inside sdkconfig.h:

```c
#define FREERTOSPORTCONFIG_H         "FreeRTOSPortConfig.h"
```

Inside **FreeRTOSPortConfig.h**:

```c
// Enable support for both dynamic and static allocation
#define configSUPPORT_STATIC_ALLOCATION           1
#define configSUPPORT_DYNAMIC_ALLOCATION       1

#undef configTIMER_TASK_STACK_DEPTH
#define configTIMER_TASK_STACK_DEPTH        4096
```
@projectgus
Copy link
Contributor

projectgus commented Feb 21, 2017

Hi Daniel,

Thanks for sending this PR, it's a useful idea.

If we're going to add this option, it'd need to be exposed in a Kconfig file so that users could set it without manually editing sdkconfig.h (which is an automatically generated file, and not one we'd normally recommend leaving in source control - the sdkconfig file is the source file for this.)

What is the FreeRTOS configuration that Micropython needs to override? Ideally, it'd be closer to the "IDF way" if these were exposed as individual options in Kconfig. Then Micropython can ship an sdkconfig file or an sdkconfig.defaults file with these options set as necessary. The IDF build process will generate sdkconfig.h from this file.

Angus

@dpgeorge
Copy link

What is the FreeRTOS configuration that Micropython needs to override?

We want to support both static and dynamic allocation of tasks, so that when we create a new uPy thread the TCB and stack can be allocated within the uPy heap.

Then Micropython can ship an sdkconfig file or an sdkconfig.defaults file with these options set as necessary. The IDF build process will generate sdkconfig.h from this file.

We don't use the IDF build process, we use our own Makefile and just include a list of the IDF source files, which are then compiled by our system.

@Spritetm
Copy link
Member

Just to chime in: One of the reasons of us wanting to have a Kconfig equivalent is because the Kconfig documents what we officially support. Adding the ability to plunk in any random FreeRTOS configuration file implies that we do support and have tested anything you can throw into that, and that most definitely is not the case. So I second Angus here: we much rather prefer hearing about the options you want to have the ability to change then just add a catch-all include option.

@projectgus
Copy link
Contributor

projectgus commented Feb 22, 2017

Hi Damien,

Thanks for going into more detail about this. I admit I hadn't looked very closely at the Micropython ESP32 source yet!

We don't use the IDF build process, we use our own Makefile and just include a list of the IDF source
files, which are then compiled by our system.

One of the things we've discussed but haven't implemented is a way to do a "make standalone" in the IDF build process, the output of which is a set of static libraries and a single structured header directory. Such a build would take an sdkconfig file as input, so a specific output would correspond to a particular IDF version and a set configuration.

Is that something that would be useful for Micropython? To update this "standalone" IDF you'd need to maintain an sdkconfig file and a pointer to the IDF version to use (similar to the current ESPIDF_SUPHASH). However the rest of the Micropython build could be implemented using the Micropython build process without restriction - to link in IDF you'd provide one single header search directory, linker search directory, and list of the IDF libraries to link.

@dpgeorge
Copy link

Is that something that would be useful for Micropython?

Maybe... one thing I like about the way we do it now is that we have full control over everything: compiler flags, which compiler to use, and can even override some of the IDF source files with our own versions if needed (eg we do this for one of the linker scripts). But it would be nice to use your "make standalone" if it can be cleanly integrated into our build system.

@projectgus
Copy link
Contributor

compiler flags, which compiler to use, and can even override some of the IDF source files with our own versions if needed (eg we do this for one of the linker scripts).

OK. Linker scripts would have to be part of the output of a "make standalone" as well, now you mention it. So you would have flexibility there.

Regarding compiler flags, for things which are useful we're very receptive to merging them as configuration options in IDF itself. Or provide more hooks to override some flags when you run make (there are some compiler flag variables you can override already, but they may need more granularity to be useful.)

Regarding toolchains, there's an sdkconfig option for toolchain prefix so you can do this already. We recommend & support a specific toolchain for IDF, though.

But it would be nice to use your "make standalone" if it can be cleanly integrated into our build system.

OK. I'll take a look at this over the next week or so.

I think this will also help with flexibility from the Micropython side when changing IDF versions. We do semantic versioning of IDF public APIs, but we don't make any guarantees about not changing source file paths, build internals, and other such implementation details.

@danicampora
Copy link
Contributor Author

Thanks Angus and Joroen.

So I second Angus here: we much rather prefer hearing about the options you want to have the ability to change then just add a catch-all include option.

OK, so we need in principle a few things:

#define configSUPPORT_STATIC_ALLOCATION 1
#define configSUPPORT_DYNAMIC_ALLOCATION 1
#define configTIMER_TASK_STACK_DEPTH        CONFIG_TIMER_TASK_STACK_DEPTH

and

#define portCLEAN_UP_TCB(pxTCB)             some_function_to_clean_up_tcb(pxTCB)

See #359 for the details on the last one.

I can create another pull-request to add all those to kconfig (but I'm not sure how the the last one to clean up the TCB will fit there). Should I do that? Please let me know... Thanks!

@me-no-dev
Copy link
Member

Ok, let me see if I understand the "custom include" issue correctly. You want to be able to include some custom header in the system and you do not want to bother with the IDF build system?
Even so, you have somewhere "sdkconfig.h" that was previously generated by IDF and is included in all important places (could be added if missing somewhere), so if those includes are added in sdkconfig.h either by you or by the IDF build system on the next sdkconfig.h generation, you should be able to do what you want without messing with freertos or other component headers manually.
BTW I'm all up for adding such feature to menuconfig :)

@danicampora
Copy link
Contributor Author

@me-no-dev yes, your understanding is correct! :-)

@projectgus
Copy link
Contributor

Hi @danicampora sorry for the slow turnaround here, it was a busy week last week!

I can create another pull-request to add all those to kconfig

That would be great, thanks.

I'm not sure how the the last one to clean up the TCB will fit there).

Hmm. I guess there's a couple of ways this could possibly be done. I tend to favour weak-linked functions (in which case nothing needs to be added to sdkconfig, the "config" happens at link time). Although that's not really "the FreeRTOS way".

Or an API call to attach a cleanup hook to a particular task would also be an API choice I'd favour, although again this isn't the way FreeRTOS has designed it.

The simplest (and least invasive to FreeRTOS' design) change would be an optional "string" type config item for the macro value. If this value is defined and non-empty, we use that as the portCLEAN_UP_TCB value and otherwise we keep the default.

@Spritetm might also have some ideas about this one.

@projectgus
Copy link
Contributor

projectgus commented Feb 27, 2017

I'm not sure how the the last one to clean up the TCB will fit there).

Realised over lunch that my above suggestions were pretty rubbish.

Instead, I think a boolean config item "Expose static task cleanup hook" or something like this. When enabled, the FreeRTOS headers define a function signature and has the portCLEAN_UP_TAB macro evaluate to call that function. Then the project (Micropython in this case) is responsible for providing an implementation of that function at link time.

@danicampora
Copy link
Contributor Author

Instead, I think a boolean config item "Expose static task cleanup hook" or something like this. When enabled, the FreeRTOS headers define a function signature and has the portCLEAN_UP_TAB macro evaluate to call that function. Then the project (Micropython in this case) is responsible for providing an implementation of that function at link time.

@projectgus OK I like that one. I'll create another PR following your suggestions. Thanks!

@danicampora
Copy link
Contributor Author

Sorry for the long delay. Superseded by: #444

0xFEEDC0DE64 pushed a commit to 0xFEEDC0DE64/esp-idf that referenced this pull request May 5, 2021
* Update IDF to 65acd99

* Update platformio and arduino build paths and libs

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

Successfully merging this pull request may close these issues.

None yet

5 participants