Skip to content

Untangle config dependencies, misc prefactors#2180

Closed
aubsw wants to merge 6 commits intocommaai:masterfrom
aubsw:leave-dependency-hell-config
Closed

Untangle config dependencies, misc prefactors#2180
aubsw wants to merge 6 commits intocommaai:masterfrom
aubsw:leave-dependency-hell-config

Conversation

@aubsw
Copy link
Copy Markdown
Contributor

@aubsw aubsw commented Apr 3, 2025

A pre-factor for #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

@aubsw aubsw force-pushed the leave-dependency-hell-config branch 3 times, most recently from edb0879 to f2508ad Compare April 3, 2025 17:29
@aubsw aubsw force-pushed the leave-dependency-hell-config branch from f2508ad to 596e505 Compare April 8, 2025 23:15
@aubsw aubsw marked this pull request as ready for review April 9, 2025 12:02
@aubsw aubsw changed the title [WIP] Untangle board_declarations deps Untangle config dependencies Apr 9, 2025
@aubsw aubsw changed the title Untangle config dependencies Untangle config dependencies, misc prefactors Apr 9, 2025
@robbederks
Copy link
Copy Markdown
Contributor

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

return (GPIO->IDR & (1UL << pin)) == (1UL << pin);
}

// cppcheck-suppress[misra-c2012-9.3,misra-c2012-8.7]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

try not to suppress any misra errors like these, let's fix them instead

@aubsw
Copy link
Copy Markdown
Contributor Author

aubsw commented Apr 9, 2025

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 #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 aubsw marked this pull request as draft April 9, 2025 18:58
@aubsw
Copy link
Copy Markdown
Contributor Author

aubsw commented Apr 18, 2025

Closing in favor of #2185

@aubsw aubsw closed this Apr 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants