boards/nucleo: concept for vastly improving code re-usage [WIP] #8044
Conversation
haukepetersen
added
Area: boards
|
For the moment, I've only looked at the PR description and not the code
I totally with that. Since I'm mainly the one to blame for this code duplication/maintenance hell in nucleos, I was also thinking that factorizing things is required.
Yes, I'm +1 with that. in #7277 you already started to do that and I'm pretty in favor of that. But it's simple for ADC because the configuration are quite the same for a large number of boards (espacially nucleo 64) For other peripherals it might be more complex. Maybe using linker tricks (like for isr vectors), for grouping list could make it with UART or PWM. |
| @@ -0,0 +1,2 @@ | |||
| MODULE = board | |||
There was a problem hiding this comment.
I think naming it board is a bad idea, it breaks the current conventions.
Having common boards implementing the board module will lead to errors.
(It happened with the cpu for #8032)
I would rather see it called board_nucleo_common and depend on it by including Makefile.include/Makefile.features as we currently do for other boards common.
There was a problem hiding this comment.
Actually, in this case I do it on purpose, as the actual boards don't produce any archive, so this is the only compilation result produced by all the nucleo board folders
There was a problem hiding this comment.
They produce an empty and ignored $(dirname).a file.
$ ls bin/nucleo-f411/nucleo-f411*
bin/nucleo-f411/nucleo-f411.a
bin/nucleo-f411/nucleo-f411:
And if a specific nucleo-wtf3000 uses the common module, and still needs one specific C file, it will add USEMODULE += nucleo-wtf3000 in its board Makefile.include which is not conventional for boards.
| * @author Hauke Petersen <[email protected]> | ||
| */ | ||
|
|
||
| #ifndef PERIPH_ADC_144_AC_H |
There was a problem hiding this comment.
Why naming it periph_..144_ac" instead of putting it in the nucleo144/include` directory ?
Maybe I am missing something.
There was a problem hiding this comment.
This still needs to be sorted out, I have the feeling that there won't be a nucleo144/include once this effort ist done...
There was a problem hiding this comment.
It's because of the corresponding port of the configured ADCs. For those type of boards, you have 2, one on port A and one on port C.
There was a problem hiding this comment.
some other has 6 => aaaccc (or something like this)
There was a problem hiding this comment.
@aabadie was refering to the 144 in particular, but it was not clear.
There was a problem hiding this comment.
as stated above: let's not dive into these kind of specific questions (yet), as the conversion is far from complete and not everything is figured out yet...
There was a problem hiding this comment.
While the Nucleo-X boards do undeniably all support ADC, it is just wrong to support the ADC peripheral at BOARD level in any way as the ADC in Nucleos are always feature of the MCU.
Some Nucleo boards do have peripherals that are external to the MCU and supporting them at the BOARD level files is proper and good. The clock resonator configuration is a great example of something that is a Nucleo BOARD level feature. Other very good examples of BOARD features in Nucleo are the buttons and LEDs. Some Nucleos even have network chips and acceleration sensors and the like soldered on board.
Also more about ADC: since the Nucleo family is specifically designed to map out all MCU pins, all of the Nucleo boards support all the peripherals that the MCU happens to have, which means there are multiple multichannel ADC devices, hordes of multiple types if timers, SPI, I2C, CEC, some have LCD drivers etc etc.
Most of these peripherals are features of MCU and thus discussing WHICH BOARD level file should support them is moot since the answer is: none. They should be supported in MCU files.
There was a problem hiding this comment.
Most of these peripherals are features of MCU and thus discussing WHICH BOARD level file should support them is moot since the answer is: none. They should be supported in MCU files.
I'd say this is only partly correct, as there are some implications to consider:
- all the peripherals that can be mapped to alternative pin mappings: the board configuration must specify which mapping to use (how should the generic CPU code know this?) (e.g. SPI, UART, ...)
- all peripherals that use resources (e.g. RAM for state): the board configuration must specify which ones and how many of them to use (e.g. timers, UART, ...)
Now (for simplicity let's stay in the domain of STM CPUs) the ADC falls into the second category, as the driver at least allocates memory for 1 mutex per ADC device -> so IMHO, there has to be some kind of configuration in the board!
+1 Would also like to do the same for CPUs. (would require #8032) I also think it should be done with the current architecture before refactoring boards together. |
Done just that: #8058 |
|
As info, I will not bring this PR in a mergable state, but PR the ideas included in this PR separately. I will leave this one open for now, as the discussion here might not be quite over... |
| @@ -0,0 +1,5 @@ | |||
| # include nucleo common serial configuration | |||
| include $(RIOTBOARD)/common/nucleo/Makefile.include.serial | |||
There was a problem hiding this comment.
Its included in Makefile.include now, required to try compiling.
There was a problem hiding this comment.
yes, and I am not sure, but I think I fixed it in #8065 (at least I hope so...).
|
@haukepetersen now that #8058 has been merged I guess we can close this one? |
|
actually, the changes in this PR are rather targeted by #8065, and that only partly. I'd like to leave this PR open for just a little bit longer, until I have opened one ore two more PRs containing the rest of the ideas presented here... |
When talking offline about #7975, @kaspar030 and me came to the conclusion, that the approach in #7975 might not be the best way to go, but that it might be better to keep one folder per board but rather improve the usage of common folders.
Following this idea, I thought it might be best to look at the family of boards with by far the most code duplication: the nucleos. This PR is still heavy work in progress, but I wanted to showcase/discuss some approaches before spending more time on this. Although many boards are touched by this PR, please look only at the
nucleo144-f746for now, as this is (I think) the only nucleo that builds currently...Concepts I'd like to get feedback on:
- Putting shard code into
boards/common/xxThis 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/stmand put STM specific clock configurations (likeperiph_clk_f4_180.hin 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-commonfolder into boards, which might look a little strange, and I think having a structuredboards/common/xxplace for configuration snippets might be cleaner.- Splitting the
periph_conf.hin snippetsLooking over all the nucleo boards (for now the 144ers), the
periph_conf.hfiles are mostly to 70% identical. We have some 2-dimensional mapping, where some configuration items are identical for nucleo families (144, 64, 32 -> e.g. UART), and others are dependent on the used CPU (f4 vs f7 vs f2 -> clock config), but again the same over all Nucleo variants.My proposed solution: we split the complete peripheral configuration into snippets, each configuration one specific peripheral. I did this by adding a single header file for each snippet, so that the actual board specific
periph_conf.hfile is only including a list of these pre-defined snippets. So far I tried to come up with some more or less speaking naming scheme (e.g.periph_clk_f4_100.h-> clock config forstm32f4at 100MHz orperiph_uart_144_261.h-> uart config for nucleo-144 boards using Uart2, Uart6 and Uart1).As stated above, this concept is not really tied to the Nucleo boards per-se, but could also be more generalized to be used by other boards. But for now I'd like to keep this confined to the nucleos to get a better understanding of the impacts of this approach first.
So what do you think? I only touched (some of the) nucleo144 boards for now, but already save 1000 lines of code and found 3 configuration errors. So to me this approach seems pretty legid :-)