panda icon indicating copy to clipboard operation
panda copied to clipboard

Untangle config dependencies, misc prefactors

Open aubsw opened this issue 8 months ago • 2 comments

A pre-factor for https://github.com/commaai/panda/issues/2171 in the interest of reducing diff size.

Config files refactor

We split the board/config.h into the following headers:

  • include/board/stm_config.h: declarations + headers that depend on which STM microcontroller we're targetting.
  • include/board/platform_definitions.h: declarations ONLY. To be included in source files that need to know about STM-dependent definitions, but don't need to know about all existing headers (IWYU).
  • include/board/board_config.h: global value declarations

There will be additional de-spaghetti-ification in subsequent PRs.

Misc refactors

  • Add header guard to utils.h + some tweaks to ensure compatibility with opendebc (we must land this PR before https://github.com/commaai/opendbc/pull/2076)
  • Add declarations for gpio.h
  • Rework can_common declarations a bit

aubsw avatar Apr 03 '25 16:04 aubsw

why is this needed exactly? this just seems to increase complexity to me without any real benefits

can we keep #2175 to a minimal diff to just get everything building / working without shuffling too much code around. it's already a big enough change as it is

robbederks avatar Apr 09 '25 14:04 robbederks

why is this needed exactly?

I did it this way because when I first started working this, it was the only way to unambiguously break the circular dependency chain–otherwise it was difficult to comprehend why compilation was busted.

I disgree with you that this is adding complexity–I think that this is a strict reduction in complexity overall because it flattens the dependency heirarchy. You also need to keep track of less state when reading the code this way.

can we keep https://github.com/commaai/panda/pull/2175 to a minimal diff to just get everything building

Definitely a fair consideration.

At the very least we need to keep a dedicated header whose purpose is to plex on stm32h7xx.h, stm32h7xx_hal_gpio_ex.h, etc, because this is currently only exposed in config.h, which causes circular depedencies.

I think this is hopefully a fair compromise. Working on proving that this works seperately.

aubsw avatar Apr 09 '25 18:04 aubsw

Closing in favor of https://github.com/commaai/panda/pull/2185

aubsw avatar Apr 18 '25 14:04 aubsw