New compile infrastructure by jorgesg82 · Pull Request #533 · Hyperloop-UPV/ST-LIB

jorgesg82 · 2025-12-01T22:50:16Z

Infraestructura de Domains y Board

Este PR introduce el nuevo sistema de domains y board en ST_LIB, diseñado para estructurar la configuración de periféricos de forma estática, segura y extensible.

Cambios principales

Añadido BuildCtx, encargado de recopilar y organizar todos los dispositivos por dominio en compile-time.
Añadido Board<...>, que:
- Genera configuraciones para cada dominio durante la compilación.
- Inicializa el hardware en tiempo de ejecución mediante init().
- Ofrece acceso a instancias reales usando instance_of<dev>().
Dominio GPIO migrado como implementación de referencia.

Ventajas del nuevo sistema

Validación completa en compile-time: detección de recursos duplicados, configuraciones inválidas o incompatibles.
Separación clara entre compile-time y runtime:
- build(): validación y generación de configuraciones.
- Init::init(): inicialización del hardware.

API simplificada para el usuario:

using MyBoard = Board<dev1, dev2, ...>;
MyBoard::init();
auto &inst = MyBoard::instance_of<dev>();

Extensibilidad limpia: crear un nuevo dominio solo requiere definir Entry, Device, Config, build(), Instance y Init.

Estado

La infraestructura está funcionando y el dominio GPIO sirve como plantilla para la migración del resto de periféricos.

Copilot

Pull request overview

This PR introduces a new compile-time infrastructure for ST_LIB that separates device configuration validation (compile-time) from hardware initialization (runtime). The system is built around a domain/board pattern where domains (like GPIO) define peripheral behaviors, and Board orchestrates compile-time validation and runtime initialization.

Key changes:

Added BuildCtx for compile-time device organization by domain
Added Board<...> template providing build() for validation, init() for hardware setup, and instance_of<dev>() for runtime access
Migrated GPIO as reference domain implementation demonstrating the new pattern

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 14 comments.

Show a summary per file

File	Description
Inc/ST-LIB.hpp	Implements core `BuildCtx` and `Board` templates for domain/board infrastructure
Inc/HALAL/Models/GPIO.hpp	Defines `GPIODomain` as reference implementation with Entry, Config, build(), Instance, and Init
Inc/HALAL/Models/Pin.hpp	Provides constexpr pin definitions for all STM32H7 GPIO ports (PA0-PH15)
Inc/HALAL/HALAL.hpp	Adds includes for new GPIO and Pin headers
CMakeLists.txt	Enables exceptions (required for compile-time error handling) and removes warning suppressions
Inc/stm32h7xx_hal_conf.h	Enables USE_FULL_ASSERT for debug validation
.vscode/settings.json	Updates C++ standard to C++23

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-01T22:56:11Z

+  # $<$<COMPILE_LANGUAGE:CXX>:-Wno-gnu-zero-variadic-macro-arguments>
+  # $<$<COMPILE_LANGUAGE:CXX>:-Wno-inconsistent-missing-override>


[nitpick] Warning suppressions -Wno-gnu-zero-variadic-macro-arguments and -Wno-inconsistent-missing-override have been commented out without explanation. If these warnings are no longer needed, the commented lines should be removed entirely. If they might be needed in the future, add a comment explaining why they're disabled and under what conditions they should be re-enabled.

Suggested change

# $<$<COMPILE_LANGUAGE:CXX>:-Wno-gnu-zero-variadic-macro-arguments>

# $<$<COMPILE_LANGUAGE:CXX>:-Wno-inconsistent-missing-override>

…Alternate functions

Copilot

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 11 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-02T23:34:03Z

