Automatically enable used HAL driver modules

[shoftmadder]

Currently, to add a driver module to a project, you have to:

• Add the driver module in CMakeLists.txt
• Set the driver module enabled (e.g. in a HAL config file, such as stm32f4xx_hal_conf.h)

This change automatically sets the driver module enable definition when a module is included in CMakeLists.txt, meaning that you don't also have to edit a header file to enable the module.

[Hish15]

Hello @shoftmadder, That's a nice catch ! I never noticed that!
(If you include the drivers files where you need them (#include "stm32f1xx_hal_dac.h" for instance), you don't see it.)

Can you change the exemples, in particular blinky, so it uses your change ?
I mean changing stm32xxxx_hal_conf.h files.

[shoftmadder]

Yep, I can modify the examples -- something along the lines of this in the various .h files?

/* ########################## Module Selection ############################## */
/**
  * @brief This is the list of modules to be used in the HAL driver
  *
  * `stm32-cmake` automatically defines the `HAL_*_MODULE_ENABLED` variables
  * when a driver is added to a project as a target, so we don't need these here
  */
...

And then comment out all of the #defines below

---

As an aside, I could never work out exactly what needed including in what order to use the hal drivers as individual headers, which is probably why I noticed this!

[shoftmadder]

Cool I think that's good now

If it matters, I'm assigning all rights to the authors of this project

[atsju]

I believe there is a slight difference as now flash.c is compiled.
Before your change, only flash.h was included but flash.c not built.

[shoftmadder]

Ooh, good catch. Not sure how best to address this?

[atsju]

It's not an issue from my point of view we can live with it. Will see with @Hish15

However, have a look at workflow failure

[shoftmadder]

The root cause is that:

• The STM32 modules depend on each others' headers, but
• The modules only include stm32fxx_hal.h (rather than the headers they actually need), and
• stm32fxx_hal.h only includes headers (via stm32fxx_hal_conf.h) for enabled modules (HAL_XYZ_MODULE_ENABLED)

Having thought about it, the options I can see are:

1. Explicitly encode the dependencies somewhere, so that each driver module sets HAL_XYZ_MODULE_ENABLED for both itself and also its dependencies.
   • A lot of work, and potentially a bit fragile, but not impossible.
   • Could test by enabling only one driver module at a time, and checking it builds -- but this wouldn't test that the dependencies are minimal
2. Add an "enable only" target (e.g. HAL::STM32::F4::ENABLE_FLASH) so that the user can enable the headers for any modules that are depended upon, without needing to build the corresponding .c file
3. Leave it like this -- the driver module .c files don't take that long to compile anyway

2 and 3 have the downside of leaving the header dependencies up to the user, but this is currently the case with stm32fxx_hal_conf.h anyway

[atsju]

If it matters, I'm assigning all rights to the authors of this project

It doesn't matter for us. However, it makes me ask myself more questions than it answers :)

@Hish15 please see my review comment and choose about merging

[atsju]

@shoftmadder #317 workflow builds fine. You will have to find out why test fails on MP1 and G0 families with your change.

[shoftmadder]

@shoftmadder #317 workflow builds fine. You will have to find out why test fails on MP1 and G0 families with your change.

Problems are due to modules being enabled (HAL_XYZ_MODULE_ENABLED) that weren't enabled before.

G0

The HCD module is broken in older versions of STM32CubeG0. The test environment appears to be using an older version of STM32CubeG0.

These tests work on master (in the test environment with the older version of STM32CubeG0) since HAL_HCD_MODULE_ENABLED is explicitly commented out in stm32g0xx_hal_conf.h:

stm32-cmake/tests/hal/stm32g0xx_hal_conf.h

Line 55 in 5d70c43

// #define HAL_HCD_MODULE_ENABLED

Obviously, on this branch, HAL_HCD_MODULE_ENABLED is defined, since the module is used in target_link_libraries.

The HCD module is fixed in STM32CubeG0 >= 1.5.0 -- see the fix here

Recommended solution -- move to a non-broken version of STM32CubeG0 in the test environment

MP1

For MP1, the MDIOS module is the broken in varied and interesting ways. There doesn't appear to be a working version.

HAL_MDIOS_MODULE_ENABLED doesn't appear at all in stm32mp1xx_hal_conf.h, and so wasn't included in tests before. This patch obviously adds the definition (again, since the module is used in target_link_libraries)

I've modified stm32mp1xx_hal_conf.h to explicitly disable the broken module

[atsju]

Recommended solution -- move to a non-broken version of STM32CubeG0 in the test environment

It's not only for test environment. You must update lines 47-49 from utilities.cmake for doing that

About MP1: having -D by compiler and #undef in one file only, the result will depend on the how user includes files.

---

Thinking out loud: user is asking for specific (sometimes all) components in find_package(HAL. in blinky it's HAL_COMP_LIST.
Maybe you can try to target_compile_definitions every requested component in each library of the family. For blinky this will lead to needing FLASH in HAL_COMP_LIST but not the actual library HAL::STM32::F4::FLASH with the sources. This would be more in the spirit of HAL. For users that do not specify specific HAL components but only the family it would behave as if all HAL were included but not built (my typical lazy hal_conf.h just enables everything so this behaves the same). However sources would be built only if needed.
I had a quick glance and it seems doable. Dos it seem reasonable ?

[shoftmadder]

I've added <DRIVER>_ENABLE targets, which I think is most in keeping with the spirit of HAL -- it's basically just the HAL_<DRIVER>_MODULE_ENABLED macros in the configuration file, but in CMake.

The <DRIVER> targets depend on the <DRIVER>_ENABLE targets (so auto-enable themselves), while required headers can be enabled with the <DRIVER>_ENABLE target. This leaves dependency management in the hands of the user, but it's at least no worse than the situation before in stm32fxx_hal_conf.h.

I've also added the ability to exclude drivers from autodetection -- this means that the (broken) MDIOS driver can be excluded for the MP1 family.

Finally, I bumped the G0 version to one with a working HCD driver

I think, with these changes, the tests should pass

[atsju]

I'm a bit mitigated, this is not exactly what I meant.
Also note you should see the test on your fork repo. Maybe by just modifying a bit the https://github.com/ObKo/stm32-cmake/blob/master/.github/workflows/cmake.yml Here I need to approve every run.
@Hish15 any though ?

[shoftmadder]

I didn't like the solution of enabling all searched-for HAL modules by default -- I am approaching as "doing in cmake what you are doing in stm32fxx_hal_conf.h". In that file, you still have to manually uncomment HAL_<DRIVER>_MODULE_ENABLE lines; I don't see a problem with needing to manually specify <DRIVER>_ENABLE in CMake.

And, manually enabling modules in stm32fxx_hal_conf.h still works! So your "lazy" hal conf file will still work exactly as it did before -- this patch is providing an additional mechanism for those who want to use CMake to enable HAL modules

[Hish15]

@shoftmadder Thanks for the changes, I will discuss this with @atsju. I think I understand your thought process, but I'm not sure about the drop_excluded_hal_drivers approach.
We will come back to you.

Cya

[simoneruffini]

I would like to add that if you use a tool like STM32 CubeMX it automatically does that for you: changing the stm32fxxx_hal_conf.h file with the correctly enabled peripherals.