core: make messaging optional#3851
Conversation
|
Interesting |
|
@kaspar030, may be it will be better to add new flag DISABLE_MESSAGING instead of MODULE_MSG - it makes more sense. I think is important PR, because not in every case we need messaging, especially if RIOT controls simple sensor node. |
|
Sorry, this got lost somehow. Could you rebase, @kaspar030? |
b1bae9f to
f08ff73
Compare
|
tests/sizeof_tcb/Makefile
Outdated
| include ../Makefile.tests_common | ||
|
|
||
| # optionally disable messaging | ||
| #DISABLE_MODULE += msg |
There was a problem hiding this comment.
Wouldn't it make sense to comment this in here?
Maybe you could add this snippet also to dist/Makefile?
There was a problem hiding this comment.
Wouldn't it make sense to comment this in here?
hm?
There was a problem hiding this comment.
I meant: why not indeed disabling the msg module here?
There was a problem hiding this comment.
Ah. Nope, IMHO sizeof_tcb should show the tcb size in the general case. disabling messaging seems more on the special side.
There was a problem hiding this comment.
Ah, yes, sorry, confused with the minimal example.
|
I'm not sure, I see a concrete use case for that right now, but increasing modularity and making more features optional seems always a good idea if the overhead is manageable - which is the case for this change. |
dc1c004 to
89e3727
Compare
|
I've re-done the PR, it felt pointless to create a whole directory for msg, now I'm using ifdef within msg.c. I've moved thread_print_msg_queue() to msg.c and renamed it to msg_queue_print(). @authmillenon, do you approve? |
|
I would prefer creating a new directory. Other then that |
|
(especially since compilers will complain about the empty compile unit). |
|
((I saw enough of this type of "modulization" in lwip and emb6 that I know I don't want to have that in RIOT ;-))) |
why?
|
|
I gave already a bunch of reasons but here are more:
Compared to that:
|
|
Martine, how is it possible that we end up having completely opposite opinions on so many matters? ;)
Subjective. I don't agree. At least it is a lot less ugly than a subdirectory.
this is not the first module to do this.
Meaning - what?
It was in a subdirectory before the last commit(s). That looked ugly and completely unnecessary. Mind you - the whole idea of disabling messaging is more an excercise of "because we can". One-file modules in their own directory always seemed like a deficiency of the build system anyways. |
|
Hm, difficult thing. On the one hand, having a subdirectory is kinda ugly, particular since it would be the only subfolder in |
|
Well if #4919 is imported and also added as a directory |
|
But fine. If I'm the only one that diggs subdirectories I'm fine with #ifdef blocks |
Which other pseudo-module does this? I see only dependency-collecting modules, modules that are only header files, common API modules, and "pseudo-submodules" (modules that add additional functionality to an existing module) in Makefile.pseudomodules. There is no module that is exactly like a normal module, but is a pseudo-module because the complete code for is in an |
|
Needs a rebase. |
89e3727 to
27ee8e4
Compare
|
I like the PR and I agree with @kaspar030, that I don't really like putting it into a separate folder. The only thing I think could make sense, is to rename the module to so 1st ACK |
|
Yeah. |
d0439c5 to
cb3c991
Compare
|
cb3c991 to
b36b802
Compare
|
Would someone give a second ACK? |
|
green! :) |
dist/Makefile
Outdated
| # If your application is very simple and doesn't use modules that use | ||
| # messaging, it can be disabled to save some memory: | ||
|
|
||
| #DISABLE_MODULE += msg |
b36b802 to
58a12e5
Compare
|
One minor doc thingy. Otherwise, fine... ACK |
|
Thx! addressed & amended. |
In order to enhance RIOT's modularity, make messaging optional.
When not used but compiled in, messaging uses a whopping 24b of code on 32bit platforms, but doubles the size of each tcb (16b -> 40b).
Not sure the savings are worth the effort, but who knows...
(Added msg to DEFAULT_MODULES, so this will not affect anything but those brave souls who compile RIOT with
DISABLE_MODULE += msg).