+template <auto &...devs> struct Board {
+  static consteval auto build_ctx() {
+    DomainsCtx ctx{};
+    (devs.inscribe(ctx), ...);
+    return ctx;
+  }
+
+  static constexpr auto ctx = build_ctx();
+
+  template <typename D> static consteval std::size_t domain_size() {
+    return ctx.template span<D>().size();
+  }
+
+  static consteval auto build() {
+    constexpr std::size_t gpioN = domain_size<GPIODomain>();
+    constexpr std::size_t doutN = domain_size<DigitalOutputDomain>();
+    constexpr std::size_t dinN = domain_size<DigitalInputDomain>();
+    // ...
+
+    struct ConfigBundle {
+      std::array<GPIODomain::Config, gpioN> gpio_cfgs;
+      std::array<DigitalOutputDomain::Config, doutN> dout_cfgs;
+      std::array<DigitalInputDomain::Config, dinN> din_cfgs;
+      // ...
+    };
+
+    return ConfigBundle{
+        .gpio_cfgs =
+            GPIODomain::template build<gpioN>(ctx.template span<GPIODomain>()),
+        .dout_cfgs = DigitalOutputDomain::template build<doutN>(
+            ctx.template span<DigitalOutputDomain>()),
+        .din_cfgs = DigitalInputDomain::template build<dinN>(
+            ctx.template span<DigitalInputDomain>()),
+        // ...
+    };
+  }
+
+  static constexpr auto cfg = build();
+
+  static void init() {
+    constexpr std::size_t gpioN = domain_size<GPIODomain>();
+    constexpr std::size_t doutN = domain_size<DigitalOutputDomain>();
+    constexpr std::size_t dinN = domain_size<DigitalInputDomain>();
+    // ...
+
+    GPIODomain::Init<gpioN>::init(cfg.gpio_cfgs);
+    DigitalOutputDomain::Init<doutN>::init(cfg.dout_cfgs,
+                                           GPIODomain::Init<gpioN>::instances);
+    DigitalInputDomain::Init<dinN>::init(cfg.din_cfgs,
+                                         GPIODomain::Init<gpioN>::instances);
+    // ...
+  }
+
+  template <typename Domain>
+  static consteval std::size_t domain_size_for_instance() {
+    return domain_size<Domain>();
+  }
+
+  template <typename Domain, auto &Target, std::size_t I = 0>
+  static consteval std::size_t domain_index_of_impl() {
+    std::size_t idx = 0;
+    bool found = false;
+
+    (
+        [&] {
+          using DevT = std::remove_cvref_t<decltype(devs)>;
+          if constexpr (std::is_same_v<typename DevT::domain, Domain>) {
+            if (!found) {
+              if (&devs == &Target) {
+                found = true;
+              } else {
+                ++idx;
+              }
+            }
+          }
+        }(),
+        ...);
+
+    if (!found) {
+      struct device_not_found_for_domain {};
+      throw device_not_found_for_domain{};
+    }
+
+    return idx;
+  }
+
+  template <typename Domain, auto &Target>
+  static consteval std::size_t domain_index_of() {
+    return domain_index_of_impl<Domain, Target>();
+  }
+
+  template <auto &Target> static auto &instance_of() {
+    using DevT = std::remove_cvref_t<decltype(Target)>;
+    using Domain = typename DevT::domain;
+
+    constexpr std::size_t idx = domain_index_of<Domain, Target>();
+    constexpr std::size_t N = domain_size_for_instance<Domain>();
+
+    return Domain::template Init<N>::instances[idx];
+  }
+};


The new Board infrastructure and domain system lacks test coverage. Consider adding tests that verify: 1) compile-time validation (duplicate GPIO detection, invalid AF detection), 2) runtime initialization, and 3) instance retrieval with instance_of<>(). Other features in this codebase have test coverage (see Tests/DigitalOutTest.cpp, Tests/DigitalInTest.cpp).

Copilot · 2025-12-02T23:34:04Z

+    constexpr std::size_t I = domain_index<D>();
+    auto &arr = std::get<I>(storage);
+    auto &size = sizes[I];
+    const auto idx = size;


Missing bounds checking in add() method. If size reaches max_count_v<D>, the next arr[size++] = e will write out of bounds. Add a check like if (size >= max_count_v<D>) throw "Domain instance limit exceeded"; before line 54.

Suggested change

const auto idx = size;

const auto idx = size;

if (size >= max_count_v<D>) throw "Domain instance limit exceeded";

Copilot · 2025-12-02T23:34:04Z

+    case Pull::Up:
+      return GPIO_PULLUP;
+    case Pull::Down:
+      return GPIO_PULLDOWN;


Missing return statement or default case in switch. If an invalid Pull value is passed, this function has undefined behavior. Consider adding a default case that triggers a compile-time error or returns a safe default value.

Suggested change

return GPIO_PULLDOWN;

return GPIO_PULLDOWN;

default:

#if defined(__GNUC__) || defined(__clang__)

__builtin_unreachable();

#else

// Fallback: return a safe default or terminate

return GPIO_NOPULL;

#endif

Copilot · 2025-12-02T23:34:04Z

+    NO_AF = 20,
+    AF0 = 15,
+    AF1 = 14,
+    AF2 = 13,
+    AF3 = 12,
+    AF4 = 11,
+    AF5 = 10,
+    AF6 = 9,
+    AF7 = 8,
+    AF8 = 7,
+    AF9 = 6,
+    AF10 = 5,
+    AF11 = 4,
+    AF12 = 3,
+    AF13 = 2,
+    AF14 = 1,
+    AF15 = 0


The AlternateFunction enum values appear inverted (AF0=15, AF15=0), which is unconventional and confusing. Consider adding a comment explaining this encoding scheme, or if this is a mistake, the values should be corrected to match the naming (AF0=0, AF15=15).

Suggested change

NO_AF = 20,

AF0 = 15,

AF1 = 14,

AF2 = 13,

AF3 = 12,

AF4 = 11,

AF5 = 10,

AF6 = 9,

AF7 = 8,

AF8 = 7,

AF9 = 6,

AF10 = 5,

AF11 = 4,

AF12 = 3,

AF13 = 2,

AF14 = 1,

AF15 = 0

NO_AF = 16,

AF0 = 0,

AF1 = 1,

AF2 = 2,

AF3 = 3,

AF4 = 4,

AF5 = 5,

AF6 = 6,

AF7 = 7,

AF8 = 8,

AF9 = 9,

AF10 = 10,

AF11 = 11,

AF12 = 12,

AF13 = 13,

AF14 = 14,

AF15 = 15

Copilot · 2025-12-02T23:34:04Z

+template <typename... Domains> struct BuildCtx {
+  template <typename D> using Decl = typename D::Entry;
+  template <typename D>
+  static constexpr std::size_t max_count_v = D::max_instances;
+
+  std::tuple<std::array<Decl<Domains>, max_count_v<Domains>>...> storage{};
+  std::array<std::size_t, sizeof...(Domains)> sizes{};
+
+  template <typename D, std::size_t I = 0>
+  static consteval std::size_t domain_index() {
+    if constexpr (I >= sizeof...(Domains)) {
+      static_assert([] { return false; }(), "Domain not found");
+      return 0;
+    } else if constexpr (std::is_same_v<D, std::tuple_element_t<
+                                               I, std::tuple<Domains...>>>) {
+      return I;
+    } else {
+      return domain_index<D, I + 1>();
+    }
+  }
+
+  template <typename D> consteval std::size_t add(typename D::Entry e) {
+    constexpr std::size_t I = domain_index<D>();
+    auto &arr = std::get<I>(storage);
+    auto &size = sizes[I];
+    const auto idx = size;
+    arr[size++] = e;
+    return idx;
+  }
+
+  template <typename D> consteval auto span() const {
+    constexpr std::size_t I = domain_index<D>();
+    auto const &arr = std::get<I>(storage);
+    auto const size = sizes[I];
+    using E = typename D::Entry;
+    return std::span<const E>{arr.data(), size};
+  }
+
+  template <typename D> consteval std::size_t size() const {
+    constexpr std::size_t I = domain_index<D>();
+    return sizes[I];
+  }
+};


Missing documentation for the BuildCtx template struct, which is a core part of the new infrastructure. Consider adding a comment block explaining its purpose (compile-time context for collecting device configurations), usage, and template parameters.

Copilot · 2025-12-02T23:34:05Z

@@ -0,0 +1,151 @@
+#include "HALAL/Models/GPIO.hpp"


Missing #pragma once include guard. This file should start with #pragma once to prevent multiple inclusion issues.

Copilot · 2025-12-02T23:34:05Z

+  static consteval array<Config, N> build(span<const Entry> outputs) {
+    array<Config, N> cfgs{};
+
+    for (std::size_t i = 0; i < N; ++i) {
+      cfgs[i].gpio_idx = outputs[i].gpio_idx;


Parameter name outputs is misleading for a DigitalInput domain. Consider renaming to inputs for clarity and consistency.

Suggested change

static consteval array<Config, N> build(span<const Entry> outputs) {

array<Config, N> cfgs{};

for (std::size_t i = 0; i < N; ++i) {

cfgs[i].gpio_idx = outputs[i].gpio_idx;

static consteval array<Config, N> build(span<const Entry> inputs) {

array<Config, N> cfgs{};

for (std::size_t i = 0; i < N; ++i) {

cfgs[i].gpio_idx = inputs[i].gpio_idx;

Copilot · 2025-12-02T23:34:05Z

+  static constexpr uint32_t to_hal_mode(OperationMode m) {
+    switch (m) {
+    case OperationMode::INPUT:
+      return GPIO_MODE_INPUT;
+    case OperationMode::OUTPUT_PUSHPULL:
+      return GPIO_MODE_OUTPUT_PP;
+    case OperationMode::OUTPUT_OPENDRAIN:
+      return GPIO_MODE_OUTPUT_OD;
+    case OperationMode::ANALOG:
+      return GPIO_MODE_ANALOG;
+    case OperationMode::ALT_PP:
+      return GPIO_MODE_AF_PP;
+    case OperationMode::ALT_OD:
+      return GPIO_MODE_AF_OD;
+    case OperationMode::EXTI_RISING:
+      return GPIO_MODE_IT_RISING;
+    case OperationMode::EXTI_FALLING:
+      return GPIO_MODE_IT_FALLING;
+    case OperationMode::EXTI_RISING_FALLING:
+      return GPIO_MODE_IT_RISING_FALLING;
+    }
+  }


Missing return statement or default case in switch. If an invalid OperationMode value is passed, this function has undefined behavior. Consider adding a default case that triggers a compile-time error or returns a safe default value.

Copilot · 2025-12-02T23:34:06Z

+    case Speed::High:
+      return GPIO_SPEED_FREQ_HIGH;
+    case Speed::VeryHigh:
+      return GPIO_SPEED_FREQ_VERY_HIGH;


Missing return statement or default case in switch. If an invalid Speed value is passed, this function has undefined behavior. Consider adding a default case that triggers a compile-time error or returns a safe default value.

Suggested change

return GPIO_SPEED_FREQ_VERY_HIGH;

return GPIO_SPEED_FREQ_VERY_HIGH;

default:

// Defensive: return a safe default or trigger a compile-time error

return GPIO_SPEED_FREQ_LOW;

jorgesg82 added 3 commits November 30, 2025 13:57

Initial structure

e2af8c5

no need to have an id for instance

3eec305

erased no longer needed code

fa3498b

jorgesg82 self-assigned this Dec 1, 2025

Copilot AI review requested due to automatic review settings December 1, 2025 22:50

Copilot started reviewing on behalf of jorgesg82 December 1, 2025 22:50 View session

Copilot finished reviewing on behalf of jorgesg82 December 1, 2025 22:53

Copilot AI reviewed Dec 1, 2025

View reviewed changes

Added DigitalInput and DigitalOutput Services, and added support for …

c92a738

…Alternate functions

jorgesg82 requested a review from Copilot December 2, 2025 23:24

Copilot started reviewing on behalf of jorgesg82 December 2, 2025 23:25 View session

Copilot finished reviewing on behalf of jorgesg82 December 2, 2025 23:31

Copilot AI reviewed Dec 2, 2025

View reviewed changes

jorgesg82 added 4 commits December 8, 2025 12:11

Replaced throw by undefined function

9dc516b

Implemented alternate functions

d9263fe

Added linker script and startup code, and fixed using hal template conf

4cd2796

Added initial GPIO mock

c4748dd

jorgesg82 marked this pull request as draft December 12, 2025 18:08

jorgesg82 added 2 commits December 14, 2025 12:08

added pragma once on pines file

a0489c4

Remove mock infrastructure (still wip)

11d77a0

jorgesg82 marked this pull request as ready for review December 14, 2025 11:12

jorgesg82 merged commit 6da5fad into development Dec 14, 2025
12 checks passed

		# $<$<COMPILE_LANGUAGE:CXX>:-Wno-gnu-zero-variadic-macro-arguments>
		# $<$<COMPILE_LANGUAGE:CXX>:-Wno-inconsistent-missing-override>

	const auto idx = size;
	const auto idx = size;
	if (size >= max_count_v<D>) throw "Domain instance limit exceeded";

-      return GPIO_PULLDOWN;
+      return GPIO_PULLDOWN;
+    default:
+#if defined(__GNUC__) || defined(__clang__)
+      __builtin_unreachable();
+#else
+      // Fallback: return a safe default or terminate
+      return GPIO_NOPULL;
+#endif

Conversation

jorgesg82 commented Dec 1, 2025

Infraestructura de Domains y Board

Cambios principales

Ventajas del nuevo sistema

Estado

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants