Clarify and unify naming of thread-related constants (stacksize and priority) by x3ro · Pull Request #2881 · RIOT-OS/RIOT

x3ro · 2015-04-28T18:41:58Z

Changes as discussed in #2725

For base stacksizes: drop the KERNEL_CONF prefix in favor of THREAD.
- KERNEL_CONF_STACKSIZE_DEFAULT becomes THREAD_STACKSIZE_DEFAULT.
For supplemental stacksizes: drop the KERNEL_CONF prefix in favor of EXTRA.
- KERNEL_CONF_STACKSIZE_PRINTF becomes THREAD_EXTRA_STACKSIZE_PRINTF.

In addition:

Thread priorities previously called PRIORITY_... are now prefixed with THREAD_
MINIMUM_STACK_SIZE adapted to the naming scheme of the other stacksizes, thus becoming THREAD_STACKSIZE_MINIMUM

OlegHahm · 2015-04-28T18:47:59Z

boards/qemu-i386/include/cpu-conf.h

Indentation is off.

OlegHahm · 2015-04-28T18:57:20Z

You're planning to squash? Otherwise the commit messages are missing a proper prefix.

x3ro · 2015-04-28T18:57:44Z

Yep, planning to squash. Or would you say the commits should stay this way? Then I'd rename

haukepetersen · 2015-04-28T19:12:40Z

core/include/thread.h

I would prefer to put parenthesis around this

Even though it just maps to THREAD_PRIORITY_MIN which is in parentheses?

haukepetersen · 2015-04-28T19:14:09Z

ACK when comment above is addressed

OlegHahm · 2015-04-28T21:36:06Z

ACK

BytesGalore · 2015-04-29T05:16:13Z

looks good please squash, then lets see what travis says

miri64 · 2015-05-09T18:01:13Z

Please rebase

x3ro · 2015-05-09T18:34:06Z

@authmillenon Will do. Travis was unhappy about smth and I didn't manage to fix that yet, but I'll give my best to get it merged asap.

OlegHahm · 2015-05-16T10:33:55Z

Any news?

x3ro · 2015-05-17T17:51:58Z

Since I've touched a lot of files in this PR, I also have to deal with a lot of doxygen warnings that I have no clue how to fix. How do we handle this?

OlegHahm · 2015-05-17T17:53:32Z

Travis should only complain if the PR introduces doxygen warnings.

x3ro · 2015-05-17T17:55:03Z

Does it not complain about all warnings in files that have been changed in a PR? That was my impression so far.

OlegHahm · 2015-05-17T17:59:34Z

See https://github.com/RIOT-OS/RIOT/blob/master/dist/tools/doccheck/check.sh#L12:

 ERR_DIFFFILTER="--diff-filter=AC"
 WARN_DIFFFILTER="--diff-filter=MR"

Hence, only created and copied files should create errors, the rest should output only warnings.

x3ro · 2015-05-18T11:00:52Z

@OlegHahm The last Travis build passed but there was a minor merge conflict :( Can I merge this as soon as Travis comes back with a green light? It's ACKed otherwise, right?

OlegHahm · 2015-05-18T11:08:12Z

Yes, you can.

As discussed in #2725, this commit renames a number of stacksize constants to better convey their intended usage. In addition, constants for thread priority are given a `THREAD_` prefix. Changes are: * KERNEL_CONF_STACKSIZE_PRINTF renamed to THREAD_EXTRA_STACKSIZE_PRINTF * KERNEL_CONF_STACKSIZE_DEFAULT renamed to THREAD_STACKSIZE_DEFAULT * KERNEL_CONF_STACKSIZE_IDLE renamed to THREAD_STACKSIZE_IDLE * KERNEL_CONF_STACKSIZE_MAIN renamed to THREAD_STACKSIZE_MAIN * Move thread stacksizes from kernel.h to thread.h, since the prefix changed * PRIORITY_MIN renamed to THREAD_PRIORITY_MIN * PRIORITY_IDLE renamed to THREAD_PRIORITY_IDLE * PRIORITY_MAIN renamed to THREAD_PRIORITY_MAIN * Move thread priorities from kernel.h to thread.h since the prefix has changed * MINIMUM_STACK_SIZE renamed to THREAD_STACKSIZE_MINIMUM for consistency

OlegHahm · 2015-05-20T23:58:11Z

tests/driver_kw2xrf needs an update. 😩

x3ro · 2015-05-21T14:37:57Z

Build passed over at my fork's Travis. Is it okay if I merge?

OlegHahm · 2015-05-21T14:53:04Z

Works for me.

x3ro · 2015-05-21T14:54:42Z

Then brace for impact 💥

Clarify and unify naming of thread-related constants (stacksize and priority)

jonas-rem · 2015-05-21T15:29:45Z

Sry, this is too late now and maybe it´s a dump question. Why are the old auto-init apps introduced again? And why in this PR?

eg. tests/driver_at86rf2xx/auto_init_ng_netif/netif_app.c

tests/driver_kw2xrf needs an update. 😩

What exactly needs to be updated there?

x3ro · 2015-05-21T15:31:18Z

If they should not be there, then I probably messed up one of the many rebases :(

jonas-rem · 2015-05-21T15:36:15Z

Ah ok, yes they should not be there. The PR that changed the auto-init process is #2901. Maybe this was merged before your last rebase or so.

But this doesn´t break anything, so the files could just be deleted by any follow-up or so. Just asked because this could affect the driver implementation for kw2xrf.

x3ro assigned OlegHahm Apr 28, 2015

x3ro added the Area: core Area: RIOT kernel. Handle PRs marked with this with care! label Apr 28, 2015

x3ro mentioned this pull request Apr 28, 2015

Fix DEBUG_PRINT stack size check #2725

Closed

OlegHahm reviewed Apr 28, 2015
View reviewed changes

boards/qemu-i386/include/cpu-conf.h Outdated

Copy link
Copy Markdown

Member

OlegHahm Apr 28, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indentation is off.

OlegHahm added the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Apr 28, 2015

haukepetersen reviewed Apr 28, 2015
View reviewed changes

OlegHahm modified the milestone: Release 2015.06 Apr 29, 2015

x3ro force-pushed the clarify-stacksize-constants branch from adeb101 to eed76f2 Compare April 30, 2015 09:15

x3ro force-pushed the clarify-stacksize-constants branch from eed76f2 to abf650f Compare May 17, 2015 17:11

x3ro force-pushed the clarify-stacksize-constants branch 2 times, most recently from 5c20fec to 1183747 Compare May 17, 2015 21:53

x3ro removed the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label May 18, 2015

x3ro force-pushed the clarify-stacksize-constants branch from 1183747 to e5bc5a4 Compare May 18, 2015 10:59

x3ro force-pushed the clarify-stacksize-constants branch 2 times, most recently from f7cb1c7 to cf10a66 Compare May 18, 2015 21:22

x3ro force-pushed the clarify-stacksize-constants branch 3 times, most recently from a0f4b7a to f0bf928 Compare May 20, 2015 12:39

x3ro force-pushed the clarify-stacksize-constants branch from f0bf928 to 426170b Compare May 21, 2015 13:00

x3ro added a commit that referenced this pull request May 21, 2015

Merge pull request #2881 from x3ro/clarify-stacksize-constants

742c39e

Clarify and unify naming of thread-related constants (stacksize and priority)

x3ro merged commit 742c39e into RIOT-OS:master May 21, 2015

miri64 mentioned this pull request May 21, 2015

ng_net: add 6LoWPAN as dependency for IEEE 802.15.4 devices #2947

Merged

PeterKietzmann mentioned this pull request May 27, 2015

ng_sniffer: added ng version of sniffer application RIOT-OS/applications#6

Merged

Conversation

x3ro commented Apr 28, 2015

Uh oh!

OlegHahm Apr 28, 2015

Choose a reason for hiding this comment

Uh oh!

OlegHahm commented Apr 28, 2015

Uh oh!

x3ro commented Apr 28, 2015

Uh oh!

haukepetersen Apr 28, 2015

Choose a reason for hiding this comment

Uh oh!

x3ro Apr 28, 2015

Choose a reason for hiding this comment

Uh oh!

haukepetersen commented Apr 28, 2015

Uh oh!

OlegHahm commented Apr 28, 2015

Uh oh!

BytesGalore commented Apr 29, 2015

Uh oh!

miri64 commented May 9, 2015

Uh oh!

x3ro commented May 9, 2015

Uh oh!

OlegHahm commented May 16, 2015

Uh oh!

x3ro commented May 17, 2015

Uh oh!

OlegHahm commented May 17, 2015

Uh oh!

x3ro commented May 17, 2015

Uh oh!

OlegHahm commented May 17, 2015

Uh oh!

x3ro commented May 18, 2015

Uh oh!

OlegHahm commented May 18, 2015

Uh oh!

OlegHahm commented May 20, 2015

Uh oh!

x3ro commented May 21, 2015

Uh oh!

OlegHahm commented May 21, 2015

Uh oh!

x3ro commented May 21, 2015

Uh oh!

jonas-rem commented May 21, 2015

Uh oh!

x3ro commented May 21, 2015

Uh oh!

jonas-rem commented May 21, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants