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

sys: add auto_init includes treewide #18003

Closed
wants to merge 1 commit into from

Conversation

fjmolinas
Copy link
Contributor

Contribution description

Current headers are only visible in the auto-init compile unit, make them visible whenever the module is used.

Testing procedure

green ci.

@fjmolinas fjmolinas added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Apr 25, 2022
@github-actions github-actions bot added the Area: sys Area: System label Apr 25, 2022
@benpicco benpicco requested a review from fabian18 April 25, 2022 14:31
@benpicco
Copy link
Contributor

I think this might be by design - which header do you need?

@fjmolinas
Copy link
Contributor Author

I think this might be by design - which header do you need?

The auto_init_priorities.h If I want to use those defines out of the auto init compilation unit (e.g. defining an external auto-init with priority AUTO_INIT_PRIO_MOD_DUMMY_THREAD + 1

@benpicco
Copy link
Contributor

Ah ok - but better move the file to sys/include then, maybe sys/include/auto_init/priorities.h?

@fabian18
Copy link
Contributor

#define MY_MODULE_PRIO AUTO_INIT_PRIO_MOD_DUMMY_THREAD + 1

does not evaluate to a number because the preprocessor does not substitude the result.
You can look into the .map file of you compiled application and search for auto_init_xfa to see the result.
For your external module you would have to #define the actual computation result.

@fabian18
Copy link
Contributor

Something which would work, would be concatenation, but then the numeric comparability would be broken, which was requested in the original PR

@fjmolinas
Copy link
Contributor Author

UTO_INIT_PRIO_MOD_DUMMY_THRE

Huh, true, if I did AUTO_INIT_PRIO_MOD_DUMMY_THREAD - 1 I actually get an error right away... too bad

@fjmolinas fjmolinas closed this Apr 25, 2022
@fjmolinas
Copy link
Contributor Author

UTO_INIT_PRIO_MOD_DUMMY_THRE

Huh, true, if I did AUTO_INIT_PRIO_MOD_DUMMY_THREAD - 1 I actually get an error right away... too bad

I do have a question regarding how to add or change priorities going forward? If I can't refer to the macro values, then I at some point there is a new module pushed earlier than lets say random all other modules will need to be bumped as well, and then my externally defined priority is meaningless, is there a better way to do this?

@fabian18
Copy link
Contributor

hmm I´m sorry. I thought this was clear : /
We decided to keep it that simple and avoid macro hacking with concatenation.

@fabian18
Copy link
Contributor

is there a better way to do this?

concatenation, something like this

#define AUTO_INIT_PRIO_AFTER(base, position) EVAL(base) ## position

#define MY_MODULE_PRIO AUTO_INIT_PRIO_AFTER(AUTO_INIT_PRIO_MOD_DUMMY_THREAD, 1)

or make the preprocessor do the math somehow.

@fabian18
Copy link
Contributor

fabian18 commented Apr 25, 2022

Just a kind of brute force idea: Have SUCCESSOR macros for a range of numbers [1010, 1550 ].
Quick and dirty untested, but the idea should be understandable:

#define SUCCESSOR_1070 1071

#define SUCCESSOR(x) SUCCESSOR_ ## EVAL(x)


#define MY_MODULE_PRIO SUCCESSOR(AUTO_INIT_PRIO_MOD_DUMMY_THREAD)

@fabian18
Copy link
Contributor

This would be zero overhead, but many lines of code and a bit ugly.

@fabian18
Copy link
Contributor

Maybe such a header could be auto-generated for a given range of numbers. Other modules could also profit from preprocessor +1 addition and -1 subtraction.

@fabian18
Copy link
Contributor

@fjmolinas
Copy link
Contributor Author

https://www.boost.org/doc/libs/1_67_0/boost/preprocessor/arithmetic/inc.hpp

Yep, I was looking into those right now as well, does pre-procesor introduce a lot of overhead? I think we should think of two things:

  • in tree maintainability: make it easy for us to move things up-down the priority queue
  • external maintainability: make it easy for external users to keep track of how we assign priorities.

If the compilation overhead is OK, then we could use boost for everything, and define all priorities as relative to a base priority. so if inserting something in the queue you just have to re-tagg the priority just after it. That would mean zero code changes for an external user, an only a couple for us.

@fabian18
Copy link
Contributor

does pre-procesor introduce a lot of overhead?

I think we would not even notice a difference in compilation times. But I have never experimented with it.

@fjmolinas
Copy link
Contributor Author

does pre-procesor introduce a lot of overhead?

I think we would not even notice a difference in compilation times. But I have never experimented with it.

I think it would be worth the try, I'll put it on my todo list (although with low priority)

@fjmolinas fjmolinas deleted the pr_auto_init_includes branch June 9, 2022 09:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: sys Area: System CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants