stm32-cmake icon indicating copy to clipboard operation
stm32-cmake copied to clipboard

Automatically enable used HAL driver modules

Open shoftmadder opened this issue 2 years ago • 11 comments

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.

shoftmadder avatar Mar 05 '23 02:03 shoftmadder

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.

Hish15 avatar Mar 09 '23 08:03 Hish15

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 avatar Mar 09 '23 22:03 shoftmadder

Cool I think that's good now

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

shoftmadder avatar Mar 11 '23 09:03 shoftmadder

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

atsju avatar Mar 11 '23 13:03 atsju

@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: https://github.com/ObKo/stm32-cmake/blob/5d70c4384e6b963ad5968501daa5870963f645c6/tests/hal/stm32g0xx_hal_conf.h#L55

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

shoftmadder avatar Mar 11 '23 23:03 shoftmadder

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 ?

atsju avatar Mar 12 '23 07:03 atsju

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

shoftmadder avatar Mar 13 '23 12:03 shoftmadder

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 ?

atsju avatar Mar 13 '23 12:03 atsju

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

shoftmadder avatar Mar 13 '23 12:03 shoftmadder

@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

Hish15 avatar Mar 13 '23 18:03 Hish15

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.

simoneruffini avatar May 16 '23 08:05 simoneruffini