boards: move shared code to boards/common/xx

[haukepetersen]

As discussed/proposed in #8044: this PR puts all shared board code under /boards/common/xx/ instead of using shared folders as in boards/xx-common/.

Quote:

This allows us to use shard code among boards, that are not logically connected to each other (e.g. not part of one family or revisions of each other etc). So for example we could add boards/common/stm and put STM specific clock configurations (like periph_clk_f4_180.h in this PR) into this 'module'. These configuration snippets can then simply be used from any board!

Right now this would mean to put a stm-common folder into boards, which might look a little strange, and I think having a structured boards/common/xx place for configuration snippets might be cleaner.

[cladmi]

Do we want to make the common boards in a separate group for the documentation, like board_common ? This way it could be displayed separately in the documentation http://doc.riot-os.org/group__boards.html

If it is wanted, I can try something after this PR is merged.

[haukepetersen]

If it is wanted, I can try something after this PR is merged.

Yes, I think this would be a great idea. I intentionally did not touch any doxygen grouping with this PR, so adding that on top would be my preferred approach.

[cladmi]

Is it required to write commits so that every commits work? Or at least that we try to.

I ask because right now, nucleo144 and nucleo32 commits will not work as nucleo-common is migrated to common/nucleo in a following commit.

And also, things will be broken before the last fix commits.

[kaspar030]

Is it required to write commits so that every commits work? Or at least that we try to.

We have the (unwritten) rule that commits must work on PR merge commit level. So, if some commits within one PR don't build unless some other commits of the same PR get merged, that's fine.

[kaspar030]

next time use "git mv" ;)

[pyropeter]

Git does not handle renames. git mv just calls git rm, mv and git add. This is likely a glitch from gits magic rename detection code that tries to guess if files were renamed while looking at a changeset.

[kaspar030]

pre-ACK. We should give a change of this magnitude a little more time. @gebart @miri64 @smlng what do you think?

[haukepetersen]

We should give a change of this magnitude a little more time

+1

[aabadie]

Thinking of it, I would also add a boards/common/stm: there we can put what's common to all stm based board. This directory could be the base not only for nucleo but also the stm32discovery, the b-l072z-lrwan1 (I hate this name), bluepill, the one that will come with #7585, etc
The same idea could apply to sam0 based board (arduino-zero, sodaq-, arduino-mkr)

What do you think ?

[smlng]

While I agree with the idea in general, I don't like the approach taken here. Major issue/concern I have with it is, that it moves related code into different levels of subdirectories which should (semantically) reside on the same.

What I mean is as follows, with this approach we'll get something like:

boards/arduino-mega2560
boards/arduino-nano
boards/arduino-uno
...
boards/common/arduino-atmega
...
boards/common/msba2
boards/common/nucleo
...
boards/common/wsn430
...
boards/nucleo-f411

While the (semantically) better approach (I suggest) would be, to group similar boards into one subdirectory which would have a common folder, i.e., the above would like:

boards/arduino-atmega/common
boards/arduino-atmega/arduino-mega2560
boards/arduino-atmega/arduino-nano
boards/arduino-atmega/arduino-uno
...
boards/msba2/common
...
boards/nucleo/common
boards/nucleo/nucleo-f411
...
boards/wsn430/common

Though I know this might be more difficult to coop with our make/build system, it is IMHO more logical.

[miri64]

While the (semantically) better approach (I suggest) would be, to group similar boards into one subdirectory which would have a common folder, i.e., the above would like:

I really don't like that proposal. I think it is really nice that you basically can get all supported boards by doing

ls boards/ | grep -v "common"

and the current proposal makes that even easier, since you can basically ditch the grep -v "common" and just ignore the common directory "manually".

With your proposal that would be more complicated:

ls boards/*/ | grep -v "common"

I don't think it's more or less logical logical than the one @haukepetersen proposed. Your suggestion implies that all stuff is at its next higher hierarchy (both common and specific), @haukepetersen's approach implies that all common things are in one place, ordered by the next higher hierarchy.

[haukepetersen]

@smlng I don't agree for multiple reasons:

1. the list of supported board (as in one folder per board) should be in a single, flat directory. Everything else would add unnecessary complexity to the build system, but would also IMHO make a developer's life (especially for non-core-people) harder, as they would have no clue where there board implementation resides -> simplicity in this case is king! With a flat directory containting all the boards, you would always have a starting point from where one can deduct all the rest of shared modules. Also keeping the flat top-level board structure is non-intrusive and does not require anything changed in the build system.

2. (and this is my much stronger argument) Boards and their shared code don't have a strict hierarchical structure! As this is mostly the case for our supported CPUs (CPU_ARCH -> CPU_FAM -> CPU -> CPU_MODEL), this is not the case for common board code. Sure we could map something like stm32 -> nucleo -> nucleo144 -> nucleo144-f746 or sam0 -> arduino-mkr -> ardunino-mkr1000 to a sub-folder structure, but what if we also want to introduce a sub-module, that is used by all stm32 and sam0 CPUs? This would not fit in this structure!

So looking at

boards/stm32/common
boards/stm32/nucleo/common/nucleo144/
boards/stm32/nucleo/nucleo144-f746/
boards/stm32/opencm904/

boards/sam0/common
boards/sam0/arduino-mkr/common
boards/sam0/arduino-mkr/arduino-mkr1000
boards/sam0/arduino-mkr/arduino-mkrzero

compared to

boards/nucleo144-f746
boards/opencm904
boards/arduino-mkr1000
boards/arduino-mkrzero
boards/common/stm
boards/common/nucleo
boards/common/nucleo144
boards/common/sam0
boards/common/arduino-mkr

I'd am strongly for the second, flat, and simple structure!

On top, keeping the structure flat and simple, it means that we can easily create 'vertical' shared modules, that don't fit in the context of a certain board or cpu family. So for example we could easily add a module like boards/common/tools and boards/common/tools and put in specific Makfiles for the 2 prevalent serial configuration that we encounter everywhere in the board code.

[haukepetersen]

Or in less words -> 'What @miri64 said` :-)

[jnohlgard]

I like this proposal. Especially being able to create common clock configurations for all boards sharing a single CPU will reduce code duplication a lot.
I agree with @miri64 and @haukepetersen regarding the structure, it makes it easier to get an overview rather than to have to search for names

[smlng]

Or in less words -> 'What @miri64 said` :-)

Not really -- @miri64 mainly complained that my suggested approach would make it harder to use grep, while you put up some more valid arguments. However, on the one hand you over exaggerated my approach and on the other you also started to mix CPU/MCU and boards, i.e., there is no sam0_common folder in boards right now -- or do you plan to introduce one? At least I wasn't.

For the former: my idea wasn't to introduce an arbitrarily deep directory structure for boards, but rather group those boards that already have a <board-prefix>_common folder into <board-prefix>/ with subdirs <board-prefix>/common, <board-prefix>/<board1>, ..., <board-prefix>/<boardN>. Mapping this onto the current board structure is as easy as your approach, besides that the nucleos are poorly/inconsistently named -> nucleo-* should be nucleo64-* to match with nucleo32-* and nucleo144-*.

Anyhow, I agree with you that simplicity is key, nevertheless in the long run a little structure might also help. My points is, that we should not hastily restructure boards without considering and discussing alternatives - hence my suggestion, to show there are other possibilities.

[miri64]

Not really -- @miri64 mainly complained that my suggested approach would make it harder to use grep

I also spoke about the hierarchical implications. And I said that with this approach I won't even need grep ;-).

[smlng]

it makes it easier to get an overview rather than to have to search for names

so you think searching in a flat hierarchy is faster than traversing a (shallow) tree 😉

Assuming you have a random nucleo144-* or want to buy one and see which one is supported by RIOT. With @haukepetersen approach @miri64 would have to ls boards/ | grep nucleo144 and with my suggestion this becomes ls boards/nucleo144/.

Though I admit, getting an overview over all boards is a bit more complicated as @miri64 showed above - you have to use boards/* to find all 😮

[jnohlgard]

@smlng

there is no sam0_common folder in boards right now -- or do you plan to introduce one? At least I wasn't.

I intend to introduce a kinetis directory for the kinetis clock configuration which will be shared between all Kinetis boards. My previous proposal was to put it in cpu, but you suggested that it should not be in cpu but under boards, so with this method in combination with the same ideas in #8044 we can reduce code duplication.

[smlng]

@gebart I understand, maybe we should also consider #8066 here?

[miri64]

With @haukepetersen approach @miri64 would have to ls boards/ | grep nucleo144 and with my suggestion this becomes ls boards/nucleo144/.

Nope, far to complicated :P I would just use ls boards/nucleo144*, and I usually try to get an overview over all boards than trying to single out a specific board (with boards like the PhyNode I would otherwise be lost ;-)).

[cladmi]

I would merge it yes.

But as @kaspar030 said "We should give a change of this magnitude a little more time".
I think we can still wait at least 3 more days to have a symbolic 2 week elapsed since the PR creation to gather feedback.

It could also be an occasion to write the hierarchy in the documentation.

@gebart would that give you time to review it ? do you agree to merge it without your review or do you want a bit more time ?

[jnohlgard]

I agree with merging even without my review as long as at least one other person has reviewed it

[PeterKietzmann]

Three days passed

[smlng]

needs rebase, my ACK holds

[haukepetersen]

and only 5 new conflicts... As stated before, waiting with this kind of PR is no fun for the author :-)

[aabadie]

waiting with this kind of PR is no fun for the author :-)

It's no fun either for authors of PRs introducing new boards ;)

[haukepetersen]

It's no fun either for authors of PRs introducing new boards ;)

Nope, neither the hen nor the eggs have fun :-)

[haukepetersen]

rebased

[cladmi]

Murdock is happy, no one complained.

[haukepetersen]

awesome, thanks everyone.