From ef9c2f53411bb5d391804943a855909db0d79b63 Mon Sep 17 00:00:00 2001 From: Nehal Patel Date: Fri, 4 Sep 2026 05:10:25 +0000 Subject: [PATCH 01/12] build: make the platform-backend contract fit a second backend Every seam below board::Board has had exactly one implementation, so the contracts a backend must satisfy were never written down and the parts that are host-specific were never separated from the parts that are not. Each of these bites on the first cross build, none of them are about ARM code: - clang-tidy ran on cross builds and could not parse them at all. clang 18 defines __cpp_concepts as 201907L; libstdc++-13's guards its contents on >= 202002L, so every translation unit reports "no template named 'expected'" and src/.clang-tidy makes warnings errors. Gate it on the host build. The fix is a newer host clang, noted where it applies. - The "Available: host" in both not-implemented messages was hand-written and already wrong: stm32f3_discovery has been a visible preset for a while. Derive the list from the directories that exist. - A cross MCU backend with EMBEDDED_CPP_BOARD still 'host' pulled in host_board, and with it the cppzmq the dependency section deliberately does not fetch when cross-compiling. Fail where the cause is legible. - -fno-exceptions is claimed in CLAUDE.md and was set nowhere. It cannot be project-wide -- the host entry point is an exception boundary by design, because cppzmq throws -- so scope it, with -fno-threadsafe-statics, to cross builds. - The apps linked a target named literally `sys`, defined only in the host board directory and documented nowhere. Rename it platform_entry and write the contract down: a board backend provides main() under that name. Also: nosys.specs so a board links before it has written its syscalls (_sbrk is not optional even with no allocation -- a vtable references its deleting destructor, which references operator delete), a .elf suffix, nucleo-f767zi presets, and the flashing tools in the dev image. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 28 +++++++++++++--- CMakeLists.txt | 53 ++++++++++++++++++++++++++++-- CMakePresets.json | 51 ++++++++++++++++++++++++++++ Dockerfile | 13 +++++++- cmake/backends.cmake | 25 ++++++++++++++ cmake/toolchain/armgcc.cmake | 12 ++++++- src/apps/blinky/CMakeLists.txt | 2 +- src/apps/i2c_demo/CMakeLists.txt | 2 +- src/apps/uart_echo/CMakeLists.txt | 2 +- src/libs/board/CMakeLists.txt | 4 ++- src/libs/board/host/CMakeLists.txt | 15 ++++++--- src/libs/common/logger.hpp | 8 ++++- src/libs/mcu/CMakeLists.txt | 5 ++- 13 files changed, 201 insertions(+), 19 deletions(-) create mode 100644 cmake/backends.cmake diff --git a/CLAUDE.md b/CLAUDE.md index e644b93..54219dc 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -39,10 +39,14 @@ cmake --build build/host --target format-check # Python type-check (not covered by format.sh - types are not formatting) cd py/host-emulator && uv run mypy -# Cross-compile for ARM - not yet functional. Toolchain files and configure -# presets exist, but only the `host` MCU/board implementations do; configuring -# an ARM preset stops with a message saying the backend is not implemented. -# Host build and emulation come first; hardware follows. +# Cross-compile for the STM32F767ZI Nucleo (Cortex-M7). Configure + build only: +# firmware has no tests that run on the build machine, so there is no test step +# and no workflow preset runs ctest. CI verifies the image with readelf/nm. +cmake --workflow --preset=nucleo-f767zi-debug +cmake --workflow --preset=nucleo-f767zi-release + +# Other ARM presets are toolchain-only: no arm_cm4 backend or stm32f3_discovery +# board exists yet, so configuring stops with a message naming what does. cmake --preset=stm32f3_discovery # Docker alternative @@ -59,6 +63,22 @@ Application (apps/) → Board (libs/board/) → MCU (libs/mcu/) → Platfo **Host emulation**: C++ apps communicate with Python hardware emulator via ZeroMQ/JSON IPC. This enables desktop development and integration testing without hardware. +**Platform backend contract**. `EMBEDDED_CPP_MCU` and `EMBEDDED_CPP_BOARD` each +select a sibling directory (`src/libs/mcu/`, `src/libs/board/`); an +unknown name stops the configure with the list of directories that do exist. +A board backend must define a target named **`platform_entry`** — an OBJECT +library providing `main()` and calling `app::AppMain(board::Board&)`. The apps +link it by name (`src/apps/*/CMakeLists.txt`) and know nothing else about the +platform. OBJECT rather than a static archive because pulling `main()` (or a +vector table) out of an archive depends on link-order symbol resolution and +breaks under `--gc-sections`/LTO. + +Cross builds differ from the host build in three ways worth knowing: they add +`-fno-exceptions -fno-threadsafe-statics` (the host entry point is an exception +boundary by design, since cppzmq throws), they skip cppzmq/JSON/googletest and +the Python emulator, and they run without clang-tidy — clang 18 cannot parse +libstdc++-13's ``, so the fix is a newer host clang, not a workaround. + ## Key Constraints - **No exceptions** - RTTI disabled; use `std::expected` for all fallible operations diff --git a/CMakeLists.txt b/CMakeLists.txt index 34aafff..c0690d7 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -23,7 +23,20 @@ set(EMBEDDED_CPP_MCU "host" CACHE STRING set_property(CACHE EMBEDDED_CPP_MCU PROPERTY STRINGS host arm_cm4 arm_cm7) set(EMBEDDED_CPP_BOARD "host" CACHE STRING "Board implementation to build (selects src/libs/board/)") -set_property(CACHE EMBEDDED_CPP_BOARD PROPERTY STRINGS host stm32f3_discovery) +set_property(CACHE EMBEDDED_CPP_BOARD PROPERTY STRINGS + host stm32f3_discovery stm32f767zi_nucleo) + +# A hardware MCU backend with the host board would pull in host_board, and with +# it cppzmq and nlohmann-json -- which the dependency section below deliberately +# does not fetch when cross-compiling. Catch the mismatch here, where the cause +# is obvious, rather than as a missing-target error hundreds of lines later. +if(NOT EMBEDDED_CPP_MCU STREQUAL "host" AND EMBEDDED_CPP_BOARD STREQUAL "host") + message(FATAL_ERROR + "EMBEDDED_CPP_MCU='${EMBEDDED_CPP_MCU}' needs a matching hardware board, " + "but EMBEDDED_CPP_BOARD is still 'host'. The host board is built on " + "ZeroMQ/JSON emulation and has no hardware equivalent. Select a board " + "with -DEMBEDDED_CPP_BOARD=, or use a preset that sets both.") +endif() # One bin/ directory per build tree (with per-config subdirectories under the # multi-config generator). A build-layout decision, so it lives here rather @@ -63,6 +76,25 @@ target_include_directories(project_options INTERFACE $) target_compile_options(project_options INTERFACE $<$:-fno-rtti>) + +# The no-exceptions policy is real on hardware and cannot be project-wide: the +# host entry point (src/libs/board/host/main.cpp) is an exception boundary by +# design, because cppzmq and nlohmann-json throw. On a cross build nothing +# links either, so -fno-exceptions costs nothing and buys the absence of +# unwind tables and landing pads. std::expected degrades accordingly -- +# .value() on an error aborts instead of throwing bad_expected_access, which +# is the better failure mode on a target with no way to service an unwind. +# +# -fno-threadsafe-statics drops the __cxa_guard_acquire/release pair around +# every function-local static. Correct on a single-core target with no RTOS. +# Revisit if FreeRTOS lands (docs/PROJECT_PLAN.md Milestone 4): with two tasks +# racing through the same function-local static, this flag turns a handled +# case into a real data race. +if(CMAKE_CROSSCOMPILING) + target_compile_options(project_options INTERFACE + $<$:-fno-exceptions;-fno-threadsafe-statics>) +endif() + target_link_libraries(project_options INTERFACE project_warnings) # --------------------------------------------------------------------------- @@ -139,11 +171,28 @@ endif() # developer should get the same guard rails. include("${PROJECT_SOURCE_DIR}/cmake/format.cmake") +# implemented_backends(): used by the mcu and board layers to report which +# platform implementations exist when an unknown one is requested. +include("${PROJECT_SOURCE_DIR}/cmake/backends.cmake") + # --------------------------------------------------------------------------- # Targets # --------------------------------------------------------------------------- -clang_tidy("-header-filter=${PROJECT_SOURCE_DIR}/src/.*") +# clang-tidy runs on host builds only. On a cross build it fails before seeing +# any project code: clang 18 defines __cpp_concepts as 201907L, while +# libstdc++-13's guards its contents on __cpp_concepts >= 202002L, +# so every translation unit reports "no template named 'expected' in namespace +# 'std'" -- and src/.clang-tidy sets WarningsAsErrors: "*". +# +# This is a clang limitation, not a cross-compilation one. The real fix is to +# raise the host clang pin (cmake/toolchain/host-clang.cmake) to 19 or newer, +# which defines __cpp_concepts correctly; the dev image currently ships only +# clang-18. Until then the analysis runs against the host build, which compiles +# the same portable headers, and cross builds keep clang-format only. +if(NOT CMAKE_CROSSCOMPILING) + clang_tidy("-header-filter=${PROJECT_SOURCE_DIR}/src/.*") +endif() add_subdirectory(src) reset_clang_tidy() diff --git a/CMakePresets.json b/CMakePresets.json index e05f63f..e488794 100644 --- a/CMakePresets.json +++ b/CMakePresets.json @@ -64,6 +64,17 @@ "cacheVariables": { "EMBEDDED_CPP_BOARD": "stm32f3_discovery" } + }, + { + "name": "nucleo-f767zi", + "displayName": "STM32F767ZI Nucleo", + "description": "Cross-compile for the STM32F767ZI Nucleo (Cortex-M7)", + "inherits": [ + "arm-cm7" + ], + "cacheVariables": { + "EMBEDDED_CPP_BOARD": "stm32f767zi_nucleo" + } } ], "buildPresets": [ @@ -101,6 +112,18 @@ "description": "Host build (RelWithDebInfo) using Ninja", "inherits": "host", "configuration": "RelWithDebInfo" + }, + { + "name": "nucleo-f767zi-debug", + "displayName": "STM32F767ZI Nucleo (Debug)", + "configurePreset": "nucleo-f767zi", + "configuration": "Debug" + }, + { + "name": "nucleo-f767zi-release", + "displayName": "STM32F767ZI Nucleo (Release)", + "configurePreset": "nucleo-f767zi", + "configuration": "Release" } ], "testPresets": [ @@ -203,6 +226,34 @@ "name": "host-relwithdebinfo" } ] + }, + { + "name": "nucleo-f767zi-debug", + "displayName": "STM32F767ZI Nucleo Debug (configure + build)", + "steps": [ + { + "type": "configure", + "name": "nucleo-f767zi" + }, + { + "type": "build", + "name": "nucleo-f767zi-debug" + } + ] + }, + { + "name": "nucleo-f767zi-release", + "displayName": "STM32F767ZI Nucleo Release (configure + build)", + "steps": [ + { + "type": "configure", + "name": "nucleo-f767zi" + }, + { + "type": "build", + "name": "nucleo-f767zi-release" + } + ] } ] } diff --git a/Dockerfile b/Dockerfile index 601a87c..790f25a 100644 --- a/Dockerfile +++ b/Dockerfile @@ -27,9 +27,20 @@ RUN apt-get update && apt-get --no-install-recommends -y full-upgrade && apt-get # Additional tools libzmq3-dev \ unzip \ - # ARM GCC toolchain + # ARM GCC toolchain. Pinned by the base image at 13.2.rel1, which compiles + # every portable header and app source at -std=c++23. libstdc++ 13 has no + # , but the only std::println calls live in host-only translation + # units. Staying on the distro package keeps a contributor's local + # toolchain byte-identical to CI's. gcc-arm-none-eabi \ binutils-arm-none-eabi \ + # Flashing and on-chip debugging. Note these are for use from the host OS + # in the usual devcontainer setup: reaching an ST-LINK from inside the + # container needs USB passthrough that is awkward on Linux and effectively + # unavailable on macOS/Windows. The F767ZI's USB mass-storage interface + # needs no tooling at all -- copy the .bin to the NODE_F767ZI volume. + openocd \ + stlink-tools \ gdb \ gdb-multiarch \ neovim \ diff --git a/cmake/backends.cmake b/cmake/backends.cmake new file mode 100644 index 0000000..eab9895 --- /dev/null +++ b/cmake/backends.cmake @@ -0,0 +1,25 @@ +# Platform-backend discovery. +# +# The MCU and board layers each select an implementation by directory name +# (EMBEDDED_CPP_MCU / EMBEDDED_CPP_BOARD). When the requested name has no +# directory, the error message should say what *is* available -- and say it +# correctly. A hand-written list drifts: it named only "host" long after +# stm32f3_discovery became a visible preset. + +# Set to a comma-separated, sorted list of the implementation +# directories under . A directory counts as an implementation only if it +# has a CMakeLists.txt, so a stray or half-deleted directory is not advertised +# as a working backend. +function(implemented_backends out_var dir) + file(GLOB entries LIST_DIRECTORIES true "${dir}/*") + set(found "") + foreach(entry IN LISTS entries) + if(IS_DIRECTORY "${entry}" AND EXISTS "${entry}/CMakeLists.txt") + cmake_path(GET entry FILENAME name) + list(APPEND found "${name}") + endif() + endforeach() + list(SORT found) + list(JOIN found ", " joined) + set(${out_var} "${joined}" PARENT_SCOPE) +endfunction() diff --git a/cmake/toolchain/armgcc.cmake b/cmake/toolchain/armgcc.cmake index 271b927..d9a0370 100644 --- a/cmake/toolchain/armgcc.cmake +++ b/cmake/toolchain/armgcc.cmake @@ -44,7 +44,17 @@ set(CMAKE_ASM_OPTIONS "-x assembler-with-cpp") set(CMAKE_C_FLAGS_INIT "${CMAKE_COMMON_FLAGS}") set(CMAKE_CXX_FLAGS_INIT "${CMAKE_COMMON_FLAGS}") set(CMAKE_ASM_FLAGS_INIT "${CMAKE_COMMON_FLAGS} ${CMAKE_ASM_OPTIONS}") -set(CMAKE_EXE_LINKER_FLAGS_INIT "--specs=nano.specs -Wl,--gc-sections,-print-memory-usage,--no-warn-rwx-segments") +# nano.specs selects newlib-nano; nosys.specs supplies stub implementations of +# the syscalls it expects (_sbrk, _write, _close, ...) so a board backend links +# before it has written its own. Note _sbrk is not optional even in a design +# that never calls new: a polymorphic class's vtable references its deleting +# destructor, which references operator delete. A board replaces nosys with a +# real syscalls translation unit once it has a UART to write to. +set(CMAKE_EXE_LINKER_FLAGS_INIT "--specs=nano.specs --specs=nosys.specs -Wl,--gc-sections,-print-memory-usage,--no-warn-rwx-segments") + +# Firmware images, not host executables. Makes blinky.elf and blinky.bin +# unambiguous in the build tree and in flashing instructions. +set(CMAKE_EXECUTABLE_SUFFIX ".elf") set(CMAKE_C_FLAGS_DEBUG_INIT "-O0") set(CMAKE_CXX_FLAGS_DEBUG_INIT "-O0") diff --git a/src/apps/blinky/CMakeLists.txt b/src/apps/blinky/CMakeLists.txt index 0f6df8a..2063dc7 100644 --- a/src/apps/blinky/CMakeLists.txt +++ b/src/apps/blinky/CMakeLists.txt @@ -1,2 +1,2 @@ add_executable(blinky blinky.cpp) -target_link_libraries(blinky PRIVATE project_options app sys mcu error) +target_link_libraries(blinky PRIVATE project_options app platform_entry mcu error) diff --git a/src/apps/i2c_demo/CMakeLists.txt b/src/apps/i2c_demo/CMakeLists.txt index 4024ed0..a7006f0 100644 --- a/src/apps/i2c_demo/CMakeLists.txt +++ b/src/apps/i2c_demo/CMakeLists.txt @@ -1,2 +1,2 @@ add_executable(i2c_demo i2c_demo.cpp) -target_link_libraries(i2c_demo PRIVATE project_options app sys mcu error) +target_link_libraries(i2c_demo PRIVATE project_options app platform_entry mcu error) diff --git a/src/apps/uart_echo/CMakeLists.txt b/src/apps/uart_echo/CMakeLists.txt index c9e4adf..d2e28e4 100644 --- a/src/apps/uart_echo/CMakeLists.txt +++ b/src/apps/uart_echo/CMakeLists.txt @@ -1,2 +1,2 @@ add_executable(uart_echo uart_echo.cpp) -target_link_libraries(uart_echo PRIVATE project_options app sys mcu error) +target_link_libraries(uart_echo PRIVATE project_options app platform_entry mcu error) diff --git a/src/libs/board/CMakeLists.txt b/src/libs/board/CMakeLists.txt index 7dceef9..d720fb9 100644 --- a/src/libs/board/CMakeLists.txt +++ b/src/libs/board/CMakeLists.txt @@ -5,9 +5,11 @@ target_sources(board INTERFACE FILES board.hpp) target_link_libraries(board INTERFACE mcu error) +# See the note in src/libs/mcu/CMakeLists.txt: the list is derived, not written. if(NOT IS_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}/${EMBEDDED_CPP_BOARD}") + implemented_backends(available "${CMAKE_CURRENT_SOURCE_DIR}") message(FATAL_ERROR "Board '${EMBEDDED_CPP_BOARD}' is not implemented yet. " - "Available: host. Hardware boards will be added later; see CLAUDE.md.") + "Available: ${available}. See CLAUDE.md.") endif() add_subdirectory(${EMBEDDED_CPP_BOARD}) diff --git a/src/libs/board/host/CMakeLists.txt b/src/libs/board/host/CMakeLists.txt index ec2f7b8..794c91f 100644 --- a/src/libs/board/host/CMakeLists.txt +++ b/src/libs/board/host/CMakeLists.txt @@ -7,8 +7,13 @@ target_link_libraries(host_board PUBLIC board mcu host_mcu PRIVATE project_options cppzmq nlohmann_json::nlohmann_json) -# The platform entry point. An OBJECT library so main.o is linked into each -# app directly — pulling main() out of a static archive relies on link-time -# symbol-resolution order and breaks under --gc-sections/LTO. -add_library(sys OBJECT main.cpp) -target_link_libraries(sys PRIVATE project_options app board host_board) +# The platform entry point, and the contract every board backend must satisfy: +# a target named `platform_entry` that provides main() and calls app::AppMain. +# The apps link it by name and know nothing else about the platform. +# +# An OBJECT library so main.o is linked into each app directly — pulling main() +# out of a static archive relies on link-time symbol-resolution order and +# breaks under --gc-sections/LTO. A hardware backend adds its startup code, +# vector table and syscalls here for the same reason. +add_library(platform_entry OBJECT main.cpp) +target_link_libraries(platform_entry PRIVATE project_options app board host_board) diff --git a/src/libs/common/logger.hpp b/src/libs/common/logger.hpp index 009d752..9732cb7 100644 --- a/src/libs/common/logger.hpp +++ b/src/libs/common/logger.hpp @@ -18,7 +18,13 @@ class Logger { virtual auto Error(std::string_view msg) -> void = 0; }; -// Null logger - discards all messages (default for embedded) +// Null logger - discards all messages (default for embedded). +// +// This is the only Logger a cross build can use. ConsoleLogger's out-of-line +// definitions (logger.cpp) are written against , which libstdc++ 13 +// does not provide -- and arm-none-eabi-g++ is 13.2. A hardware target that +// wants real output should retarget newlib's _write to a UART instead of +// reaching for std::println here. class NullLogger : public Logger { public: auto Debug(std::string_view /* msg */) -> void override {} diff --git a/src/libs/mcu/CMakeLists.txt b/src/libs/mcu/CMakeLists.txt index 1bb65e9..a8f3de8 100644 --- a/src/libs/mcu/CMakeLists.txt +++ b/src/libs/mcu/CMakeLists.txt @@ -5,9 +5,12 @@ target_sources(mcu INTERFACE FILES pin.hpp uart.hpp i2c.hpp delay.hpp) target_link_libraries(mcu INTERFACE error) +# The list of backends is derived from the directories that actually exist, so +# the message cannot drift out of date as backends are added or removed. if(NOT IS_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}/${EMBEDDED_CPP_MCU}") + implemented_backends(available "${CMAKE_CURRENT_SOURCE_DIR}") message(FATAL_ERROR "MCU backend '${EMBEDDED_CPP_MCU}' is not implemented yet. " - "Available: host. Hardware backends will be added later; see CLAUDE.md.") + "Available: ${available}. See CLAUDE.md.") endif() add_subdirectory(${EMBEDDED_CPP_MCU}) From e500025970801c1856caf6a4c03acd88ca3d6203 Mon Sep 17 00:00:00 2001 From: Nehal Patel Date: Fri, 4 Sep 2026 05:10:25 +0000 Subject: [PATCH 02/12] fix(emulator): answer unroutable messages instead of dying on them _dispatch raises UnhandledMessageError for an object type that is not in the routing table, and for a name that is not among its candidates. Both escaped the serve loop, were swallowed by its `except Exception`, and took the emulator thread with them. The device, blocked in Transport::Receive on a PAIR socket, then waited out its timeout with nothing coming: a forgotten registry entry showed up as a hung test, never as an error. An unroutable message is a protocol error, not a fatal one. Log it and reply Unhandled, which Transact() folds into the error channel as common::Error::kUnhandled -- the device gets a real error at the call that caused it. The reply carries the union of the fields the three response structs deserialize, so it decodes cleanly whichever one the caller expects. Only requests are answered. A Response is the tail of an exchange the emulator itself started; replying would leave an extra frame on the socket and desynchronize every exchange after it. All four new tests fail without the fix. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/host_emulator/emulator.py | 46 ++++++- .../tests/test_unroutable_message.py | 125 ++++++++++++++++++ 2 files changed, 169 insertions(+), 2 deletions(-) create mode 100644 py/host-emulator/tests/test_unroutable_message.py diff --git a/py/host-emulator/src/host_emulator/emulator.py b/py/host-emulator/src/host_emulator/emulator.py index 88b02a4..a106f9d 100755 --- a/py/host-emulator/src/host_emulator/emulator.py +++ b/py/host-emulator/src/host_emulator/emulator.py @@ -8,7 +8,7 @@ import zmq -from .common import UnhandledMessageError +from .common import MessageType, Status, UnhandledMessageError from .endpoint import EndpointLock, has_live_owner from .i2c import I2C from .peripheral import Peripheral @@ -162,7 +162,10 @@ def run(self) -> None: logger.warning("Received non-JSON message: %s", message) continue - self._dispatch(json_message) + try: + self._dispatch(json_message) + except UnhandledMessageError as exc: + self._reject(json_message, exc) except Exception: logger.exception("Emulator thread error") @@ -188,6 +191,45 @@ def _dispatch(self, json_message: dict[str, Any]) -> None: f"{object_type} not found: {json_message.get('name')}" ) + def _reject(self, json_message: dict[str, Any], exc: Exception) -> None: + """Answer a message no peripheral owns, and keep serving. + + An unroutable message is a protocol error, not a fatal one: it says the + device knows about a peripheral this emulator was not configured with, + which is a bug in one registry or the other. Letting it propagate would + kill the serve thread, and the device -- blocked in Transport::Receive + on a PAIR socket -- would then hang until its timeout with no clue why. + So log it and reply with Unhandled, which the C++ Transact() folds + straight into the error channel as common::Error::kUnhandled. + + Only requests get a reply. A Response is the tail of an exchange the + emulator itself started; answering one would leave an extra frame on + the socket and desynchronize every exchange after it. + """ + logger.error("Unhandled message: %s", exc) + + if json_message.get("type") != MessageType.Request: + return + + # The union of the fields the three response structs deserialize, so + # this decodes cleanly whichever one the device is expecting. Values + # other than status are placeholders: Transact() rejects on a non-Ok + # status before the caller ever sees them. + self.from_device_socket.send_string( + json.dumps( + { + "type": MessageType.Response, + "object": json_message.get("object"), + "name": json_message.get("name"), + "state": PinState.Hi_Z, + "address": json_message.get("address", 0), + "data": [], + "bytes_transferred": 0, + "status": Status.Unhandled, + } + ) + ) + def start(self) -> None: """Start the emulator, raising if it could not claim its endpoint. diff --git a/py/host-emulator/tests/test_unroutable_message.py b/py/host-emulator/tests/test_unroutable_message.py new file mode 100644 index 0000000..4f7a60e --- /dev/null +++ b/py/host-emulator/tests/test_unroutable_message.py @@ -0,0 +1,125 @@ +"""The emulator must survive a message no peripheral owns. + +``_dispatch`` raises ``UnhandledMessageError`` for an object type that is not +in the registry, and for a name that is. Both used to escape the serve loop, +be swallowed by its ``except Exception``, and take the emulator thread with +them -- leaving the device blocked in ``Transport::Receive`` on a PAIR socket +with nothing coming. The symptom was a hung test, never an error. + +These are pure emulator tests -- no application binary -- so they run without +any of the --blinky/--uart-echo/--i2c-demo options. +""" + +import json +from collections.abc import Generator +from pathlib import Path +from typing import Any + +import pytest +import zmq + +from host_emulator import DeviceEmulator +from host_emulator.common import MessageType, ObjectType, Operation, Status +from host_emulator.endpoint import endpoint_path +from host_emulator.pin import PinState + +RECV_TIMEOUT_MS = 2000 + + +@pytest.fixture +def standalone_emulator(tmp_path: Path) -> Generator[DeviceEmulator]: + """A private emulator on its own endpoints, stopped afterwards.""" + from_device = f"ipc://{tmp_path}/from_device.ipc" + to_device = f"ipc://{tmp_path}/to_device.ipc" + device_emulator = DeviceEmulator(from_device, to_device) + device_emulator.start() + try: + yield device_emulator + finally: + if device_emulator.running: + device_emulator.stop() + for endpoint in (from_device, to_device): + path = endpoint_path(endpoint) + if path is not None: + path.unlink(missing_ok=True) + path.with_name(path.name + ".lock").unlink(missing_ok=True) + + +@pytest.fixture +def device(standalone_emulator: DeviceEmulator) -> Generator[zmq.Socket[bytes]]: + """A PAIR socket standing in for the C++ device side.""" + context = zmq.Context.instance() + socket: zmq.Socket[bytes] = context.socket(zmq.PAIR) + socket.setsockopt(zmq.RCVTIMEO, RECV_TIMEOUT_MS) + socket.setsockopt(zmq.LINGER, 0) + socket.connect(standalone_emulator.from_device_endpoint) + try: + yield socket + finally: + socket.close() + + +def _request(**overrides: Any) -> str: + """A well-formed pin request, with fields overridden per test.""" + request: dict[str, Any] = { + "type": MessageType.Request, + "object": ObjectType.Pin, + "name": "LED 1", + "operation": Operation.Get, + "state": PinState.Low, + } + request.update(overrides) + return json.dumps(request) + + +@pytest.mark.parametrize( + ("overrides", "case"), + [ + ({"object": "Spi"}, "unregistered object type"), + ({"name": "no_such_pin"}, "unregistered name"), + ], +) +def test_unroutable_request_is_answered_not_dropped( + device: zmq.Socket[bytes], overrides: dict[str, Any], case: str +) -> None: + """An unroutable request gets an Unhandled response instead of silence.""" + device.send_string(_request(**overrides)) + + response = json.loads(device.recv()) + + assert response["status"] == Status.Unhandled, case + assert response["type"] == MessageType.Response + + +def test_emulator_still_serves_after_an_unroutable_request( + standalone_emulator: DeviceEmulator, device: zmq.Socket[bytes] +) -> None: + """The serve thread survives, and the next real request still works.""" + device.send_string(_request(object="Spi")) + device.recv() + + assert standalone_emulator.running + + device.send_string(_request(name="LED 1", operation=Operation.Set)) + response = json.loads(device.recv()) + + assert response["status"] == Status.Ok + + +def test_unroutable_response_is_not_answered( + standalone_emulator: DeviceEmulator, device: zmq.Socket[bytes] +) -> None: + """A Response gets no reply -- one would desynchronize the socket. + + Answering the tail of an exchange would leave an extra frame queued, and + every later request would read the previous request's reply. + """ + device.send_string(_request(type=MessageType.Response, object="Spi")) + + with pytest.raises(zmq.Again): + device.recv() + + assert standalone_emulator.running + + device.send_string(_request(name="LED 1", operation=Operation.Set)) + assert json.loads(device.recv())["status"] == Status.Ok From 1ef7102482a19dfe00a9b271a79e201cfa328889 Mon Sep 17 00:00:00 2001 From: Nehal Patel Date: Fri, 4 Sep 2026 05:17:54 +0000 Subject: [PATCH 03/12] feat(arm): first firmware image for the STM32F767ZI Nucleo Adds the arm_cm7 MCU backend and the Nucleo board, and takes all three applications from "does not configure" to a linked firmware image verified in both Debug and Release. No hardware behaviour yet: the peripherals are placeholders that return kInvalidOperation, so every application links from this commit and the bring-up can proceed one peripheral at a time rather than all at once. Vendor code is CMSIS headers only -- ST's register definitions for the F7 plus the Cortex-M core headers, both Apache-2.0, fetched and pinned. The drivers are written against registers directly. The Cube HAL would hide exactly the parts worth understanding (RCC enable ordering, the MODER/OTYPER/AFR layout, the EXTI/SYSCFG/NVIC chain) behind a gigabyte of middleware, while the 30k lines of register #defines it would save teach nothing that transcribing them by hand would not also risk getting wrong. startup.s is recovered from fa430af (ST, BSD-3) with two changes: its `.fpu softvfp` contradicted the command line's -mfpu=fpv5-sp-d16 and would have produced ABI-mismatch diagnostics, and `bl main` is now followed by an infinite loop rather than `bx lr`. The 110-entry vector table is why it is kept rather than rewritten. The linker script is written, not recovered. The Ac6 one in history carries a no-redistribution notice, sizes RAM at 320K (F746 numbers; the F767ZI has 512K), and discards every section of libc.a, libm.a and libgcc.a. The first word of blinky.bin is now 0x20080000 -- the top of the real 512K. Two things the first cross build found: - logger.cpp is written against , which libstdc++ 13 lacks. Only host_transport links it, so it is now built for the host only; the Logger interface and NullLogger are header-only and stay available. - CMAKE_EXECUTABLE_SUFFIX is reset by CMake's platform initialization after the toolchain file runs. The per-language variables survive. tools/verify-firmware.sh checks what decides whether the chip boots -- vector table at 0x08000000, entry point is Reset_Handler with the Thumb bit, .init_array non-empty so static constructors run, no undefined symbols -- all without hardware, which is what makes it CI's job later. Co-Authored-By: Claude Opus 5 (1M context) --- CMakeLists.txt | 35 + cmake/platform_artifacts.cmake | 30 + cmake/toolchain/armgcc.cmake | 8 +- src/apps/blinky/CMakeLists.txt | 2 + src/apps/i2c_demo/CMakeLists.txt | 2 + src/apps/uart_echo/CMakeLists.txt | 2 + .../board/stm32f767zi_nucleo/CMakeLists.txt | 31 + src/libs/board/stm32f767zi_nucleo/main.cpp | 33 + .../board/stm32f767zi_nucleo/nucleo_board.cpp | 29 + .../board/stm32f767zi_nucleo/nucleo_board.hpp | 40 ++ src/libs/board/stm32f767zi_nucleo/pin_map.hpp | 40 ++ src/libs/board/stm32f767zi_nucleo/startup.s | 627 ++++++++++++++++++ .../board/stm32f767zi_nucleo/stm32f767zi.ld | 147 ++++ .../unimplemented_peripherals.hpp | 102 +++ src/libs/common/CMakeLists.txt | 19 +- src/libs/mcu/arm_cm7/CMakeLists.txt | 16 + src/libs/mcu/arm_cm7/cmsis.hpp | 15 + src/libs/mcu/arm_cm7/cortex_m7.cpp | 25 + src/libs/mcu/arm_cm7/cortex_m7.hpp | 17 + src/libs/mcu/arm_cm7/delay.cpp | 57 ++ src/libs/mcu/arm_cm7/systick.cpp | 32 + src/libs/mcu/arm_cm7/systick.hpp | 19 + tools/verify-firmware.sh | 103 +++ 23 files changed, 1424 insertions(+), 7 deletions(-) create mode 100644 cmake/platform_artifacts.cmake create mode 100644 src/libs/board/stm32f767zi_nucleo/CMakeLists.txt create mode 100644 src/libs/board/stm32f767zi_nucleo/main.cpp create mode 100644 src/libs/board/stm32f767zi_nucleo/nucleo_board.cpp create mode 100644 src/libs/board/stm32f767zi_nucleo/nucleo_board.hpp create mode 100644 src/libs/board/stm32f767zi_nucleo/pin_map.hpp create mode 100644 src/libs/board/stm32f767zi_nucleo/startup.s create mode 100644 src/libs/board/stm32f767zi_nucleo/stm32f767zi.ld create mode 100644 src/libs/board/stm32f767zi_nucleo/unimplemented_peripherals.hpp create mode 100644 src/libs/mcu/arm_cm7/CMakeLists.txt create mode 100644 src/libs/mcu/arm_cm7/cmsis.hpp create mode 100644 src/libs/mcu/arm_cm7/cortex_m7.cpp create mode 100644 src/libs/mcu/arm_cm7/cortex_m7.hpp create mode 100644 src/libs/mcu/arm_cm7/delay.cpp create mode 100644 src/libs/mcu/arm_cm7/systick.cpp create mode 100644 src/libs/mcu/arm_cm7/systick.hpp create mode 100755 tools/verify-firmware.sh diff --git a/CMakeLists.txt b/CMakeLists.txt index c0690d7..5880ad2 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -133,6 +133,18 @@ FetchContent_Declare( GIT_TAG 0ca0fe433eb70cea0d5761079c0c5b47b736565b # v3.11.2 ) +FetchContent_Declare( + cmsis_core + GIT_REPOSITORY https://github.com/STMicroelectronics/cmsis_core.git + GIT_TAG 455a49f764c3378bcbc7d611deacae58cb027cd9 # v5.9.0 +) + +FetchContent_Declare( + cmsis_device_f7 + GIT_REPOSITORY https://github.com/STMicroelectronics/cmsis_device_f7.git + GIT_TAG 3ba5bdaf3584a6a907f854e553ee8e5b88f7677c # v1.2.9 +) + FetchContent_MakeAvailable(CmakeScripts) list(PREPEND CMAKE_MODULE_PATH "${cmakescripts_SOURCE_DIR}") @@ -149,6 +161,25 @@ if(EMBEDDED_CPP_MCU STREQUAL "host") endif() endif() +# CMSIS: the Cortex-M core headers and ST's register definitions for the F7. +# Headers only -- the peripheral drivers in src/libs/mcu/arm_cm7 are written +# against these registers directly rather than against the Cube HAL. The +# register definitions are mechanical transcriptions of RM0410 and there is +# nothing to learn from retyping them; the clock ordering and bit layouts that +# the HAL would hide are exactly what this project is for. +if(EMBEDDED_CPP_MCU MATCHES "^arm_") + FetchContent_MakeAvailable(cmsis_core cmsis_device_f7) + + # Neither repository ships a usable CMakeLists.txt, so wrap them here. + add_library(cmsis_f767 INTERFACE) + # SYSTEM is not optional: the vendor headers use anonymous structs and unions + # that project_warnings' -Wpedantic -Werror rejects. + target_include_directories(cmsis_f767 SYSTEM INTERFACE + "${cmsis_core_SOURCE_DIR}/Include" + "${cmsis_device_f7_SOURCE_DIR}/Include") + target_compile_definitions(cmsis_f767 INTERFACE STM32F767xx) +endif() + # --------------------------------------------------------------------------- # Tooling: clang-tidy, coverage, formatting # --------------------------------------------------------------------------- @@ -175,6 +206,10 @@ include("${PROJECT_SOURCE_DIR}/cmake/format.cmake") # platform implementations exist when an unknown one is requested. include("${PROJECT_SOURCE_DIR}/cmake/backends.cmake") +# add_platform_artifacts(): flashable .bin/.hex beside an application's ELF on +# cross builds; a no-op on the host. +include("${PROJECT_SOURCE_DIR}/cmake/platform_artifacts.cmake") + # --------------------------------------------------------------------------- # Targets # --------------------------------------------------------------------------- diff --git a/cmake/platform_artifacts.cmake b/cmake/platform_artifacts.cmake new file mode 100644 index 0000000..af653d5 --- /dev/null +++ b/cmake/platform_artifacts.cmake @@ -0,0 +1,30 @@ +# Per-platform build artifacts for application executables. +# +# A host executable is the deliverable; a firmware image is not. The linker +# produces an ELF, and the board wants a raw binary (to copy onto the ST-LINK +# mass-storage volume) or Intel HEX (for most flashing tools). +# +# This lives here rather than in the board directory because the targets it +# decorates are the applications, and a board's CMakeLists cannot reach them -- +# the board is added to the build after src/apps. + +function(add_platform_artifacts target) + if(NOT CMAKE_CROSSCOMPILING) + return() # A host build's executable is already what you run. + endif() + + add_custom_command(TARGET ${target} POST_BUILD + COMMAND ${CMAKE_OBJCOPY} -O binary $ + $/${target}.bin + COMMAND ${CMAKE_OBJCOPY} -O ihex $ + $/${target}.hex + # BYPRODUCTS resolves a narrower set of generator expressions than COMMAND + # does -- TARGET_FILE_DIR is not among them -- so spell the output + # directory out. It matches what TARGET_FILE_DIR expands to above, since + # the applications use the project-wide runtime output directory. + BYPRODUCTS + ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/$/${target}.bin + ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/$/${target}.hex + COMMENT "Creating ${target}.bin and ${target}.hex" + VERBATIM) +endfunction() diff --git a/cmake/toolchain/armgcc.cmake b/cmake/toolchain/armgcc.cmake index d9a0370..a461e80 100644 --- a/cmake/toolchain/armgcc.cmake +++ b/cmake/toolchain/armgcc.cmake @@ -54,7 +54,13 @@ set(CMAKE_EXE_LINKER_FLAGS_INIT "--specs=nano.specs --specs=nosys.specs -Wl,--gc # Firmware images, not host executables. Makes blinky.elf and blinky.bin # unambiguous in the build tree and in flashing instructions. -set(CMAKE_EXECUTABLE_SUFFIX ".elf") +# +# Per language: CMake's platform initialization resets the language-agnostic +# CMAKE_EXECUTABLE_SUFFIX after the toolchain file runs, so setting only that +# one silently does nothing. The per-language variables survive. +set(CMAKE_EXECUTABLE_SUFFIX_C ".elf") +set(CMAKE_EXECUTABLE_SUFFIX_CXX ".elf") +set(CMAKE_EXECUTABLE_SUFFIX_ASM ".elf") set(CMAKE_C_FLAGS_DEBUG_INIT "-O0") set(CMAKE_CXX_FLAGS_DEBUG_INIT "-O0") diff --git a/src/apps/blinky/CMakeLists.txt b/src/apps/blinky/CMakeLists.txt index 2063dc7..fda4c67 100644 --- a/src/apps/blinky/CMakeLists.txt +++ b/src/apps/blinky/CMakeLists.txt @@ -1,2 +1,4 @@ add_executable(blinky blinky.cpp) target_link_libraries(blinky PRIVATE project_options app platform_entry mcu error) + +add_platform_artifacts(blinky) diff --git a/src/apps/i2c_demo/CMakeLists.txt b/src/apps/i2c_demo/CMakeLists.txt index a7006f0..b29686f 100644 --- a/src/apps/i2c_demo/CMakeLists.txt +++ b/src/apps/i2c_demo/CMakeLists.txt @@ -1,2 +1,4 @@ add_executable(i2c_demo i2c_demo.cpp) target_link_libraries(i2c_demo PRIVATE project_options app platform_entry mcu error) + +add_platform_artifacts(i2c_demo) diff --git a/src/apps/uart_echo/CMakeLists.txt b/src/apps/uart_echo/CMakeLists.txt index d2e28e4..17b94c0 100644 --- a/src/apps/uart_echo/CMakeLists.txt +++ b/src/apps/uart_echo/CMakeLists.txt @@ -1,2 +1,4 @@ add_executable(uart_echo uart_echo.cpp) target_link_libraries(uart_echo PRIVATE project_options app platform_entry mcu error) + +add_platform_artifacts(uart_echo) diff --git a/src/libs/board/stm32f767zi_nucleo/CMakeLists.txt b/src/libs/board/stm32f767zi_nucleo/CMakeLists.txt new file mode 100644 index 0000000..65f54c3 --- /dev/null +++ b/src/libs/board/stm32f767zi_nucleo/CMakeLists.txt @@ -0,0 +1,31 @@ +add_library(nucleo_board nucleo_board.cpp) +target_sources(nucleo_board PUBLIC + FILE_SET HEADERS + BASE_DIRS ${PROJECT_SOURCE_DIR}/src + FILES nucleo_board.hpp pin_map.hpp unimplemented_peripherals.hpp) +target_link_libraries(nucleo_board + PUBLIC board mcu arm_cm7_mcu + PRIVATE project_options) + +# The platform entry point (see CLAUDE.md for the contract). An OBJECT library +# so that main(), the vector table and the interrupt handlers are linked into +# each application directly: pulling them out of a static archive depends on +# link-order symbol resolution and breaks under --gc-sections and LTO, and a +# vector table nothing references is exactly what --gc-sections removes. +add_library(platform_entry OBJECT main.cpp startup.s) +target_link_libraries(platform_entry PRIVATE project_options app board nucleo_board) + +set(LINKER_SCRIPT "${CMAKE_CURRENT_SOURCE_DIR}/stm32f767zi.ld") + +# INTERFACE rather than PRIVATE: platform_entry is an OBJECT library and never +# links anything itself. The link that needs these options is each application +# executable, which gets them through this target's usage requirements. +target_link_options(platform_entry INTERFACE + "-T${LINKER_SCRIPT}" + "LINKER:-Map=$.map") + +# INTERFACE_LINK_DEPENDS, not LINK_DEPENDS. The plain property is not +# transitive and would be attached to a target that never links, so editing the +# linker script would silently not relink the applications. +set_property(TARGET platform_entry APPEND + PROPERTY INTERFACE_LINK_DEPENDS "${LINKER_SCRIPT}") diff --git a/src/libs/board/stm32f767zi_nucleo/main.cpp b/src/libs/board/stm32f767zi_nucleo/main.cpp new file mode 100644 index 0000000..c2c1908 --- /dev/null +++ b/src/libs/board/stm32f767zi_nucleo/main.cpp @@ -0,0 +1,33 @@ +#include + +#include "apps/app.hpp" +#include "libs/board/stm32f767zi_nucleo/nucleo_board.hpp" + +namespace { + +// Namespace scope, not a local in main(), for two reasons. The board owns +// peripheral state that interrupt handlers reach for, and an ISR can fire +// before main() has entered its loop. And its construction here is what +// exercises .init_array: Reset_Handler calls __libc_init_array before main, +// and if the linker script's KEEP on that section were ever dropped, this +// object would silently never be constructed. +board::NucleoF767ZiBoard g_board; + +} // namespace + +extern "C" auto main() -> int { + if (auto result = app::AppMain(g_board); !result) { + // Nowhere to report to yet: this board has no console until USART3 is + // implemented. Keep the error where a debugger can read it and stop, so a + // failure is a halt at a known place rather than a silently idle board. + const auto error = std::to_underlying(result.error()); + static_cast(error); + while (true) { + } + } + + // main() must not return on bare metal. Reset_Handler hangs if it does; be + // explicit here rather than relying on that. + while (true) { + } +} diff --git a/src/libs/board/stm32f767zi_nucleo/nucleo_board.cpp b/src/libs/board/stm32f767zi_nucleo/nucleo_board.cpp new file mode 100644 index 0000000..c0d37c4 --- /dev/null +++ b/src/libs/board/stm32f767zi_nucleo/nucleo_board.cpp @@ -0,0 +1,29 @@ +#include "libs/board/stm32f767zi_nucleo/nucleo_board.hpp" + +#include + +#include "libs/common/error.hpp" +#include "libs/mcu/arm_cm7/systick.hpp" +#include "libs/mcu/i2c.hpp" +#include "libs/mcu/pin.hpp" +#include "libs/mcu/uart.hpp" + +namespace board { + +auto NucleoF767ZiBoard::Init() -> std::expected { + // The core is already configured: SystemInit ran from Reset_Handler, before + // .data was copied. What is left is everything that needs a working C++ + // runtime -- starting with the tick that mcu::Delay is built on. + mcu::InitSysTick(); + return {}; +} + +auto NucleoF767ZiBoard::UserLed1() -> mcu::OutputPin& { return user_led_1_; } +auto NucleoF767ZiBoard::UserLed2() -> mcu::OutputPin& { return user_led_2_; } +auto NucleoF767ZiBoard::UserButton1() -> mcu::InputPin& { + return user_button_1_; +} +auto NucleoF767ZiBoard::I2C1() -> mcu::I2CController& { return i2c_1_; } +auto NucleoF767ZiBoard::Uart1() -> mcu::Uart& { return uart_1_; } + +} // namespace board diff --git a/src/libs/board/stm32f767zi_nucleo/nucleo_board.hpp b/src/libs/board/stm32f767zi_nucleo/nucleo_board.hpp new file mode 100644 index 0000000..c93c191 --- /dev/null +++ b/src/libs/board/stm32f767zi_nucleo/nucleo_board.hpp @@ -0,0 +1,40 @@ +#pragma once + +#include + +#include "libs/board/board.hpp" +#include "libs/board/stm32f767zi_nucleo/unimplemented_peripherals.hpp" +#include "libs/common/error.hpp" +#include "libs/mcu/i2c.hpp" +#include "libs/mcu/pin.hpp" +#include "libs/mcu/uart.hpp" + +namespace board { + +/// @brief The STM32F767ZI Nucleo-144 board. +/// +/// Peripherals are members rather than pointers: there is no allocator worth +/// using here, and their lifetime is the board's. The board itself is a +/// namespace-scope object in main.cpp, so its constructor runs from +/// __libc_init_array before main(). +class NucleoF767ZiBoard final : public Board { + public: + [[nodiscard]] auto Init() -> std::expected override; + + [[nodiscard]] auto UserLed1() -> mcu::OutputPin& override; + [[nodiscard]] auto UserLed2() -> mcu::OutputPin& override; + [[nodiscard]] auto UserButton1() -> mcu::InputPin& override; + [[nodiscard]] auto I2C1() -> mcu::I2CController& override; + [[nodiscard]] auto Uart1() -> mcu::Uart& override; + + private: + // Placeholders until each peripheral's hardware implementation lands. See + // unimplemented_peripherals.hpp. + UnimplementedPin user_led_1_; + UnimplementedPin user_led_2_; + UnimplementedPin user_button_1_; + UnimplementedI2CController i2c_1_; + UnimplementedUart uart_1_; +}; + +} // namespace board diff --git a/src/libs/board/stm32f767zi_nucleo/pin_map.hpp b/src/libs/board/stm32f767zi_nucleo/pin_map.hpp new file mode 100644 index 0000000..9895f2a --- /dev/null +++ b/src/libs/board/stm32f767zi_nucleo/pin_map.hpp @@ -0,0 +1,40 @@ +#pragma once + +#include + +#include "libs/mcu/arm_cm7/cmsis.hpp" + +/// @file +/// Every board-specific pin assignment, in one table. +/// +/// Source: UM1974, "STM32 Nucleo-144 boards". The user LEDs and button are on +/// the board itself; the I2C and USART assignments follow the Arduino and +/// ST-LINK virtual COM port wiring respectively. +namespace board::pin_map { + +struct PinLocation { + GPIO_TypeDef* port; + std::uint32_t pin; ///< Bit position within the port, 0-15. +}; + +// LD1 green, LD2 blue, LD3 red. LD3 is not exposed through board::Board yet. +constexpr PinLocation kUserLed1{GPIOB, 0}; +constexpr PinLocation kUserLed2{GPIOB, 7}; +constexpr PinLocation kUserLed3{GPIOB, 14}; + +// B1, the blue user button. Externally pulled down, so it reads high when +// pressed and needs no internal pull. +constexpr PinLocation kUserButton1{GPIOC, 13}; + +// USART3 is wired to the ST-LINK virtual COM port: no external adapter needed, +// output shows up on /dev/ttyACM0. Alternate function 7. +constexpr PinLocation kUart1Tx{GPIOD, 8}; +constexpr PinLocation kUart1Rx{GPIOD, 9}; +constexpr std::uint32_t kUart1AlternateFunction = 7; + +// I2C1 on the Arduino connector: D15 (SCL) and D14 (SDA). Alternate function 4. +constexpr PinLocation kI2C1Scl{GPIOB, 8}; +constexpr PinLocation kI2C1Sda{GPIOB, 9}; +constexpr std::uint32_t kI2C1AlternateFunction = 4; + +} // namespace board::pin_map diff --git a/src/libs/board/stm32f767zi_nucleo/startup.s b/src/libs/board/stm32f767zi_nucleo/startup.s new file mode 100644 index 0000000..82e2cb1 --- /dev/null +++ b/src/libs/board/stm32f767zi_nucleo/startup.s @@ -0,0 +1,627 @@ +/* Recovered from this repository's history (commit fa430af) and modified. + Original: STMicroelectronics startup_stm32f767xx.s, BSD-3-Clause. + + Local changes: + - dropped `.fpu softvfp`, which contradicted the command-line FPU flags + - `bl main` is followed by an infinite loop, not `bx lr` + + Kept rather than rewritten because the 110-entry interrupt vector table + below must match the reference manual exactly, and transcribing it by hand + buys nothing but the chance of a typo. */ +/** + ****************************************************************************** + * @file startup_stm32f767xx.s + * @author MCD Application Team + * @brief STM32F767xx Devices vector table for GCC based toolchain. + * This module performs: + * - Set the initial SP + * - Set the initial PC == Reset_Handler, + * - Set the vector table entries with the exceptions ISR address + * - Branches to main in the C library (which eventually + * calls main()). + * After Reset the Cortex-M7 processor is in Thread mode, + * priority is Privileged, and the Stack is set to Main. + ****************************************************************************** + * @attention + * + * Copyright (c) 2016 STMicroelectronics. + * All rights reserved. + * + * This software is licensed under terms that can be found in the LICENSE file + * in the root directory of this software component. + * If no LICENSE file comes with this software, it is provided AS-IS. + * + ****************************************************************************** + */ + + .syntax unified + .cpu cortex-m7 + /* No .fpu directive: cmake/toolchain/armgcc-cm7.cmake puts + -mfpu=fpv5-sp-d16 -mfloat-abi=hard on the command line, and a + directive here would silently override it. */ + .thumb + +.global g_pfnVectors +.global Default_Handler + +/* start address for the initialization values of the .data section. +defined in linker script */ +.word _sidata +/* start address for the .data section. defined in linker script */ +.word _sdata +/* end address for the .data section. defined in linker script */ +.word _edata +/* start address for the .bss section. defined in linker script */ +.word _sbss +/* end address for the .bss section. defined in linker script */ +.word _ebss +/* stack used for SystemInit_ExtMemCtl; always internal RAM used */ + +/** + * @brief This is the code that gets called when the processor first + * starts execution following a reset event. Only the absolutely + * necessary set is performed, after which the application + * supplied main() routine is called. + * @param None + * @retval : None +*/ + + .section .text.Reset_Handler + .weak Reset_Handler + .type Reset_Handler, %function +Reset_Handler: + ldr sp, =_estack /* set stack pointer */ + +/* Copy the data segment initializers from flash to SRAM */ + movs r1, #0 + b LoopCopyDataInit + +CopyDataInit: + ldr r3, =_sidata + ldr r3, [r3, r1] + str r3, [r0, r1] + adds r1, r1, #4 + +LoopCopyDataInit: + ldr r0, =_sdata + ldr r3, =_edata + adds r2, r0, r1 + cmp r2, r3 + bcc CopyDataInit + ldr r2, =_sbss + b LoopFillZerobss +/* Zero fill the bss segment. */ +FillZerobss: + movs r3, #0 + str r3, [r2], #4 + +LoopFillZerobss: + ldr r3, = _ebss + cmp r2, r3 + bcc FillZerobss + +/* Call the clock system initialization function.*/ + bl SystemInit +/* Call static constructors */ + bl __libc_init_array +/* Call the application's entry point.*/ + bl main +/* main() does not return on bare metal. If it ever does, hang here rather + than branching to whatever lr happened to hold. */ + b . +.size Reset_Handler, .-Reset_Handler + +/** + * @brief This is the code that gets called when the processor receives an + * unexpected interrupt. This simply enters an infinite loop, preserving + * the system state for examination by a debugger. + * @param None + * @retval None +*/ + .section .text.Default_Handler,"ax",%progbits +Default_Handler: +Infinite_Loop: + b Infinite_Loop + .size Default_Handler, .-Default_Handler +/****************************************************************************** +* +* The minimal vector table for a Cortex M7. Note that the proper constructs +* must be placed on this to ensure that it ends up at physical address +* 0x0000.0000. +* +*******************************************************************************/ + .section .isr_vector,"a",%progbits + .type g_pfnVectors, %object + .size g_pfnVectors, .-g_pfnVectors + + +g_pfnVectors: + .word _estack + .word Reset_Handler + + .word NMI_Handler + .word HardFault_Handler + .word MemManage_Handler + .word BusFault_Handler + .word UsageFault_Handler + .word 0 + .word 0 + .word 0 + .word 0 + .word SVC_Handler + .word DebugMon_Handler + .word 0 + .word PendSV_Handler + .word SysTick_Handler + + /* External Interrupts */ + .word WWDG_IRQHandler /* Window WatchDog */ + .word PVD_IRQHandler /* PVD through EXTI Line detection */ + .word TAMP_STAMP_IRQHandler /* Tamper and TimeStamps through the EXTI line */ + .word RTC_WKUP_IRQHandler /* RTC Wakeup through the EXTI line */ + .word FLASH_IRQHandler /* FLASH */ + .word RCC_IRQHandler /* RCC */ + .word EXTI0_IRQHandler /* EXTI Line0 */ + .word EXTI1_IRQHandler /* EXTI Line1 */ + .word EXTI2_IRQHandler /* EXTI Line2 */ + .word EXTI3_IRQHandler /* EXTI Line3 */ + .word EXTI4_IRQHandler /* EXTI Line4 */ + .word DMA1_Stream0_IRQHandler /* DMA1 Stream 0 */ + .word DMA1_Stream1_IRQHandler /* DMA1 Stream 1 */ + .word DMA1_Stream2_IRQHandler /* DMA1 Stream 2 */ + .word DMA1_Stream3_IRQHandler /* DMA1 Stream 3 */ + .word DMA1_Stream4_IRQHandler /* DMA1 Stream 4 */ + .word DMA1_Stream5_IRQHandler /* DMA1 Stream 5 */ + .word DMA1_Stream6_IRQHandler /* DMA1 Stream 6 */ + .word ADC_IRQHandler /* ADC1, ADC2 and ADC3s */ + .word CAN1_TX_IRQHandler /* CAN1 TX */ + .word CAN1_RX0_IRQHandler /* CAN1 RX0 */ + .word CAN1_RX1_IRQHandler /* CAN1 RX1 */ + .word CAN1_SCE_IRQHandler /* CAN1 SCE */ + .word EXTI9_5_IRQHandler /* External Line[9:5]s */ + .word TIM1_BRK_TIM9_IRQHandler /* TIM1 Break and TIM9 */ + .word TIM1_UP_TIM10_IRQHandler /* TIM1 Update and TIM10 */ + .word TIM1_TRG_COM_TIM11_IRQHandler /* TIM1 Trigger and Commutation and TIM11 */ + .word TIM1_CC_IRQHandler /* TIM1 Capture Compare */ + .word TIM2_IRQHandler /* TIM2 */ + .word TIM3_IRQHandler /* TIM3 */ + .word TIM4_IRQHandler /* TIM4 */ + .word I2C1_EV_IRQHandler /* I2C1 Event */ + .word I2C1_ER_IRQHandler /* I2C1 Error */ + .word I2C2_EV_IRQHandler /* I2C2 Event */ + .word I2C2_ER_IRQHandler /* I2C2 Error */ + .word SPI1_IRQHandler /* SPI1 */ + .word SPI2_IRQHandler /* SPI2 */ + .word USART1_IRQHandler /* USART1 */ + .word USART2_IRQHandler /* USART2 */ + .word USART3_IRQHandler /* USART3 */ + .word EXTI15_10_IRQHandler /* External Line[15:10]s */ + .word RTC_Alarm_IRQHandler /* RTC Alarm (A and B) through EXTI Line */ + .word OTG_FS_WKUP_IRQHandler /* USB OTG FS Wakeup through EXTI line */ + .word TIM8_BRK_TIM12_IRQHandler /* TIM8 Break and TIM12 */ + .word TIM8_UP_TIM13_IRQHandler /* TIM8 Update and TIM13 */ + .word TIM8_TRG_COM_TIM14_IRQHandler /* TIM8 Trigger and Commutation and TIM14 */ + .word TIM8_CC_IRQHandler /* TIM8 Capture Compare */ + .word DMA1_Stream7_IRQHandler /* DMA1 Stream7 */ + .word FMC_IRQHandler /* FMC */ + .word SDMMC1_IRQHandler /* SDMMC1 */ + .word TIM5_IRQHandler /* TIM5 */ + .word SPI3_IRQHandler /* SPI3 */ + .word UART4_IRQHandler /* UART4 */ + .word UART5_IRQHandler /* UART5 */ + .word TIM6_DAC_IRQHandler /* TIM6 and DAC1&2 underrun errors */ + .word TIM7_IRQHandler /* TIM7 */ + .word DMA2_Stream0_IRQHandler /* DMA2 Stream 0 */ + .word DMA2_Stream1_IRQHandler /* DMA2 Stream 1 */ + .word DMA2_Stream2_IRQHandler /* DMA2 Stream 2 */ + .word DMA2_Stream3_IRQHandler /* DMA2 Stream 3 */ + .word DMA2_Stream4_IRQHandler /* DMA2 Stream 4 */ + .word ETH_IRQHandler /* Ethernet */ + .word ETH_WKUP_IRQHandler /* Ethernet Wakeup through EXTI line */ + .word CAN2_TX_IRQHandler /* CAN2 TX */ + .word CAN2_RX0_IRQHandler /* CAN2 RX0 */ + .word CAN2_RX1_IRQHandler /* CAN2 RX1 */ + .word CAN2_SCE_IRQHandler /* CAN2 SCE */ + .word OTG_FS_IRQHandler /* USB OTG FS */ + .word DMA2_Stream5_IRQHandler /* DMA2 Stream 5 */ + .word DMA2_Stream6_IRQHandler /* DMA2 Stream 6 */ + .word DMA2_Stream7_IRQHandler /* DMA2 Stream 7 */ + .word USART6_IRQHandler /* USART6 */ + .word I2C3_EV_IRQHandler /* I2C3 event */ + .word I2C3_ER_IRQHandler /* I2C3 error */ + .word OTG_HS_EP1_OUT_IRQHandler /* USB OTG HS End Point 1 Out */ + .word OTG_HS_EP1_IN_IRQHandler /* USB OTG HS End Point 1 In */ + .word OTG_HS_WKUP_IRQHandler /* USB OTG HS Wakeup through EXTI */ + .word OTG_HS_IRQHandler /* USB OTG HS */ + .word DCMI_IRQHandler /* DCMI */ + .word 0 /* Reserved */ + .word RNG_IRQHandler /* RNG */ + .word FPU_IRQHandler /* FPU */ + .word UART7_IRQHandler /* UART7 */ + .word UART8_IRQHandler /* UART8 */ + .word SPI4_IRQHandler /* SPI4 */ + .word SPI5_IRQHandler /* SPI5 */ + .word SPI6_IRQHandler /* SPI6 */ + .word SAI1_IRQHandler /* SAI1 */ + .word LTDC_IRQHandler /* LTDC */ + .word LTDC_ER_IRQHandler /* LTDC error */ + .word DMA2D_IRQHandler /* DMA2D */ + .word SAI2_IRQHandler /* SAI2 */ + .word QUADSPI_IRQHandler /* QUADSPI */ + .word LPTIM1_IRQHandler /* LPTIM1 */ + .word CEC_IRQHandler /* HDMI_CEC */ + .word I2C4_EV_IRQHandler /* I2C4 Event */ + .word I2C4_ER_IRQHandler /* I2C4 Error */ + .word SPDIF_RX_IRQHandler /* SPDIF_RX */ + .word 0 /* Reserved */ + .word DFSDM1_FLT0_IRQHandler /* DFSDM1 Filter 0 global Interrupt */ + .word DFSDM1_FLT1_IRQHandler /* DFSDM1 Filter 1 global Interrupt */ + .word DFSDM1_FLT2_IRQHandler /* DFSDM1 Filter 2 global Interrupt */ + .word DFSDM1_FLT3_IRQHandler /* DFSDM1 Filter 3 global Interrupt */ + .word SDMMC2_IRQHandler /* SDMMC2 */ + .word CAN3_TX_IRQHandler /* CAN3 TX */ + .word CAN3_RX0_IRQHandler /* CAN3 RX0 */ + .word CAN3_RX1_IRQHandler /* CAN3 RX1 */ + .word CAN3_SCE_IRQHandler /* CAN3 SCE */ + .word JPEG_IRQHandler /* JPEG */ + .word MDIOS_IRQHandler /* MDIOS */ + +/******************************************************************************* +* +* Provide weak aliases for each Exception handler to the Default_Handler. +* As they are weak aliases, any function with the same name will override +* this definition. +* +*******************************************************************************/ + .weak NMI_Handler + .thumb_set NMI_Handler,Default_Handler + + .weak HardFault_Handler + .thumb_set HardFault_Handler,Default_Handler + + .weak MemManage_Handler + .thumb_set MemManage_Handler,Default_Handler + + .weak BusFault_Handler + .thumb_set BusFault_Handler,Default_Handler + + .weak UsageFault_Handler + .thumb_set UsageFault_Handler,Default_Handler + + .weak SVC_Handler + .thumb_set SVC_Handler,Default_Handler + + .weak DebugMon_Handler + .thumb_set DebugMon_Handler,Default_Handler + + .weak PendSV_Handler + .thumb_set PendSV_Handler,Default_Handler + + .weak SysTick_Handler + .thumb_set SysTick_Handler,Default_Handler + + .weak WWDG_IRQHandler + .thumb_set WWDG_IRQHandler,Default_Handler + + .weak PVD_IRQHandler + .thumb_set PVD_IRQHandler,Default_Handler + + .weak TAMP_STAMP_IRQHandler + .thumb_set TAMP_STAMP_IRQHandler,Default_Handler + + .weak RTC_WKUP_IRQHandler + .thumb_set RTC_WKUP_IRQHandler,Default_Handler + + .weak FLASH_IRQHandler + .thumb_set FLASH_IRQHandler,Default_Handler + + .weak RCC_IRQHandler + .thumb_set RCC_IRQHandler,Default_Handler + + .weak EXTI0_IRQHandler + .thumb_set EXTI0_IRQHandler,Default_Handler + + .weak EXTI1_IRQHandler + .thumb_set EXTI1_IRQHandler,Default_Handler + + .weak EXTI2_IRQHandler + .thumb_set EXTI2_IRQHandler,Default_Handler + + .weak EXTI3_IRQHandler + .thumb_set EXTI3_IRQHandler,Default_Handler + + .weak EXTI4_IRQHandler + .thumb_set EXTI4_IRQHandler,Default_Handler + + .weak DMA1_Stream0_IRQHandler + .thumb_set DMA1_Stream0_IRQHandler,Default_Handler + + .weak DMA1_Stream1_IRQHandler + .thumb_set DMA1_Stream1_IRQHandler,Default_Handler + + .weak DMA1_Stream2_IRQHandler + .thumb_set DMA1_Stream2_IRQHandler,Default_Handler + + .weak DMA1_Stream3_IRQHandler + .thumb_set DMA1_Stream3_IRQHandler,Default_Handler + + .weak DMA1_Stream4_IRQHandler + .thumb_set DMA1_Stream4_IRQHandler,Default_Handler + + .weak DMA1_Stream5_IRQHandler + .thumb_set DMA1_Stream5_IRQHandler,Default_Handler + + .weak DMA1_Stream6_IRQHandler + .thumb_set DMA1_Stream6_IRQHandler,Default_Handler + + .weak ADC_IRQHandler + .thumb_set ADC_IRQHandler,Default_Handler + + .weak CAN1_TX_IRQHandler + .thumb_set CAN1_TX_IRQHandler,Default_Handler + + .weak CAN1_RX0_IRQHandler + .thumb_set CAN1_RX0_IRQHandler,Default_Handler + + .weak CAN1_RX1_IRQHandler + .thumb_set CAN1_RX1_IRQHandler,Default_Handler + + .weak CAN1_SCE_IRQHandler + .thumb_set CAN1_SCE_IRQHandler,Default_Handler + + .weak EXTI9_5_IRQHandler + .thumb_set EXTI9_5_IRQHandler,Default_Handler + + .weak TIM1_BRK_TIM9_IRQHandler + .thumb_set TIM1_BRK_TIM9_IRQHandler,Default_Handler + + .weak TIM1_UP_TIM10_IRQHandler + .thumb_set TIM1_UP_TIM10_IRQHandler,Default_Handler + + .weak TIM1_TRG_COM_TIM11_IRQHandler + .thumb_set TIM1_TRG_COM_TIM11_IRQHandler,Default_Handler + + .weak TIM1_CC_IRQHandler + .thumb_set TIM1_CC_IRQHandler,Default_Handler + + .weak TIM2_IRQHandler + .thumb_set TIM2_IRQHandler,Default_Handler + + .weak TIM3_IRQHandler + .thumb_set TIM3_IRQHandler,Default_Handler + + .weak TIM4_IRQHandler + .thumb_set TIM4_IRQHandler,Default_Handler + + .weak I2C1_EV_IRQHandler + .thumb_set I2C1_EV_IRQHandler,Default_Handler + + .weak I2C1_ER_IRQHandler + .thumb_set I2C1_ER_IRQHandler,Default_Handler + + .weak I2C2_EV_IRQHandler + .thumb_set I2C2_EV_IRQHandler,Default_Handler + + .weak I2C2_ER_IRQHandler + .thumb_set I2C2_ER_IRQHandler,Default_Handler + + .weak SPI1_IRQHandler + .thumb_set SPI1_IRQHandler,Default_Handler + + .weak SPI2_IRQHandler + .thumb_set SPI2_IRQHandler,Default_Handler + + .weak USART1_IRQHandler + .thumb_set USART1_IRQHandler,Default_Handler + + .weak USART2_IRQHandler + .thumb_set USART2_IRQHandler,Default_Handler + + .weak USART3_IRQHandler + .thumb_set USART3_IRQHandler,Default_Handler + + .weak EXTI15_10_IRQHandler + .thumb_set EXTI15_10_IRQHandler,Default_Handler + + .weak RTC_Alarm_IRQHandler + .thumb_set RTC_Alarm_IRQHandler,Default_Handler + + .weak OTG_FS_WKUP_IRQHandler + .thumb_set OTG_FS_WKUP_IRQHandler,Default_Handler + + .weak TIM8_BRK_TIM12_IRQHandler + .thumb_set TIM8_BRK_TIM12_IRQHandler,Default_Handler + + .weak TIM8_UP_TIM13_IRQHandler + .thumb_set TIM8_UP_TIM13_IRQHandler,Default_Handler + + .weak TIM8_TRG_COM_TIM14_IRQHandler + .thumb_set TIM8_TRG_COM_TIM14_IRQHandler,Default_Handler + + .weak TIM8_CC_IRQHandler + .thumb_set TIM8_CC_IRQHandler,Default_Handler + + .weak DMA1_Stream7_IRQHandler + .thumb_set DMA1_Stream7_IRQHandler,Default_Handler + + .weak FMC_IRQHandler + .thumb_set FMC_IRQHandler,Default_Handler + + .weak SDMMC1_IRQHandler + .thumb_set SDMMC1_IRQHandler,Default_Handler + + .weak TIM5_IRQHandler + .thumb_set TIM5_IRQHandler,Default_Handler + + .weak SPI3_IRQHandler + .thumb_set SPI3_IRQHandler,Default_Handler + + .weak UART4_IRQHandler + .thumb_set UART4_IRQHandler,Default_Handler + + .weak UART5_IRQHandler + .thumb_set UART5_IRQHandler,Default_Handler + + .weak TIM6_DAC_IRQHandler + .thumb_set TIM6_DAC_IRQHandler,Default_Handler + + .weak TIM7_IRQHandler + .thumb_set TIM7_IRQHandler,Default_Handler + + .weak DMA2_Stream0_IRQHandler + .thumb_set DMA2_Stream0_IRQHandler,Default_Handler + + .weak DMA2_Stream1_IRQHandler + .thumb_set DMA2_Stream1_IRQHandler,Default_Handler + + .weak DMA2_Stream2_IRQHandler + .thumb_set DMA2_Stream2_IRQHandler,Default_Handler + + .weak DMA2_Stream3_IRQHandler + .thumb_set DMA2_Stream3_IRQHandler,Default_Handler + + .weak DMA2_Stream4_IRQHandler + .thumb_set DMA2_Stream4_IRQHandler,Default_Handler + + .weak ETH_IRQHandler + .thumb_set ETH_IRQHandler,Default_Handler + + .weak ETH_WKUP_IRQHandler + .thumb_set ETH_WKUP_IRQHandler,Default_Handler + + .weak CAN2_TX_IRQHandler + .thumb_set CAN2_TX_IRQHandler,Default_Handler + + .weak CAN2_RX0_IRQHandler + .thumb_set CAN2_RX0_IRQHandler,Default_Handler + + .weak CAN2_RX1_IRQHandler + .thumb_set CAN2_RX1_IRQHandler,Default_Handler + + .weak CAN2_SCE_IRQHandler + .thumb_set CAN2_SCE_IRQHandler,Default_Handler + + .weak OTG_FS_IRQHandler + .thumb_set OTG_FS_IRQHandler,Default_Handler + + .weak DMA2_Stream5_IRQHandler + .thumb_set DMA2_Stream5_IRQHandler,Default_Handler + + .weak DMA2_Stream6_IRQHandler + .thumb_set DMA2_Stream6_IRQHandler,Default_Handler + + .weak DMA2_Stream7_IRQHandler + .thumb_set DMA2_Stream7_IRQHandler,Default_Handler + + .weak USART6_IRQHandler + .thumb_set USART6_IRQHandler,Default_Handler + + .weak I2C3_EV_IRQHandler + .thumb_set I2C3_EV_IRQHandler,Default_Handler + + .weak I2C3_ER_IRQHandler + .thumb_set I2C3_ER_IRQHandler,Default_Handler + + .weak OTG_HS_EP1_OUT_IRQHandler + .thumb_set OTG_HS_EP1_OUT_IRQHandler,Default_Handler + + .weak OTG_HS_EP1_IN_IRQHandler + .thumb_set OTG_HS_EP1_IN_IRQHandler,Default_Handler + + .weak OTG_HS_WKUP_IRQHandler + .thumb_set OTG_HS_WKUP_IRQHandler,Default_Handler + + .weak OTG_HS_IRQHandler + .thumb_set OTG_HS_IRQHandler,Default_Handler + + .weak DCMI_IRQHandler + .thumb_set DCMI_IRQHandler,Default_Handler + + .weak RNG_IRQHandler + .thumb_set RNG_IRQHandler,Default_Handler + + .weak FPU_IRQHandler + .thumb_set FPU_IRQHandler,Default_Handler + + .weak UART7_IRQHandler + .thumb_set UART7_IRQHandler,Default_Handler + + .weak UART8_IRQHandler + .thumb_set UART8_IRQHandler,Default_Handler + + .weak SPI4_IRQHandler + .thumb_set SPI4_IRQHandler,Default_Handler + + .weak SPI5_IRQHandler + .thumb_set SPI5_IRQHandler,Default_Handler + + .weak SPI6_IRQHandler + .thumb_set SPI6_IRQHandler,Default_Handler + + .weak SAI1_IRQHandler + .thumb_set SAI1_IRQHandler,Default_Handler + + .weak LTDC_IRQHandler + .thumb_set LTDC_IRQHandler,Default_Handler + + .weak LTDC_ER_IRQHandler + .thumb_set LTDC_ER_IRQHandler,Default_Handler + + .weak DMA2D_IRQHandler + .thumb_set DMA2D_IRQHandler,Default_Handler + + .weak SAI2_IRQHandler + .thumb_set SAI2_IRQHandler,Default_Handler + + .weak QUADSPI_IRQHandler + .thumb_set QUADSPI_IRQHandler,Default_Handler + + .weak LPTIM1_IRQHandler + .thumb_set LPTIM1_IRQHandler,Default_Handler + + .weak CEC_IRQHandler + .thumb_set CEC_IRQHandler,Default_Handler + + .weak I2C4_EV_IRQHandler + .thumb_set I2C4_EV_IRQHandler,Default_Handler + + .weak I2C4_ER_IRQHandler + .thumb_set I2C4_ER_IRQHandler,Default_Handler + + .weak SPDIF_RX_IRQHandler + .thumb_set SPDIF_RX_IRQHandler,Default_Handler + + .weak DFSDM1_FLT0_IRQHandler + .thumb_set DFSDM1_FLT0_IRQHandler,Default_Handler + + .weak DFSDM1_FLT1_IRQHandler + .thumb_set DFSDM1_FLT1_IRQHandler,Default_Handler + + .weak DFSDM1_FLT2_IRQHandler + .thumb_set DFSDM1_FLT2_IRQHandler,Default_Handler + + .weak DFSDM1_FLT3_IRQHandler + .thumb_set DFSDM1_FLT3_IRQHandler,Default_Handler + + .weak SDMMC2_IRQHandler + .thumb_set SDMMC2_IRQHandler,Default_Handler + + .weak CAN3_TX_IRQHandler + .thumb_set CAN3_TX_IRQHandler,Default_Handler + + .weak CAN3_RX0_IRQHandler + .thumb_set CAN3_RX0_IRQHandler,Default_Handler + + .weak CAN3_RX1_IRQHandler + .thumb_set CAN3_RX1_IRQHandler,Default_Handler + + .weak CAN3_SCE_IRQHandler + .thumb_set CAN3_SCE_IRQHandler,Default_Handler + + .weak JPEG_IRQHandler + .thumb_set JPEG_IRQHandler,Default_Handler + + .weak MDIOS_IRQHandler + .thumb_set MDIOS_IRQHandler,Default_Handler + + diff --git a/src/libs/board/stm32f767zi_nucleo/stm32f767zi.ld b/src/libs/board/stm32f767zi_nucleo/stm32f767zi.ld new file mode 100644 index 0000000..6848b01 --- /dev/null +++ b/src/libs/board/stm32f767zi_nucleo/stm32f767zi.ld @@ -0,0 +1,147 @@ +/* Linker script for the STM32F767ZI (2 MB flash, 512 KB SRAM). + * + * Written rather than recovered. The Ac6/System Workbench script that used to + * live here carried a no-redistribution notice, sized RAM at 320 KB (those are + * F746 numbers -- the F767ZI has 512 KB), and discarded every section of + * libc.a, libm.a and libgcc.a. + * + * RAM is one region because the F767ZI's three blocks are address-contiguous: + * DTCM 128 KB @0x20000000, SRAM1 368 KB @0x20020000, SRAM2 16 KB @0x2007C000. + * Split them if DMA ever needs to avoid DTCM, which is reachable by DMA only + * through the CPU's AHBS slave port (RM0410). + */ + +ENTRY(Reset_Handler) + +MEMORY +{ + FLASH (rx) : ORIGIN = 0x08000000, LENGTH = 2048K + RAM (rwx) : ORIGIN = 0x20000000, LENGTH = 512K +} + +/* The reset vector loads SP from the first word of the vector table. */ +_estack = ORIGIN(RAM) + LENGTH(RAM); + +_Min_Heap_Size = 0x200; /* newlib's malloc arena, reached via _sbrk */ +_Min_Stack_Size = 0x400; + +SECTIONS +{ + /* The boot ROM reads the vector table from the start of flash. Nothing + references it, so without KEEP --gc-sections removes it and the chip + boots into whatever follows. */ + .isr_vector : + { + . = ALIGN(4); + KEEP(*(.isr_vector)) + . = ALIGN(4); + } >FLASH + + .text : + { + . = ALIGN(4); + *(.text) + *(.text*) + *(.glue_7) + *(.glue_7t) + *(.eh_frame) + + KEEP(*(.init)) + KEEP(*(.fini)) + + . = ALIGN(4); + _etext = .; + } >FLASH + + .rodata : + { + . = ALIGN(4); + *(.rodata) + *(.rodata*) + . = ALIGN(4); + } >FLASH + + /* Unwind tables. -fno-exceptions means these should be empty; keeping the + sections costs nothing and avoids a link error if a library brings some. */ + .ARM.extab : { *(.ARM.extab* .gnu.linkonce.armextab.*) } >FLASH + .ARM : + { + __exidx_start = .; + *(.ARM.exidx*) + __exidx_end = .; + } >FLASH + + /* Static constructors. __libc_init_array walks these; KEEP is what makes + namespace-scope objects with non-trivial constructors actually run. */ + .preinit_array : + { + PROVIDE_HIDDEN(__preinit_array_start = .); + KEEP(*(.preinit_array*)) + PROVIDE_HIDDEN(__preinit_array_end = .); + } >FLASH + + .init_array : + { + PROVIDE_HIDDEN(__init_array_start = .); + KEEP(*(SORT(.init_array.*))) + KEEP(*(.init_array*)) + PROVIDE_HIDDEN(__init_array_end = .); + } >FLASH + + .fini_array : + { + PROVIDE_HIDDEN(__fini_array_start = .); + KEEP(*(SORT(.fini_array.*))) + KEEP(*(.fini_array*)) + PROVIDE_HIDDEN(__fini_array_end = .); + } >FLASH + + /* Initialized data: stored in flash, copied to RAM by Reset_Handler. + _sidata is the load address it copies from. */ + _sidata = LOADADDR(.data); + + .data : + { + . = ALIGN(4); + _sdata = .; + *(.data) + *(.data*) + . = ALIGN(4); + _edata = .; + } >RAM AT> FLASH + + . = ALIGN(4); + .bss : + { + _sbss = .; + __bss_start__ = _sbss; + *(.bss) + *(.bss*) + *(COMMON) + . = ALIGN(4); + _ebss = .; + __bss_end__ = _ebss; + } >RAM + + /* _sbrk starts the heap at `end` and refuses to grow past __heap_limit, + so a runaway allocation returns ENOMEM instead of quietly overwriting + the stack growing down from _estack. */ + ._user_heap_stack : + { + . = ALIGN(8); + PROVIDE(end = .); + PROVIDE(_end = .); + . = . + _Min_Heap_Size; + . = . + _Min_Stack_Size; + . = ALIGN(8); + } >RAM + + __heap_limit = _estack - _Min_Stack_Size; + + /DISCARD/ : { libc.a(*.note.GNU-stack) } + + .ARM.attributes 0 : { *(.ARM.attributes) } +} + +ASSERT(_estack - _ebss >= _Min_Heap_Size + _Min_Stack_Size, + "RAM exhausted: not enough room left for the heap and stack") diff --git a/src/libs/board/stm32f767zi_nucleo/unimplemented_peripherals.hpp b/src/libs/board/stm32f767zi_nucleo/unimplemented_peripherals.hpp new file mode 100644 index 0000000..aefba54 --- /dev/null +++ b/src/libs/board/stm32f767zi_nucleo/unimplemented_peripherals.hpp @@ -0,0 +1,102 @@ +#pragma once + +#include +#include +#include +#include +#include +#include + +#include "libs/common/error.hpp" +#include "libs/mcu/i2c.hpp" +#include "libs/mcu/pin.hpp" +#include "libs/mcu/uart.hpp" + +/// @file +/// Placeholders for the peripherals this board has not implemented yet. +/// +/// board::Board is an all-or-nothing interface: a board that implements none +/// of it does not compile, and one that implements only pins cannot be +/// constructed. These stubs let the board satisfy the interface from the +/// first commit, so every application links and the hardware bring-up can +/// proceed one peripheral at a time instead of all at once. +/// +/// Each returns kInvalidOperation rather than pretending to succeed: an +/// application that reaches for an unimplemented peripheral gets an error at +/// the call, not silence. They are deleted as the real implementations land. +namespace board { + +/// Satisfies both pin interfaces: OutputPin inherits InputPin virtually, so a +/// stand-in for an output pin must answer Get() and SetInterruptHandler() too. +class UnimplementedPin final : public mcu::BidirectionalPin { + public: + [[nodiscard]] auto Get() + -> std::expected override { + return std::unexpected(common::Error::kInvalidOperation); + } + + [[nodiscard]] auto SetInterruptHandler(std::function /*handler*/, + mcu::PinTransition /*transition*/) + -> std::expected override { + return std::unexpected(common::Error::kInvalidOperation); + } + + [[nodiscard]] auto SetHigh() -> std::expected override { + return std::unexpected(common::Error::kInvalidOperation); + } + + [[nodiscard]] auto SetLow() -> std::expected override { + return std::unexpected(common::Error::kInvalidOperation); + } + + [[nodiscard]] auto Toggle() -> std::expected override { + return std::unexpected(common::Error::kInvalidOperation); + } + + [[nodiscard]] auto Configure(mcu::PinDirection /*direction*/) + -> std::expected override { + return std::unexpected(common::Error::kInvalidOperation); + } +}; + +class UnimplementedUart final : public mcu::Uart { + public: + [[nodiscard]] auto Init(const mcu::UartConfig& /*config*/) + -> std::expected override { + return std::unexpected(common::Error::kInvalidOperation); + } + + [[nodiscard]] auto Send(std::span /*data*/) + -> std::expected override { + return std::unexpected(common::Error::kInvalidOperation); + } + + [[nodiscard]] auto Receive(std::span /*buffer*/, + std::uint32_t /*timeout_ms*/) + -> std::expected override { + return std::unexpected(common::Error::kInvalidOperation); + } + + [[nodiscard]] auto SetRxHandler( + std::function /*handler*/) + -> std::expected override { + return std::unexpected(common::Error::kInvalidOperation); + } +}; + +class UnimplementedI2CController final : public mcu::I2CController { + public: + [[nodiscard]] auto SendData(std::uint16_t /*address*/, + std::span /*data*/) + -> std::expected override { + return std::unexpected(common::Error::kInvalidOperation); + } + + [[nodiscard]] auto ReceiveData(std::uint16_t /*address*/, + std::span /*buffer*/) + -> std::expected override { + return std::unexpected(common::Error::kInvalidOperation); + } +}; + +} // namespace board diff --git a/src/libs/common/CMakeLists.txt b/src/libs/common/CMakeLists.txt index 32c652c..8fbcc09 100644 --- a/src/libs/common/CMakeLists.txt +++ b/src/libs/common/CMakeLists.txt @@ -4,9 +4,16 @@ target_sources(error INTERFACE BASE_DIRS ${PROJECT_SOURCE_DIR}/src FILES error.hpp) -add_library(logger logger.cpp) -target_sources(logger PUBLIC - FILE_SET HEADERS - BASE_DIRS ${PROJECT_SOURCE_DIR}/src - FILES logger.hpp) -target_link_libraries(logger PRIVATE project_options) +# ConsoleLogger is written against , which libstdc++ 13 does not have -- +# and arm-none-eabi-g++ is 13.2. It is also the wrong abstraction for a target +# with no console. Only host_transport links this, so build it for the host +# only; logger.hpp remains includable everywhere for the Logger interface and +# NullLogger, which are header-only. +if(EMBEDDED_CPP_MCU STREQUAL "host") + add_library(logger logger.cpp) + target_sources(logger PUBLIC + FILE_SET HEADERS + BASE_DIRS ${PROJECT_SOURCE_DIR}/src + FILES logger.hpp) + target_link_libraries(logger PRIVATE project_options) +endif() diff --git a/src/libs/mcu/arm_cm7/CMakeLists.txt b/src/libs/mcu/arm_cm7/CMakeLists.txt new file mode 100644 index 0000000..bb0c7a6 --- /dev/null +++ b/src/libs/mcu/arm_cm7/CMakeLists.txt @@ -0,0 +1,16 @@ +# Cortex-M7 MCU backend. +# +# A note on the name: only cortex_m7.* , systick.* and delay.cpp are genuinely +# core peripherals. Everything else here is STM32F7-specific and belongs to a +# vendor layer that does not exist yet, because there is exactly one consumer. +# The filenames carry the distinction; split the directory when an arm_cm4 +# backend arrives and wants to share the GPIO code. +add_library(arm_cm7_mcu cortex_m7.cpp systick.cpp delay.cpp) +target_sources(arm_cm7_mcu PUBLIC + FILE_SET HEADERS + BASE_DIRS ${PROJECT_SOURCE_DIR}/src + FILES cmsis.hpp cortex_m7.hpp systick.hpp) +# cmsis_f767 is PUBLIC because cmsis.hpp, a public header, includes it. +target_link_libraries(arm_cm7_mcu + PUBLIC mcu cmsis_f767 + PRIVATE project_options) diff --git a/src/libs/mcu/arm_cm7/cmsis.hpp b/src/libs/mcu/arm_cm7/cmsis.hpp new file mode 100644 index 0000000..a9f807e --- /dev/null +++ b/src/libs/mcu/arm_cm7/cmsis.hpp @@ -0,0 +1,15 @@ +#pragma once + +/// @file +/// The one place that includes ST's CMSIS device header. +/// +/// The vendor header defines every peripheral register on the part. It is +/// included through an INTERFACE target marked SYSTEM (see the cmsis_f767 +/// target in the top-level CMakeLists.txt) because it uses anonymous structs +/// and unions that this project's -Wpedantic -Werror would otherwise reject. +/// Routing every use through this header keeps that arrangement in one place, +/// and makes the dependency on vendor code greppable. + +// NOLINTBEGIN(misc-include-cleaner) +#include +// NOLINTEND(misc-include-cleaner) diff --git a/src/libs/mcu/arm_cm7/cortex_m7.cpp b/src/libs/mcu/arm_cm7/cortex_m7.cpp new file mode 100644 index 0000000..122bb5b --- /dev/null +++ b/src/libs/mcu/arm_cm7/cortex_m7.cpp @@ -0,0 +1,25 @@ +#include "libs/mcu/arm_cm7/cortex_m7.hpp" + +#include "libs/mcu/arm_cm7/cmsis.hpp" + +namespace mcu { + +namespace { + +// CPACR bits 20-23 are the CP10/CP11 access fields; 0b11 in each grants full +// access. CP10 and CP11 must always be programmed identically (ARMv7-M ARM). +constexpr auto kCp10FullAccess = 3U << 20U; +constexpr auto kCp11FullAccess = 3U << 22U; + +} // namespace + +extern "C" auto SystemInit() -> void { + SCB->CPACR |= kCp10FullAccess | kCp11FullAccess; + + // The vector table lives at the start of flash, where the boot ROM found it. + // Set VTOR explicitly rather than relying on its reset value: it survives a + // soft reset, and a bootloader may have moved it. + SCB->VTOR = FLASH_BASE; +} + +} // namespace mcu diff --git a/src/libs/mcu/arm_cm7/cortex_m7.hpp b/src/libs/mcu/arm_cm7/cortex_m7.hpp new file mode 100644 index 0000000..bcea078 --- /dev/null +++ b/src/libs/mcu/arm_cm7/cortex_m7.hpp @@ -0,0 +1,17 @@ +#pragma once + +namespace mcu { + +/// @brief Bring the core to a state where compiled C++ can run correctly. +/// +/// Called from Reset_Handler (as SystemInit) before .data is usable and before +/// static constructors run, so it must touch nothing but core registers. +/// +/// Enables the FPU and points the vector table at flash. The FPU matters even +/// for code with no floating point in sight: the toolchain compiles with +/// -mfloat-abi=hard, so the first FP instruction from anywhere -- a library +/// routine included -- traps as a UsageFault at a program counter that looks +/// entirely unrelated to the cause. +extern "C" auto SystemInit() -> void; + +} // namespace mcu diff --git a/src/libs/mcu/arm_cm7/delay.cpp b/src/libs/mcu/arm_cm7/delay.cpp new file mode 100644 index 0000000..3a24c28 --- /dev/null +++ b/src/libs/mcu/arm_cm7/delay.cpp @@ -0,0 +1,57 @@ +#include "libs/mcu/delay.hpp" + +#include +#include + +#include "libs/mcu/arm_cm7/cmsis.hpp" +#include "libs/mcu/arm_cm7/systick.hpp" + +namespace mcu { + +namespace { + +// Sub-millisecond waits spin on the DWT cycle counter, which the SysTick +// interrupt is too coarse to serve. Enabled lazily: DWT lives in the debug +// block and is not required to be running after reset. +constexpr std::uint32_t kSystemCoreClockHz = 16'000'000; +constexpr std::uint32_t kCyclesPerMicrosecond = kSystemCoreClockHz / 1'000'000; + +auto EnableCycleCounter() -> void { + CoreDebug->DEMCR |= CoreDebug_DEMCR_TRCENA_Msk; + DWT->CTRL |= DWT_CTRL_CYCCNTENA_Msk; +} + +auto SpinCycles(std::uint32_t cycles) -> void { + EnableCycleCounter(); + const std::uint32_t start = DWT->CYCCNT; + // Unsigned subtraction, so the counter's 32-bit wrap needs no special case. + while ((DWT->CYCCNT - start) < cycles) { + } +} + +} // namespace + +auto Delay(std::chrono::microseconds duration) -> void { + if (duration.count() <= 0) { + return; + } + + const auto microseconds = static_cast(duration.count()); + const auto whole_milliseconds = + static_cast(microseconds / 1'000); + const auto remainder = static_cast(microseconds % 1'000); + + if (whole_milliseconds > 0) { + // Wait for whole_milliseconds *edges*, not elapsed time: entering this + // function part-way through a tick would otherwise round the wait down. + const std::uint32_t start = Millis(); + while ((Millis() - start) <= whole_milliseconds) { + } + } + + if (remainder > 0) { + SpinCycles(remainder * kCyclesPerMicrosecond); + } +} + +} // namespace mcu diff --git a/src/libs/mcu/arm_cm7/systick.cpp b/src/libs/mcu/arm_cm7/systick.cpp new file mode 100644 index 0000000..b8b2750 --- /dev/null +++ b/src/libs/mcu/arm_cm7/systick.cpp @@ -0,0 +1,32 @@ +#include "libs/mcu/arm_cm7/systick.hpp" + +#include + +#include "libs/mcu/arm_cm7/cmsis.hpp" + +namespace mcu { + +namespace { + +// The reset clock configuration: HSI, 16 MHz, no PLL. The board deliberately +// does not raise this yet -- a PLL, the caches and the ART accelerator are all +// changes that can break working peripherals, and are worth making one at a +// time against a known-good baseline. +constexpr std::uint32_t kSystemCoreClockHz = 16'000'000; +constexpr std::uint32_t kTickRateHz = 1'000; + +// Written by the interrupt handler, read by everything else. +volatile std::uint32_t g_ticks = 0; + +} // namespace + +// Read-modify-write spelled out: ++ on a volatile-qualified operand is +// deprecated in C++20, because the standard does not say how many accesses it +// makes. One load and one store is what is wanted here, and what this says. +extern "C" auto SysTick_Handler() -> void { g_ticks = g_ticks + 1; } + +auto Millis() -> std::uint32_t { return g_ticks; } + +auto InitSysTick() -> void { SysTick_Config(kSystemCoreClockHz / kTickRateHz); } + +} // namespace mcu diff --git a/src/libs/mcu/arm_cm7/systick.hpp b/src/libs/mcu/arm_cm7/systick.hpp new file mode 100644 index 0000000..3e7cebd --- /dev/null +++ b/src/libs/mcu/arm_cm7/systick.hpp @@ -0,0 +1,19 @@ +#pragma once + +#include + +namespace mcu { + +/// @brief Ticks since Init(), one per millisecond. +/// +/// Wraps after ~49 days. Compare differences rather than absolute values and +/// unsigned arithmetic makes the wrap harmless. +[[nodiscard]] auto Millis() -> std::uint32_t; + +/// @brief Start the 1 kHz system tick. +/// +/// Idempotent. Called from the board's Init() rather than from SystemInit, +/// because it enables an interrupt and must not run before .bss is zeroed. +auto InitSysTick() -> void; + +} // namespace mcu diff --git a/tools/verify-firmware.sh b/tools/verify-firmware.sh new file mode 100755 index 0000000..a6aaacc --- /dev/null +++ b/tools/verify-firmware.sh @@ -0,0 +1,103 @@ +#!/usr/bin/env bash +# +# Structural checks on a linked firmware image. These are the properties that +# decide whether the chip boots at all, and every one of them can be verified +# without hardware -- so CI runs this on every cross build. +# +# Usage: tools/verify-firmware.sh + +set -euo pipefail + +BUILD_DIR="${1:?usage: verify-firmware.sh }" +OBJPREFIX="${OBJPREFIX:-arm-none-eabi-}" +VECTOR_ADDRESS="08000000" + +readonly NM="${OBJPREFIX}nm" +readonly READELF="${OBJPREFIX}readelf" + +failures=0 + +fail() { + printf ' FAIL: %s\n' "$1" + failures=$((failures + 1)) +} + +check_image() { + local elf="$1" + printf '%s\n' "${elf#"${BUILD_DIR}"/}" + + # The boot ROM loads the initial stack pointer and reset vector from the + # first two words of flash. A vector table nothing references is exactly + # what --gc-sections removes, so this catches a dropped KEEP. + local vector + vector=$("${READELF}" -S "${elf}" | awk '/\.isr_vector/ {print $5}') + if [[ "${vector}" != "${VECTOR_ADDRESS}" ]]; then + fail ".isr_vector at 0x${vector:-}, expected 0x${VECTOR_ADDRESS}" + fi + + # The ELF entry point must be Reset_Handler with the Thumb bit set: on + # Cortex-M, bit 0 of a branch target selects Thumb state, and an even entry + # address faults on the first instruction. + local entry reset + entry=$("${READELF}" -h "${elf}" | awk '/Entry point address:/ {print $NF}') + reset=$("${NM}" "${elf}" | awk '$3 == "Reset_Handler" {print "0x" $1}') + if [[ -z "${reset}" ]]; then + fail "no Reset_Handler symbol" + elif (( entry != (reset | 1) )); then + fail "entry ${entry} is not Reset_Handler (${reset}) with the Thumb bit" + fi + + # Static constructors run only if __libc_init_array has entries to walk. + local init_size + init_size=$("${READELF}" -S "${elf}" | awk '/\.init_array/ {print $6}') + if [[ -z "${init_size}" || "${init_size}" == "000000" ]]; then + fail ".init_array is empty: namespace-scope constructors will not run" + fi + + # A firmware image has no loader to resolve anything later. + local undefined + undefined=$("${NM}" -u "${elf}") + if [[ -n "${undefined}" ]]; then + fail "undefined symbols: $(tr -s '[:space:]' ' ' <<<"${undefined}")" + fi +} + +# The interrupt-callback paths must not allocate. mcu::InputPin and mcu::Uart +# take std::function handlers, which is allocation-free only while the captured +# state fits libstdc++'s 16-byte small-buffer optimization. That is a real +# property of the code today, and this is what keeps it true: an operator new +# reference in these objects means a handler outgrew the buffer. +check_no_allocation() { + local pattern='_Znwj|_Znaj|_ZdlPvj' + local objects + mapfile -t objects < <(find "${BUILD_DIR}" -name 'gpio_pin.cpp.obj' \ + -o -name 'exti.cpp.obj' \ + -o -name 'usart.cpp.obj') + (( ${#objects[@]} == 0 )) && return 0 + + printf 'interrupt-path allocation check\n' + for object in "${objects[@]}"; do + if "${NM}" -u "${object}" | grep -qE "${pattern}"; then + fail "${object##*/} references operator new/delete; a std::function + handler has outgrown the small-buffer optimization. See the note in + src/libs/mcu/pin.hpp." + fi + done +} + +mapfile -t images < <(find "${BUILD_DIR}" -name '*.elf' | sort) +if (( ${#images[@]} == 0 )); then + echo "No .elf images under ${BUILD_DIR}" >&2 + exit 1 +fi + +for image in "${images[@]}"; do + check_image "${image}" +done +check_no_allocation + +if (( failures > 0 )); then + printf '\n%d check(s) failed.\n' "${failures}" >&2 + exit 1 +fi +printf '\nAll %d image(s) verified.\n' "${#images[@]}" From 9727ed0cbdfabb305299c7a8a90d46652112cba3 Mon Sep 17 00:00:00 2001 From: Nehal Patel Date: Fri, 4 Sep 2026 05:23:27 +0000 Subject: [PATCH 04/12] feat(arm): real GPIO and EXTI, so blinky drives the board's pins Replaces the pin placeholders with mcu::GpioPin against MODER/BSRR/IDR, and implements SetInterruptHandler through EXTI. blinky.cpp is unchanged and now has a complete path on hardware: LED1 toggling on the SysTick-backed delay, and LED2 lit from an interrupt handler on the button's rising edge. Two things came out of making it compile. CMSIS defines I2C1, USART3 and several hundred other bare identifiers as object-like macros, so any header that includes stm32f767xx.h rewrites matching names in every header that follows it -- board::Board::I2C1() became a syntax error the moment gpio_pin.hpp reached nucleo_board.hpp. The fix is that no public header in the backend names a vendor type: pins identify their port with mcu::GpioPort, an enum whose values are the port index (which is also the RCC_AHB1ENR bit and the SYSCFG_EXTICR value), and cmsis.hpp is included only from .cpp files. gpio_registers.hpp carries the one function whose signature needs a vendor type and says why it must not be included from a header. The interrupt-path allocation check caught its own pattern being wrong. It looked for operator delete alongside operator new, but a polymorphic class's vtable references its deleting destructor unconditionally, so every one of these objects has an undefined operator delete and always will -- which is also why _sbrk is not optional here. operator new is the symbol that means a std::function handler outgrew the small-buffer optimization, and there is none: blinky's [this] capture is 4 bytes against a 16-byte buffer, as assumed. The check now tests the claim it was meant to. Also guards mcu::Delay against being called before the board starts the tick, where waiting on Millis() would never return; it falls back to the cycle counter, so an early delay is imprecise rather than a hang. Real GPIO and EXTI cost 1744 B of flash over the placeholders (Release blinky: 2572 -> 4316 B), which is the measured price of the pin interface's virtual-inheritance diamond and its thunks. Hardware verification is still pending -- this is verified by cross-build, image layout, and symbol checks only. Co-Authored-By: Claude Opus 5 (1M context) --- docs/PROJECT_PLAN.md | 5 + .../board/stm32f767zi_nucleo/nucleo_board.cpp | 10 +- .../board/stm32f767zi_nucleo/nucleo_board.hpp | 13 +- src/libs/board/stm32f767zi_nucleo/pin_map.hpp | 20 +-- .../unimplemented_peripherals.hpp | 40 +---- src/libs/mcu/arm_cm7/CMakeLists.txt | 6 +- src/libs/mcu/arm_cm7/delay.cpp | 18 ++- src/libs/mcu/arm_cm7/exti.cpp | 137 ++++++++++++++++++ src/libs/mcu/arm_cm7/exti.hpp | 26 ++++ src/libs/mcu/arm_cm7/gpio_pin.cpp | 111 ++++++++++++++ src/libs/mcu/arm_cm7/gpio_pin.hpp | 45 ++++++ src/libs/mcu/arm_cm7/gpio_port.cpp | 27 ++++ src/libs/mcu/arm_cm7/gpio_port.hpp | 40 +++++ src/libs/mcu/arm_cm7/gpio_registers.hpp | 15 ++ src/libs/mcu/arm_cm7/systick.cpp | 4 + src/libs/mcu/arm_cm7/systick.hpp | 6 + tools/verify-firmware.sh | 7 +- 17 files changed, 478 insertions(+), 52 deletions(-) create mode 100644 src/libs/mcu/arm_cm7/exti.cpp create mode 100644 src/libs/mcu/arm_cm7/exti.hpp create mode 100644 src/libs/mcu/arm_cm7/gpio_pin.cpp create mode 100644 src/libs/mcu/arm_cm7/gpio_pin.hpp create mode 100644 src/libs/mcu/arm_cm7/gpio_port.cpp create mode 100644 src/libs/mcu/arm_cm7/gpio_port.hpp create mode 100644 src/libs/mcu/arm_cm7/gpio_registers.hpp diff --git a/docs/PROJECT_PLAN.md b/docs/PROJECT_PLAN.md index 68ae67e..4968768 100644 --- a/docs/PROJECT_PLAN.md +++ b/docs/PROJECT_PLAN.md @@ -71,6 +71,11 @@ unreachable (git history preserves them, and CubeMX regenerates them fresher). **Tasks**: - [ ] STM32F3 Discovery support (`arm_cm4` backend + board directory) +- [ ] Revisit `board::Board`'s all-or-nothing interface now that a second + hardware board exists: optional accessors + (`std::expected`) vs. capability mix-ins vs. + compile-time board traits. Deferred from Milestone 2 deliberately — with + one board there was nothing to design against. - [ ] Additional example application exercising more complex behavior - [ ] Cross-board validation: blinky runs on both boards unmodified diff --git a/src/libs/board/stm32f767zi_nucleo/nucleo_board.cpp b/src/libs/board/stm32f767zi_nucleo/nucleo_board.cpp index c0d37c4..7090b7e 100644 --- a/src/libs/board/stm32f767zi_nucleo/nucleo_board.cpp +++ b/src/libs/board/stm32f767zi_nucleo/nucleo_board.cpp @@ -15,7 +15,15 @@ auto NucleoF767ZiBoard::Init() -> std::expected { // .data was copied. What is left is everything that needs a working C++ // runtime -- starting with the tick that mcu::Delay is built on. mcu::InitSysTick(); - return {}; + + // The button is externally pulled down on this board (UM1974), so it needs + // no internal pull: it reads low at rest and high while pressed. + return user_led_1_.Configure(mcu::PinDirection::kOutput) + .and_then( + [this] { return user_led_2_.Configure(mcu::PinDirection::kOutput); }) + .and_then([this] { + return user_button_1_.Configure(mcu::PinDirection::kInput); + }); } auto NucleoF767ZiBoard::UserLed1() -> mcu::OutputPin& { return user_led_1_; } diff --git a/src/libs/board/stm32f767zi_nucleo/nucleo_board.hpp b/src/libs/board/stm32f767zi_nucleo/nucleo_board.hpp index c93c191..6f28367 100644 --- a/src/libs/board/stm32f767zi_nucleo/nucleo_board.hpp +++ b/src/libs/board/stm32f767zi_nucleo/nucleo_board.hpp @@ -3,8 +3,10 @@ #include #include "libs/board/board.hpp" +#include "libs/board/stm32f767zi_nucleo/pin_map.hpp" #include "libs/board/stm32f767zi_nucleo/unimplemented_peripherals.hpp" #include "libs/common/error.hpp" +#include "libs/mcu/arm_cm7/gpio_pin.hpp" #include "libs/mcu/i2c.hpp" #include "libs/mcu/pin.hpp" #include "libs/mcu/uart.hpp" @@ -17,6 +19,9 @@ namespace board { /// using here, and their lifetime is the board's. The board itself is a /// namespace-scope object in main.cpp, so its constructor runs from /// __libc_init_array before main(). +/// +/// Constructing a GpioPin touches no registers, which is what makes that safe +/// -- the pins record where they are, and Init() is what configures them. class NucleoF767ZiBoard final : public Board { public: [[nodiscard]] auto Init() -> std::expected override; @@ -28,11 +33,13 @@ class NucleoF767ZiBoard final : public Board { [[nodiscard]] auto Uart1() -> mcu::Uart& override; private: + mcu::GpioPin user_led_1_{pin_map::kUserLed1.port, pin_map::kUserLed1.pin}; + mcu::GpioPin user_led_2_{pin_map::kUserLed2.port, pin_map::kUserLed2.pin}; + mcu::GpioPin user_button_1_{pin_map::kUserButton1.port, + pin_map::kUserButton1.pin}; + // Placeholders until each peripheral's hardware implementation lands. See // unimplemented_peripherals.hpp. - UnimplementedPin user_led_1_; - UnimplementedPin user_led_2_; - UnimplementedPin user_button_1_; UnimplementedI2CController i2c_1_; UnimplementedUart uart_1_; }; diff --git a/src/libs/board/stm32f767zi_nucleo/pin_map.hpp b/src/libs/board/stm32f767zi_nucleo/pin_map.hpp index 9895f2a..5c625b1 100644 --- a/src/libs/board/stm32f767zi_nucleo/pin_map.hpp +++ b/src/libs/board/stm32f767zi_nucleo/pin_map.hpp @@ -2,7 +2,7 @@ #include -#include "libs/mcu/arm_cm7/cmsis.hpp" +#include "libs/mcu/arm_cm7/gpio_port.hpp" /// @file /// Every board-specific pin assignment, in one table. @@ -13,28 +13,28 @@ namespace board::pin_map { struct PinLocation { - GPIO_TypeDef* port; + mcu::GpioPort port; std::uint32_t pin; ///< Bit position within the port, 0-15. }; // LD1 green, LD2 blue, LD3 red. LD3 is not exposed through board::Board yet. -constexpr PinLocation kUserLed1{GPIOB, 0}; -constexpr PinLocation kUserLed2{GPIOB, 7}; -constexpr PinLocation kUserLed3{GPIOB, 14}; +constexpr PinLocation kUserLed1{mcu::GpioPort::kB, 0}; +constexpr PinLocation kUserLed2{mcu::GpioPort::kB, 7}; +constexpr PinLocation kUserLed3{mcu::GpioPort::kB, 14}; // B1, the blue user button. Externally pulled down, so it reads high when // pressed and needs no internal pull. -constexpr PinLocation kUserButton1{GPIOC, 13}; +constexpr PinLocation kUserButton1{mcu::GpioPort::kC, 13}; // USART3 is wired to the ST-LINK virtual COM port: no external adapter needed, // output shows up on /dev/ttyACM0. Alternate function 7. -constexpr PinLocation kUart1Tx{GPIOD, 8}; -constexpr PinLocation kUart1Rx{GPIOD, 9}; +constexpr PinLocation kUart1Tx{mcu::GpioPort::kD, 8}; +constexpr PinLocation kUart1Rx{mcu::GpioPort::kD, 9}; constexpr std::uint32_t kUart1AlternateFunction = 7; // I2C1 on the Arduino connector: D15 (SCL) and D14 (SDA). Alternate function 4. -constexpr PinLocation kI2C1Scl{GPIOB, 8}; -constexpr PinLocation kI2C1Sda{GPIOB, 9}; +constexpr PinLocation kI2C1Scl{mcu::GpioPort::kB, 8}; +constexpr PinLocation kI2C1Sda{mcu::GpioPort::kB, 9}; constexpr std::uint32_t kI2C1AlternateFunction = 4; } // namespace board::pin_map diff --git a/src/libs/board/stm32f767zi_nucleo/unimplemented_peripherals.hpp b/src/libs/board/stm32f767zi_nucleo/unimplemented_peripherals.hpp index aefba54..234a6ca 100644 --- a/src/libs/board/stm32f767zi_nucleo/unimplemented_peripherals.hpp +++ b/src/libs/board/stm32f767zi_nucleo/unimplemented_peripherals.hpp @@ -24,41 +24,15 @@ /// Each returns kInvalidOperation rather than pretending to succeed: an /// application that reaches for an unimplemented peripheral gets an error at /// the call, not silence. They are deleted as the real implementations land. +/// +/// These have a delete-by date: B7 removes UnimplementedUart, B8 +/// UnimplementedI2CController. (UnimplementedPin is gone: B4 landed.) A stub +/// still here after B8 has stopped being bring-up scaffolding and become +/// evidence of a separate problem -- that board::Board cannot express "this +/// board does not have that peripheral" (see Milestone 3 in +/// docs/PROJECT_PLAN.md). namespace board { -/// Satisfies both pin interfaces: OutputPin inherits InputPin virtually, so a -/// stand-in for an output pin must answer Get() and SetInterruptHandler() too. -class UnimplementedPin final : public mcu::BidirectionalPin { - public: - [[nodiscard]] auto Get() - -> std::expected override { - return std::unexpected(common::Error::kInvalidOperation); - } - - [[nodiscard]] auto SetInterruptHandler(std::function /*handler*/, - mcu::PinTransition /*transition*/) - -> std::expected override { - return std::unexpected(common::Error::kInvalidOperation); - } - - [[nodiscard]] auto SetHigh() -> std::expected override { - return std::unexpected(common::Error::kInvalidOperation); - } - - [[nodiscard]] auto SetLow() -> std::expected override { - return std::unexpected(common::Error::kInvalidOperation); - } - - [[nodiscard]] auto Toggle() -> std::expected override { - return std::unexpected(common::Error::kInvalidOperation); - } - - [[nodiscard]] auto Configure(mcu::PinDirection /*direction*/) - -> std::expected override { - return std::unexpected(common::Error::kInvalidOperation); - } -}; - class UnimplementedUart final : public mcu::Uart { public: [[nodiscard]] auto Init(const mcu::UartConfig& /*config*/) diff --git a/src/libs/mcu/arm_cm7/CMakeLists.txt b/src/libs/mcu/arm_cm7/CMakeLists.txt index bb0c7a6..4bd80c5 100644 --- a/src/libs/mcu/arm_cm7/CMakeLists.txt +++ b/src/libs/mcu/arm_cm7/CMakeLists.txt @@ -5,11 +5,13 @@ # vendor layer that does not exist yet, because there is exactly one consumer. # The filenames carry the distinction; split the directory when an arm_cm4 # backend arrives and wants to share the GPIO code. -add_library(arm_cm7_mcu cortex_m7.cpp systick.cpp delay.cpp) +add_library(arm_cm7_mcu + cortex_m7.cpp systick.cpp delay.cpp gpio_pin.cpp gpio_port.cpp exti.cpp) target_sources(arm_cm7_mcu PUBLIC FILE_SET HEADERS BASE_DIRS ${PROJECT_SOURCE_DIR}/src - FILES cmsis.hpp cortex_m7.hpp systick.hpp) + FILES cmsis.hpp cortex_m7.hpp systick.hpp + gpio_pin.hpp gpio_port.hpp gpio_registers.hpp exti.hpp) # cmsis_f767 is PUBLIC because cmsis.hpp, a public header, includes it. target_link_libraries(arm_cm7_mcu PUBLIC mcu cmsis_f767 diff --git a/src/libs/mcu/arm_cm7/delay.cpp b/src/libs/mcu/arm_cm7/delay.cpp index 3a24c28..028fb24 100644 --- a/src/libs/mcu/arm_cm7/delay.cpp +++ b/src/libs/mcu/arm_cm7/delay.cpp @@ -15,17 +15,23 @@ namespace { // block and is not required to be running after reset. constexpr std::uint32_t kSystemCoreClockHz = 16'000'000; constexpr std::uint32_t kCyclesPerMicrosecond = kSystemCoreClockHz / 1'000'000; +constexpr std::uint64_t kMaxSpinCycles = 0xFFFF'0000ULL; auto EnableCycleCounter() -> void { CoreDebug->DEMCR |= CoreDebug_DEMCR_TRCENA_Msk; DWT->CTRL |= DWT_CTRL_CYCCNTENA_Msk; } -auto SpinCycles(std::uint32_t cycles) -> void { +/// Busy-wait for a cycle count. Caps at just under a full wrap of the 32-bit +/// counter (~268 s at 16 MHz): a longer request cannot be distinguished from a +/// shorter one once the counter has lapped, so clamp rather than return early. +auto SpinCycles(std::uint64_t cycles) -> void { EnableCycleCounter(); + const auto bounded = static_cast( + cycles < kMaxSpinCycles ? cycles : kMaxSpinCycles); const std::uint32_t start = DWT->CYCCNT; // Unsigned subtraction, so the counter's 32-bit wrap needs no special case. - while ((DWT->CYCCNT - start) < cycles) { + while ((DWT->CYCCNT - start) < bounded) { } } @@ -41,6 +47,14 @@ auto Delay(std::chrono::microseconds duration) -> void { static_cast(microseconds / 1'000); const auto remainder = static_cast(microseconds % 1'000); + // Before the board's Init() has started the tick, Millis() never advances + // and waiting on it would never return. Spin on the cycle counter instead, + // so an early Delay() is merely imprecise rather than a hang. + if (!SysTickRunning()) { + SpinCycles(microseconds * kCyclesPerMicrosecond); + return; + } + if (whole_milliseconds > 0) { // Wait for whole_milliseconds *edges*, not elapsed time: entering this // function part-way through a tick would otherwise round the wait down. diff --git a/src/libs/mcu/arm_cm7/exti.cpp b/src/libs/mcu/arm_cm7/exti.cpp new file mode 100644 index 0000000..cbee9f2 --- /dev/null +++ b/src/libs/mcu/arm_cm7/exti.cpp @@ -0,0 +1,137 @@ +#include "libs/mcu/arm_cm7/exti.hpp" + +#include +#include +#include +#include +#include + +#include "libs/common/error.hpp" +#include "libs/mcu/arm_cm7/cmsis.hpp" +#include "libs/mcu/arm_cm7/gpio_port.hpp" +#include "libs/mcu/pin.hpp" + +namespace mcu { + +namespace { + +constexpr std::uint32_t kExtiLineCount = 16; + +struct ExtiLine { + std::function handler; + std::uint32_t port_index = 0; + bool claimed = false; +}; + +// One slot per line, indexed by pin number. Namespace scope rather than +// function-local: an interrupt can arrive before any function-local static +// would have been initialized. +std::array g_lines; + +/// Route line `pin` to `port_index` via SYSCFG. Each EXTICR word holds four +/// 4-bit fields, so line n lives in word n/4 at bit offset (n%4)*4. +auto SelectPortForLine(std::uint32_t pin, std::uint32_t port_index) -> void { + RCC->APB2ENR |= RCC_APB2ENR_SYSCFGEN; + static_cast(RCC->APB2ENR); + + const std::uint32_t word = pin / 4U; + const std::uint32_t shift = (pin % 4U) * 4U; + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-constant-array-index) + auto value = SYSCFG->EXTICR[word]; + value &= ~(0xFU << shift); + value |= port_index << shift; + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-constant-array-index) + SYSCFG->EXTICR[word] = value; +} + +/// Lines 0-4 have their own NVIC vectors; 5-9 and 10-15 are each shared. +[[nodiscard]] auto IrqForLine(std::uint32_t pin) -> IRQn_Type { + if (pin <= 4U) { + return static_cast(EXTI0_IRQn + static_cast(pin)); + } + return pin <= 9U ? EXTI9_5_IRQn : EXTI15_10_IRQn; +} + +/// Clear the pending flag first, then dispatch. Clearing after the handler +/// would drop an edge that arrived while the handler was running; clearing +/// first at worst runs the handler twice, which is the recoverable direction. +auto ServiceLine(std::uint32_t pin) -> void { + const std::uint32_t mask = 1U << pin; + if ((EXTI->PR & mask) == 0U) { + return; + } + EXTI->PR = mask; // rc_w1: writing 1 clears. + + const auto& line = g_lines.at(pin); + if (line.handler) { + line.handler(); + } +} + +auto ServiceRange(std::uint32_t first, std::uint32_t last) -> void { + for (std::uint32_t pin = first; pin <= last; ++pin) { + ServiceLine(pin); + } +} + +} // namespace + +auto RegisterExtiHandler( + GpioPort port, std::uint32_t pin, std::function handler, + PinTransition transition) -> std::expected { + if (pin >= kExtiLineCount) { + return std::unexpected(common::Error::kInvalidArgument); + } + + const auto port_index = static_cast(port); + auto& line = g_lines.at(pin); + if (line.claimed && line.port_index != port_index) { + // Another port already owns this line. Rerouting it would silently stop + // delivering the first port's interrupts. + return std::unexpected(common::Error::kInvalidState); + } + + line.handler = std::move(handler); + line.port_index = port_index; + line.claimed = true; + + SelectPortForLine(pin, port_index); + + const std::uint32_t mask = 1U << pin; + const bool rising = transition == PinTransition::kRising || + transition == PinTransition::kBoth; + const bool falling = transition == PinTransition::kFalling || + transition == PinTransition::kBoth; + + if (rising) { + EXTI->RTSR |= mask; + } else { + EXTI->RTSR &= ~mask; + } + if (falling) { + EXTI->FTSR |= mask; + } else { + EXTI->FTSR &= ~mask; + } + + EXTI->PR = mask; // Discard anything latched during configuration. + EXTI->IMR |= mask; + + const auto irq = IrqForLine(pin); + NVIC_SetPriority(irq, 5); + NVIC_EnableIRQ(irq); + + return {}; +} + +// The vector table (startup.s) declares these weak and aliased to +// Default_Handler; defining them here overrides the alias. +extern "C" auto EXTI0_IRQHandler() -> void { ServiceLine(0); } +extern "C" auto EXTI1_IRQHandler() -> void { ServiceLine(1); } +extern "C" auto EXTI2_IRQHandler() -> void { ServiceLine(2); } +extern "C" auto EXTI3_IRQHandler() -> void { ServiceLine(3); } +extern "C" auto EXTI4_IRQHandler() -> void { ServiceLine(4); } +extern "C" auto EXTI9_5_IRQHandler() -> void { ServiceRange(5, 9); } +extern "C" auto EXTI15_10_IRQHandler() -> void { ServiceRange(10, 15); } + +} // namespace mcu diff --git a/src/libs/mcu/arm_cm7/exti.hpp b/src/libs/mcu/arm_cm7/exti.hpp new file mode 100644 index 0000000..b07c980 --- /dev/null +++ b/src/libs/mcu/arm_cm7/exti.hpp @@ -0,0 +1,26 @@ +#pragma once + +#include +#include +#include + +#include "libs/common/error.hpp" +#include "libs/mcu/arm_cm7/gpio_port.hpp" +#include "libs/mcu/pin.hpp" + +namespace mcu { + +/// @brief Register a handler for edges on one EXTI line. +/// +/// The STM32 routes external interrupts by pin *number*, not by port: line 3 +/// is PA3, PB3, PC3... and only one of them at a time. Registering a second +/// port on a line already claimed by another returns kInvalidState rather +/// than silently rerouting the first one away. +/// +/// The handler runs in interrupt context. It is stored, so it must own +/// whatever it captures -- see the note on std::function in libs/mcu/pin.hpp. +[[nodiscard]] auto RegisterExtiHandler( + GpioPort port, std::uint32_t pin, std::function handler, + PinTransition transition) -> std::expected; + +} // namespace mcu diff --git a/src/libs/mcu/arm_cm7/gpio_pin.cpp b/src/libs/mcu/arm_cm7/gpio_pin.cpp new file mode 100644 index 0000000..1847f83 --- /dev/null +++ b/src/libs/mcu/arm_cm7/gpio_pin.cpp @@ -0,0 +1,111 @@ +#include "libs/mcu/arm_cm7/gpio_pin.hpp" + +#include +#include +#include +#include + +#include "libs/common/error.hpp" +#include "libs/mcu/arm_cm7/cmsis.hpp" +#include "libs/mcu/arm_cm7/exti.hpp" +#include "libs/mcu/arm_cm7/gpio_port.hpp" +#include "libs/mcu/arm_cm7/gpio_registers.hpp" +#include "libs/mcu/pin.hpp" + +namespace mcu { + +namespace { + +constexpr std::uint32_t kModeInput = 0b00; +constexpr std::uint32_t kModeOutput = 0b01; +constexpr std::uint32_t kModeFieldWidth = 2; + +} // namespace + +auto GpioPin::Configure(PinDirection direction) + -> std::expected { + // Before the port clock is running, every register here reads as zero and + // ignores writes, without faulting. + EnablePortClock(port_); + + const std::uint32_t shift = pin_ * kModeFieldWidth; + const std::uint32_t mode = + direction == PinDirection::kOutput ? kModeOutput : kModeInput; + + auto* registers = PortRegisters(port_); + auto moder = registers->MODER; + moder &= ~(0b11U << shift); + moder |= mode << shift; + registers->MODER = moder; + + direction_ = direction; + configured_ = true; + return {}; +} + +auto GpioPin::Get() -> std::expected { + if (!configured_) { + return std::unexpected(common::Error::kInvalidState); + } + // IDR, not ODR, for both directions: it reports what the pad is actually at, + // so a shorted or externally driven output reads as what it really is rather + // than as what it was told to be. + auto* registers = PortRegisters(port_); + return (registers->IDR & Mask()) != 0U ? PinState::kHigh : PinState::kLow; +} + +auto GpioPin::SetHigh() -> std::expected { + if (!configured_) { + return std::unexpected(common::Error::kInvalidState); + } + if (direction_ != PinDirection::kOutput) { + return std::unexpected(common::Error::kInvalidOperation); + } + // BSRR sets from its low half and resets from its high half, in one write. + // Read-modify-writing ODR instead would lose a concurrent change from an + // interrupt handler touching another pin on the same port. + PortRegisters(port_)->BSRR = Mask(); + return {}; +} + +auto GpioPin::SetLow() -> std::expected { + if (!configured_) { + return std::unexpected(common::Error::kInvalidState); + } + if (direction_ != PinDirection::kOutput) { + return std::unexpected(common::Error::kInvalidOperation); + } + PortRegisters(port_)->BSRR = Mask() << 16U; + return {}; +} + +auto GpioPin::Toggle() -> std::expected { + if (!configured_) { + return std::unexpected(common::Error::kInvalidState); + } + if (direction_ != PinDirection::kOutput) { + return std::unexpected(common::Error::kInvalidOperation); + } + // Read the current level from ODR (what was commanded, not what the pad + // reads) and write the opposite through BSRR. The GPIO block has no + // atomic toggle, so this is a read-modify-write of one bit -- but the write + // half goes through BSRR, so it cannot disturb the port's other pins. + auto* registers = PortRegisters(port_); + const bool is_high = (registers->ODR & Mask()) != 0U; + registers->BSRR = is_high ? (Mask() << 16U) : Mask(); + return {}; +} + +auto GpioPin::SetInterruptHandler(std::function handler, + PinTransition transition) + -> std::expected { + if (!configured_) { + return std::unexpected(common::Error::kInvalidState); + } + if (direction_ != PinDirection::kInput) { + return std::unexpected(common::Error::kInvalidOperation); + } + return RegisterExtiHandler(port_, pin_, std::move(handler), transition); +} + +} // namespace mcu diff --git a/src/libs/mcu/arm_cm7/gpio_pin.hpp b/src/libs/mcu/arm_cm7/gpio_pin.hpp new file mode 100644 index 0000000..9476cb3 --- /dev/null +++ b/src/libs/mcu/arm_cm7/gpio_pin.hpp @@ -0,0 +1,45 @@ +#pragma once + +#include +#include +#include + +#include "libs/common/error.hpp" +#include "libs/mcu/arm_cm7/gpio_port.hpp" +#include "libs/mcu/pin.hpp" + +namespace mcu { + +/// @brief One STM32 GPIO pin. +/// +/// Construction only records where the pin is; it touches no registers, so a +/// board can hold pins as members and have them constructed before the clock +/// tree is up. Configure() is what makes the pin real, and every operation +/// before it returns kInvalidState rather than writing into a dead register +/// block. +class GpioPin final : public BidirectionalPin { + public: + GpioPin(GpioPort port, std::uint32_t pin) : port_(port), pin_(pin) {} + + [[nodiscard]] auto Configure(PinDirection direction) + -> std::expected override; + + [[nodiscard]] auto Get() -> std::expected override; + [[nodiscard]] auto SetHigh() -> std::expected override; + [[nodiscard]] auto SetLow() -> std::expected override; + [[nodiscard]] auto Toggle() -> std::expected override; + + [[nodiscard]] auto SetInterruptHandler(std::function handler, + PinTransition transition) + -> std::expected override; + + private: + [[nodiscard]] auto Mask() const -> std::uint32_t { return 1U << pin_; } + + GpioPort port_; + std::uint32_t pin_; + PinDirection direction_ = PinDirection::kInput; + bool configured_ = false; +}; + +} // namespace mcu diff --git a/src/libs/mcu/arm_cm7/gpio_port.cpp b/src/libs/mcu/arm_cm7/gpio_port.cpp new file mode 100644 index 0000000..133bfb7 --- /dev/null +++ b/src/libs/mcu/arm_cm7/gpio_port.cpp @@ -0,0 +1,27 @@ +#include "libs/mcu/arm_cm7/gpio_port.hpp" + +#include + +#include "libs/mcu/arm_cm7/cmsis.hpp" +#include "libs/mcu/arm_cm7/gpio_registers.hpp" + +namespace mcu { + +auto PortRegisters(GpioPort port) -> GPIO_TypeDef* { + // The port register blocks are contiguous at 0x400 intervals from GPIOA, + // in the same order as the enumerators. + const auto base = GPIOA_BASE + (static_cast(port) * 0x400UL); + // NOLINTNEXTLINE(performance-no-int-to-ptr) + return reinterpret_cast(base); +} + +auto EnablePortClock(GpioPort port) -> void { + RCC->AHB1ENR |= 1UL << static_cast(port); + + // A peripheral clock enable takes a couple of cycles to take effect. Reading + // the register back stalls until the write has landed; without this, an + // immediately following register write can be dropped. + static_cast(RCC->AHB1ENR); +} + +} // namespace mcu diff --git a/src/libs/mcu/arm_cm7/gpio_port.hpp b/src/libs/mcu/arm_cm7/gpio_port.hpp new file mode 100644 index 0000000..273a69a --- /dev/null +++ b/src/libs/mcu/arm_cm7/gpio_port.hpp @@ -0,0 +1,40 @@ +#pragma once + +#include + +namespace mcu { + +/// @brief Which GPIO port a pin belongs to. +/// +/// A named enum rather than the vendor's `GPIO_TypeDef*` so that this header, +/// and everything that includes it, stays free of CMSIS. That is not tidiness: +/// stm32f767xx.h defines `I2C1`, `USART3` and a few hundred other bare +/// identifiers as macros, and any header that pulls it in silently rewrites +/// matching names in the code around it -- including `board::Board::I2C1()`. +/// cmsis.hpp is included only from .cpp files for that reason. +/// +/// The enumerator values are the port index, which is also the RCC_AHB1ENR bit +/// position and the value SYSCFG_EXTICR wants. +enum class GpioPort : std::uint8_t { + kA = 0, + kB, + kC, + kD, + kE, + kF, + kG, + kH, + kI, + kJ, + kK, +}; + +/// @brief Enable the AHB1 clock for one GPIO port, and wait for it to land. +/// +/// Every GPIO register reads as zero and ignores writes until its port clock +/// is running -- silently, with no fault. This is the most common way to spend +/// an evening on bare-metal GPIO, so nothing here touches a port without +/// calling this first. +auto EnablePortClock(GpioPort port) -> void; + +} // namespace mcu diff --git a/src/libs/mcu/arm_cm7/gpio_registers.hpp b/src/libs/mcu/arm_cm7/gpio_registers.hpp new file mode 100644 index 0000000..71e73ad --- /dev/null +++ b/src/libs/mcu/arm_cm7/gpio_registers.hpp @@ -0,0 +1,15 @@ +#pragma once + +#include "libs/mcu/arm_cm7/cmsis.hpp" +#include "libs/mcu/arm_cm7/gpio_port.hpp" + +namespace mcu { + +/// @brief The register block for a port. +/// +/// Deliberately not in gpio_port.hpp: its return type is a vendor type, so +/// this header may only be included from .cpp files. See the note on macro +/// collisions in gpio_port.hpp. +[[nodiscard]] auto PortRegisters(GpioPort port) -> GPIO_TypeDef*; + +} // namespace mcu diff --git a/src/libs/mcu/arm_cm7/systick.cpp b/src/libs/mcu/arm_cm7/systick.cpp index b8b2750..2d94fc2 100644 --- a/src/libs/mcu/arm_cm7/systick.cpp +++ b/src/libs/mcu/arm_cm7/systick.cpp @@ -27,6 +27,10 @@ extern "C" auto SysTick_Handler() -> void { g_ticks = g_ticks + 1; } auto Millis() -> std::uint32_t { return g_ticks; } +auto SysTickRunning() -> bool { + return (SysTick->CTRL & SysTick_CTRL_ENABLE_Msk) != 0U; +} + auto InitSysTick() -> void { SysTick_Config(kSystemCoreClockHz / kTickRateHz); } } // namespace mcu diff --git a/src/libs/mcu/arm_cm7/systick.hpp b/src/libs/mcu/arm_cm7/systick.hpp index 3e7cebd..0f597b8 100644 --- a/src/libs/mcu/arm_cm7/systick.hpp +++ b/src/libs/mcu/arm_cm7/systick.hpp @@ -10,6 +10,12 @@ namespace mcu { /// unsigned arithmetic makes the wrap harmless. [[nodiscard]] auto Millis() -> std::uint32_t; +/// @brief Whether the tick is running. +/// +/// Anything that waits on Millis() must check this: before InitSysTick(), the +/// counter never advances and a wait for it never ends. +[[nodiscard]] auto SysTickRunning() -> bool; + /// @brief Start the 1 kHz system tick. /// /// Idempotent. Called from the board's Init() rather than from SystemInit, diff --git a/tools/verify-firmware.sh b/tools/verify-firmware.sh index a6aaacc..68fa49b 100755 --- a/tools/verify-firmware.sh +++ b/tools/verify-firmware.sh @@ -68,7 +68,12 @@ check_image() { # property of the code today, and this is what keeps it true: an operator new # reference in these objects means a handler outgrew the buffer. check_no_allocation() { - local pattern='_Znwj|_Znaj|_ZdlPvj' + # operator new only. operator delete is deliberately not in this pattern: + # a polymorphic class's vtable references its deleting destructor, which + # references operator delete, whether or not anything is ever allocated -- + # so every one of these objects has an undefined _ZdlPvj and always will. + # An operator new reference is the thing that means a handler allocated. + local pattern='_Znwj|_Znaj|_Znw|_Zna' local objects mapfile -t objects < <(find "${BUILD_DIR}" -name 'gpio_pin.cpp.obj' \ -o -name 'exti.cpp.obj' \ From 867f57f82349b69eaa5445500340fd0f7aae08c7 Mon Sep 17 00:00:00 2001 From: Nehal Patel Date: Fri, 4 Sep 2026 05:25:28 +0000 Subject: [PATCH 05/12] ci(arm): cross-build job, and docs that match what the board does Adds a second CI job building both configurations for the Nucleo and running tools/verify-firmware.sh on the result. Release is the one that earns its place: -Os -flto with --gc-sections is where a missing KEEP on the vector table or a garbage-collected weak interrupt handler shows up, and neither is visible in a Debug build. The job reuses the image and GHA cache the host job populates, so the toolchain layers are a cache hit. Firmware sizes are recorded as a step and the .elf/.bin/.hex/.map uploaded as artifacts. No size budget: a threshold picked before there are data points is only a number to argue with. docs/HARDWARE.md covers the pin map, flashing, and debugging, and says plainly to flash from the host OS rather than the devcontainer -- ST-LINK USB passthrough into a container is awkward on Linux and unavailable on macOS/Windows, and the mass-storage path needs no tooling at all. README and PROJECT_PLAN said ARM was "not yet functional" and "toolchain and presets only". Both now say what is true, including which peripherals are still placeholders, and Milestone 2's remaining tasks are the ones that need the board in hand. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/ci.yml | 69 ++++++++++++++++++++++++ README.md | 22 +++++--- docs/HARDWARE.md | 112 +++++++++++++++++++++++++++++++++++++++ docs/PROJECT_PLAN.md | 61 ++++++++++++++++----- 4 files changed, 246 insertions(+), 18 deletions(-) create mode 100644 docs/HARDWARE.md diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5403da9..a19cefe 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -113,3 +113,72 @@ jobs: path: | build/host/ccov/**/* if-no-files-found: ignore + + arm-cross-build: + name: ARM Cross-Build (Nucleo F767ZI) + runs-on: ubuntu-latest + + steps: + - name: Checkout code + uses: actions/checkout@v4 + + # The same image, and the same GHA cache the host job populates, so the + # apt/toolchain layers are a cache hit rather than a second build. + - name: Set up Buildx + uses: docker/setup-buildx-action@v3 + + - name: Build dev image + uses: docker/build-push-action@v6 + with: + context: . + load: true + tags: embedded-cpp-docker:latest + cache-from: type=gha + cache-to: type=gha,mode=max + + # Both configurations, deliberately. Release is the one that matters + # here: -Os -flto with --gc-sections is where a missing KEEP on the + # vector table or a garbage-collected weak interrupt handler shows up, + # and neither is visible in a Debug build. + - name: Cross-build (Debug) + run: > + docker compose -f docker-compose.yml -f docker-compose.ci.yml run --rm + --user "$(id -u):$(id -g)" embedded-cpp-dev + cmake --workflow --preset nucleo-f767zi-debug + + - name: Cross-build (Release) + run: > + docker compose -f docker-compose.yml -f docker-compose.ci.yml run --rm + --user "$(id -u):$(id -g)" embedded-cpp-dev + cmake --workflow --preset nucleo-f767zi-release + + # Everything about a firmware image that can be checked without a board: + # the vector table's address, the entry point and its Thumb bit, a + # non-empty .init_array, no undefined symbols, and no allocation on the + # interrupt-handler paths. Same script a developer runs locally. + - name: Verify firmware images + run: > + docker compose -f docker-compose.yml -f docker-compose.ci.yml run --rm + --user "$(id -u):$(id -g)" embedded-cpp-dev + tools/verify-firmware.sh build/nucleo-f767zi + + # Recorded, not gated. A size budget is worth having once there are + # enough data points to know what normal looks like; picking a threshold + # now would only be a number to argue with. + - name: Firmware size report + run: > + docker compose -f docker-compose.yml -f docker-compose.ci.yml run --rm + --user "$(id -u):$(id -g)" embedded-cpp-dev + sh -c 'arm-none-eabi-size -A build/nucleo-f767zi/bin/*/*.elf' + + - name: Upload firmware + if: always() + uses: actions/upload-artifact@v4 + with: + name: firmware-nucleo-f767zi + path: | + build/nucleo-f767zi/bin/**/*.elf + build/nucleo-f767zi/bin/**/*.bin + build/nucleo-f767zi/bin/**/*.hex + build/nucleo-f767zi/**/*.map + if-no-files-found: ignore diff --git a/README.md b/README.md index ff129b3..c59ca3b 100644 --- a/README.md +++ b/README.md @@ -56,10 +56,18 @@ Application (apps/) → Board (libs/board/) → MCU (libs/mcu/) → Platfo cmake --workflow --preset=host-debug cmake --workflow --preset=host-release -# ARM targets - not yet functional (see Implementation Status below). -# Toolchain files and configure presets are in place, but configuring stops -# with a clear message until the MCU layer lands in src/libs/mcu/arm_cm4/ -# (and arm_cm7/ for Cortex-M7 parts). +# STM32F767ZI Nucleo (Cortex-M7). Configure and build only: firmware has no +# tests that run on the build machine, so nothing here runs ctest. See +# docs/HARDWARE.md for flashing, debugging and the pin map. +cmake --workflow --preset=nucleo-f767zi-debug +cmake --workflow --preset=nucleo-f767zi-release + +# Structural checks on the linked image -- vector table address, entry point, +# static-constructor array, undefined symbols. No hardware needed; CI runs it. +tools/verify-firmware.sh build/nucleo-f767zi + +# Other ARM presets are toolchain-only: no arm_cm4 backend or F3 Discovery +# board exists yet, so configuring stops with a message naming what does. cmake --preset=stm32f3_discovery ``` @@ -114,8 +122,10 @@ cd py/host-emulator && uv run host-emulator | Python integration tests | ✅ Working | | Docker/DevContainer | ✅ Working | | CI/CD | ✅ Working | -| ARM cross-compile toolchain | 🚧 Toolchain/presets only | -| Hardware boards (STM32, nRF52) | 📋 Planned | +| ARM cross-compile (Cortex-M7) | ✅ Working | +| STM32F767ZI Nucleo: GPIO, EXTI, SysTick | ✅ Working | +| STM32F767ZI Nucleo: UART, I2C | 🚧 Placeholders that return an error | +| Other boards (STM32F3, nRF52) | 📋 Planned | ## Resources diff --git a/docs/HARDWARE.md b/docs/HARDWARE.md new file mode 100644 index 0000000..b4ae481 --- /dev/null +++ b/docs/HARDWARE.md @@ -0,0 +1,112 @@ +# Hardware: STM32F767ZI Nucleo-144 + +The first physical target. Everything below assumes the board's ST-LINK USB +connector (CN1), which provides power, flashing and a virtual COM port over one +cable. + +## Build + +```bash +cmake --workflow --preset=nucleo-f767zi-debug +cmake --workflow --preset=nucleo-f767zi-release +``` + +Both produce `.elf`, `.bin` and `.hex` under +`build/nucleo-f767zi/bin//`, plus a `.map` beside the build tree. + +Configure and build only: firmware has no tests that run on the build machine. +What can be checked without a board is checked by: + +```bash +tools/verify-firmware.sh build/nucleo-f767zi +``` + +which asserts the vector table sits at `0x08000000`, the entry point is +`Reset_Handler` with the Thumb bit set, `.init_array` is non-empty (so +namespace-scope constructors actually run), nothing is undefined, and the +interrupt-handler paths do not allocate. CI runs the same script. + +## Flashing + +**Flash from the host OS, not from the devcontainer.** Reaching an ST-LINK +from inside a container needs USB passthrough that is awkward on Linux and +effectively unavailable on macOS and Windows. Build in the container, flash +outside it — the build tree is on the shared checkout either way. + +The board enumerates as a USB mass-storage volume named `NODE_F767ZI`. Copying +a raw binary onto it flashes the chip: + +```bash +cp build/nucleo-f767zi/bin/Debug/blinky.bin /media/$USER/NODE_F767ZI/ && sync +``` + +The `sync` is not optional: without it the file can sit in the page cache and +nothing is written. If a copy appears to succeed but the board does not change +behaviour, update the ST-LINK firmware (STSW-LINK007) before suspecting the +code — older V2-1 firmware can accept the write and discard it. + +Alternatives, both installed in the dev image: + +```bash +st-flash write build/nucleo-f767zi/bin/Debug/blinky.bin 0x8000000 +openocd -f board/st_nucleo_f7.cfg \ + -c "program build/nucleo-f767zi/bin/Debug/blinky.elf verify reset exit" +``` + +## Debugging + +`gdb-multiarch` stands in for `arm-none-eabi-gdb`, which Ubuntu does not +package. Point your IDE's `gdbPath` at it. + +```bash +openocd -f board/st_nucleo_f7.cfg # terminal 1 +gdb-multiarch build/nucleo-f767zi/bin/Debug/blinky.elf +(gdb) target extended-remote :3333 +(gdb) monitor reset halt +(gdb) load +``` + +## Pin map + +The authority is `src/libs/board/stm32f767zi_nucleo/pin_map.hpp`; this table +is for reading. Source: UM1974, *STM32 Nucleo-144 boards*. + +| Function | Pin | Notes | +|---|---|---| +| `UserLed1()` | PB0 | LD1, green | +| `UserLed2()` | PB7 | LD2, blue | +| — | PB14 | LD3, red; not yet exposed through `board::Board` | +| `UserButton1()` | PC13 | B1, blue. Externally pulled **down**: reads high while pressed, so a press is a rising edge | +| `Uart1()` | PD8 (TX), PD9 (RX) | USART3, AF7, wired to the ST-LINK virtual COM port — appears as `/dev/ttyACM0` | +| `I2C1()` | PB8 (SCL), PB9 (SDA) | AF4, Arduino D15/D14 | + +## What works today + +`blinky` is the end-to-end test: LD1 toggles every 200 ms, and pressing B1 +lights LD2 from an interrupt handler. + +`Uart1()` and `I2C1()` are still placeholders that return +`Error::kInvalidOperation`, so `uart_echo` and `i2c_demo` link and run but +fail at their first peripheral call. See `unimplemented_peripherals.hpp`. + +## Clock configuration + +The chip runs from HSI at 16 MHz — its reset configuration, with no PLL, no +instruction or data cache, and no ART accelerator. This is deliberate. Each of +those is a change that can break peripherals that currently work (enabling the +D-cache in particular interacts with DMA buffers and with the DTCM/SRAM1 +distinction), and each is worth making on its own against a baseline that is +known good. + +`mcu::Delay` and `mcu::Millis` both assume 16 MHz. Raising the clock means +updating `kSystemCoreClockHz` in `systick.cpp` and `delay.cpp` together. + +## Memory + +512 KB of RAM as one contiguous region: DTCM 128 KB at `0x20000000`, SRAM1 +368 KB at `0x20020000`, SRAM2 16 KB at `0x2007C000`. The linker script treats +them as one because they are address-contiguous. Split them if DMA ever needs +to avoid DTCM, which DMA can reach only through the CPU's AHBS slave port +(RM0410). + +Flash is 2 MB at `0x08000000`. Current usage, Release: under 6 KB. diff --git a/docs/PROJECT_PLAN.md b/docs/PROJECT_PLAN.md index 4968768..aa03fbc 100644 --- a/docs/PROJECT_PLAN.md +++ b/docs/PROJECT_PLAN.md @@ -49,16 +49,26 @@ systems through: **Goal**: First physical board (STM32F7 Nucleo) -The repository currently carries only the ARM toolchain files and configure -presets; the vendor HAL trees that briefly lived in-tree were removed while -unreachable (git history preserves them, and CubeMX regenerates them fresher). +The backend, the board and a linked firmware image exist; the drivers are +written against CMSIS register definitions rather than the Cube HAL, so the +clock ordering and bit layouts stay visible in the code. Hardware verification +is the open part. **Tasks**: -- [ ] Implement the `arm_cm7` MCU backend (`src/libs/mcu/arm_cm7/`) -- [ ] Implement the STM32F7 Nucleo board directory (pin maps, GPIO init, - interrupt wiring) against the vendor HAL -- [ ] Verify blinky builds, flashes, and runs on the physical board -- [ ] Document hardware setup: pin mapping tables, flashing, debugging +- [x] Implement the `arm_cm7` MCU backend (`src/libs/mcu/arm_cm7/`) +- [x] Implement the STM32F7 Nucleo board directory (pin map, GPIO, EXTI, + SysTick) against CMSIS register definitions +- [x] Cross-compile verification without hardware: image layout, entry point, + static-constructor array, undefined symbols, no allocation on interrupt + paths (`tools/verify-firmware.sh`, run in CI) +- [x] Document hardware setup: pin mapping tables, flashing, debugging + (`docs/HARDWARE.md`) +- [ ] Verify blinky flashes and runs on the physical board +- [ ] USART3 to the ST-LINK virtual COM port, replacing the placeholder +- [ ] I2C1 on PB8/PB9, replacing the placeholder +- [ ] Replace `nosys.specs` with real newlib syscalls once there is a UART to + retarget `_write` to, and a `_sbrk` bounded by the linker script's + `__heap_limit` **Success Criteria**: - Blinky runs on a physical STM32F7 Nucleo board @@ -90,9 +100,11 @@ unreachable (git history preserves them, and CubeMX regenerates them fresher). ## Current Priorities -1. **Complete STM32F7 Nucleo board** (Milestone 2) — proves hardware - portability, builds on the completed host foundation -2. **STM32F3 Discovery board** (Milestone 3) — proves multi-board portability +1. **Verify blinky on the physical F767ZI**, then USART3 and I2C1 + (Milestone 2) — the backend and board exist and the image is verified by + cross-build; what remains needs the board in hand +2. **STM32F3 Discovery board** (Milestone 3) — proves multi-board portability, + and is the first data point for narrowing `board::Board` 3. **Additional example applications** — more engaging demonstrations ## Technical Debt & Improvements @@ -101,7 +113,6 @@ unreachable (git history preserves them, and CubeMX regenerates them fresher). - [ ] Add hardware setup guides and architecture diagrams - [ ] Optimize Docker layer caching - [ ] Add release builds to CI -- [ ] Cross-compilation verification in CI (once an ARM backend exists) - [ ] Host emulator: GUI visualization, richer I2C device models, timing simulation - [ ] Wire up Python test coverage if it earns its keep (pytest-cov was @@ -109,6 +120,32 @@ unreachable (git history preserves them, and CubeMX regenerates them fresher). ## Decision Log +### 2026-09-04: First hardware backend (arm_cm7 + F767ZI Nucleo) + +- Chose CMSIS device headers over the Cube HAL. The 30k lines of register + #defines are a mechanical transcription of RM0410 and teach nothing, but + RCC enable ordering, the MODER/OTYPER/AFR layout and the EXTI/SYSCFG/NVIC + chain are exactly what `HAL_GPIO_Init()` hides -- and full CubeF7 is ~1 GB + of middleware for a project that wants the registers visible +- No public header in the backend names a vendor type. CMSIS defines `I2C1`, + `USART3` and hundreds of other bare identifiers as macros; the first one to + bite rewrote `board::Board::I2C1()` into a syntax error. Pins name their + port with `mcu::GpioPort` instead, and `cmsis.hpp` is included only from + .cpp files +- Wrote the linker script rather than recovering the Ac6 one from history: + that file forbids redistribution, sizes RAM at 320K (F746 numbers, not the + F767ZI's 512K), and discards every section of libc.a, libm.a and libgcc.a +- Kept `startup.s` from history (ST, BSD-3) for its 110-entry vector table, + with the contradictory `.fpu softvfp` removed +- Stayed on the distro's `gcc-arm-none-eabi` 13.2. Verified that every + portable header and app source compiles at `-std=c++23`; libstdc++ 13's + missing `` affects only two host-only translation units +- `-fno-exceptions` is now real, and cross-build-only: the host entry point + is an exception boundary by design because cppzmq throws +- Deferred the PLL, the caches and the ART accelerator. Each can break + working peripherals, and each deserves a known-good baseline to regress + against + ### 2026-08-29: Simplification pass - Codebase-wide review against the project's educational goals; the themes: one canonical form per idea (a single Transact/Peripheral implementation From 89368c42fd1d15d7ead427dd3525e2179ec601eb Mon Sep 17 00:00:00 2001 From: Nehal Patel Date: Fri, 4 Sep 2026 14:05:21 +0000 Subject: [PATCH 06/12] docs(hardware): st-flash needs --reset, and probe first Without --reset, st-flash writes the image, verifies it, reports "Go to Thumb mode" and leaves the core halted. The write succeeds in every visible way and the board does nothing, so a correct image is indistinguishable from a broken one -- which is the worst possible failure mode for the first thing anyone does with this board. Found the way these things usually are: blinky was flashed, verified, and sat there. Co-Authored-By: Claude Opus 5 (1M context) --- docs/HARDWARE.md | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/docs/HARDWARE.md b/docs/HARDWARE.md index b4ae481..982e7e4 100644 --- a/docs/HARDWARE.md +++ b/docs/HARDWARE.md @@ -48,7 +48,23 @@ code — older V2-1 firmware can accept the write and discard it. Alternatives, both installed in the dev image: ```bash -st-flash write build/nucleo-f767zi/bin/Debug/blinky.bin 0x8000000 +st-info --probe # expect chipid 0x451, dev-type STM32F76x_F77x +st-flash --reset write build/nucleo-f767zi/bin/Debug/blinky.bin 0x8000000 +``` + +`--reset` is not optional. Without it `st-flash` writes and verifies happily, +reports "Go to Thumb mode", and leaves the core halted -- so a correct image +looks exactly like a broken one. The `0x8000000` is likewise mandatory: a raw +binary carries no load address. The `.hex` does, if you would rather not type +it: + +```bash +st-flash --reset --format ihex write build/nucleo-f767zi/bin/Debug/blinky.hex +``` + +Or via OpenOCD, whose `reset` verb covers the same ground: + +```bash openocd -f board/st_nucleo_f7.cfg \ -c "program build/nucleo-f767zi/bin/Debug/blinky.elf verify reset exit" ``` From 22709e56777f2fc528ed31ee2d9d84bee5a8c94c Mon Sep 17 00:00:00 2001 From: Nehal Patel Date: Fri, 4 Sep 2026 14:06:52 +0000 Subject: [PATCH 07/12] docs: blinky verified end to end on the F767ZI LD1 toggles and B1 latches LD2 from the EXTI handler, running unmodified blinky.cpp -- the same source the host emulator runs, with zero changes under src/apps/. That closes Milestone 2's hardware task and makes the layered design's portability claim a demonstrated one. The button in particular exercises what static checks could not: the SYSCFG/NVIC routing, clearing EXTI->PR before dispatch, and a stored std::function actually invoked from interrupt context -- confirming at runtime the small-buffer assumption that tools/verify-firmware.sh only checks in the symbol table. Co-Authored-By: Claude Opus 5 (1M context) --- docs/PROJECT_PLAN.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/PROJECT_PLAN.md b/docs/PROJECT_PLAN.md index aa03fbc..5af1904 100644 --- a/docs/PROJECT_PLAN.md +++ b/docs/PROJECT_PLAN.md @@ -63,7 +63,9 @@ is the open part. paths (`tools/verify-firmware.sh`, run in CI) - [x] Document hardware setup: pin mapping tables, flashing, debugging (`docs/HARDWARE.md`) -- [ ] Verify blinky flashes and runs on the physical board +- [x] Verify blinky flashes and runs on the physical board — LD1 toggles + and B1 latches LD2 from an EXTI handler, from unmodified + `blinky.cpp` (2026-09-04) - [ ] USART3 to the ST-LINK virtual COM port, replacing the placeholder - [ ] I2C1 on PB8/PB9, replacing the placeholder - [ ] Replace `nosys.specs` with real newlib syscalls once there is a UART to From abc2509a458d7c5877e13453b33a60968f71cead Mon Sep 17 00:00:00 2001 From: Nehal Patel Date: Fri, 4 Sep 2026 14:08:04 +0000 Subject: [PATCH 08/12] docs: blink period measured at 200 ms, closing Milestone 2 bring-up 25 ON-transitions in 10 s. Each ON is a full ON/OFF/ON cycle -- two toggles -- so the cycle is 400 ms and each Delay(200ms) is 200 ms, within the ~4% resolution of counting by eye. That confirms the SysTick divisor and the 16 MHz HSI assumption both systick.cpp and delay.cpp hardcode. HARDWARE.md now states the expected count and why it is 25 rather than 50: blinks and toggles differ by a factor of two, and reading it the wrong way makes a correct board look like it is running at half speed. Co-Authored-By: Claude Opus 5 (1M context) --- docs/HARDWARE.md | 8 +++++++- docs/PROJECT_PLAN.md | 4 ++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/docs/HARDWARE.md b/docs/HARDWARE.md index 982e7e4..14ecd64 100644 --- a/docs/HARDWARE.md +++ b/docs/HARDWARE.md @@ -99,7 +99,13 @@ is for reading. Source: UM1974, *STM32 Nucleo-144 boards*. ## What works today `blinky` is the end-to-end test: LD1 toggles every 200 ms, and pressing B1 -lights LD2 from an interrupt handler. +lights LD2 from an interrupt handler (which only ever calls `SetHigh()`, so +LD2 latches on and stays lit). + +To check the timing without a scope, count LD1's ON transitions over 10 s and +expect **25**. Each ON is a full ON→OFF→ON cycle, which is *two* toggles of +200 ms each — so 50 would mean the delay is running at half its intended +length, not that it is correct. `Uart1()` and `I2C1()` are still placeholders that return `Error::kInvalidOperation`, so `uart_echo` and `i2c_demo` link and run but diff --git a/docs/PROJECT_PLAN.md b/docs/PROJECT_PLAN.md index 5af1904..89f4c95 100644 --- a/docs/PROJECT_PLAN.md +++ b/docs/PROJECT_PLAN.md @@ -63,8 +63,8 @@ is the open part. paths (`tools/verify-firmware.sh`, run in CI) - [x] Document hardware setup: pin mapping tables, flashing, debugging (`docs/HARDWARE.md`) -- [x] Verify blinky flashes and runs on the physical board — LD1 toggles - and B1 latches LD2 from an EXTI handler, from unmodified +- [x] Verify blinky flashes and runs on the physical board — LD1 toggles at a + measured 200 ms and B1 latches LD2 from an EXTI handler, from unmodified `blinky.cpp` (2026-09-04) - [ ] USART3 to the ST-LINK virtual COM port, replacing the placeholder - [ ] I2C1 on PB8/PB9, replacing the placeholder From d29de4798b003724e242db1e4aa26c85d5c94699 Mon Sep 17 00:00:00 2001 From: Nehal Patel Date: Fri, 4 Sep 2026 14:16:00 +0000 Subject: [PATCH 09/12] feat(arm): USART3 on the VCP, and real newlib syscalls Implements mcu::Usart against the F7's USART registers and wires Uart1() to USART3 on PD8/PD9 AF7, which the Nucleo routes to the ST-LINK virtual COM port. Blocking Send and Receive poll ISR; SetRxHandler goes through the RXNE interrupt into the same handler-table pattern exti.cpp uses. UnimplementedUart is deleted. Alternate-function mode is not expressible in mcu::PinDirection, and should not be -- it is a peripheral-driver concern, not an application one. Pin configuration now goes through ConfigurePin(), which writes OSPEEDR, PUPDR, OTYPER and AFR before MODER, so a pin never spends a cycle driving through whichever peripheral AF0 happens to be. GpioPin::Configure delegates to it rather than keeping a second MODER write. nosys.specs is gone; the board provides its own syscalls. _write reaches the console USART with CRLF expansion, and _sbrk is bounded by the linker script's __heap_limit, so exhausting the heap returns ENOMEM instead of handing out addresses the stack is about to grow into. The one non-obvious piece: newlib-nano ships __malloc_lock and __malloc_unlock as `bx lr`. That is correct for a single-threaded program and wrong here, because uart_echo's receive handler builds a std::vector -- it allocates in interrupt context. A USART interrupt arriving while the main context was inside malloc would re-enter the allocator and corrupt its arena, surfacing as a fault somewhere unrelated much later. Both are now overridden to mask interrupts, counting nesting and restoring the previous PRIMASK rather than assuming it was clear. Two details worth recording. Word length counts the parity bit, so 8 data bits with parity needs a 9-bit word; Init rejects 9-plus-parity, which the peripheral cannot express. Send waits for TC, not just TXE, so returning and immediately reconfiguring cannot truncate the last character. USART3 is a CMSIS macro, so nothing in usart.hpp names a vendor type -- the instance is selected by mcu::UsartId. Same rule that gpio_port.hpp records. Hardware verification pending: builds and image checks only. Co-Authored-By: Claude Opus 5 (1M context) --- cmake/toolchain/armgcc.cmake | 17 +- docs/HARDWARE.md | 32 +- docs/PROJECT_PLAN.md | 10 +- .../board/stm32f767zi_nucleo/CMakeLists.txt | 4 +- .../board/stm32f767zi_nucleo/nucleo_board.hpp | 13 +- src/libs/board/stm32f767zi_nucleo/pin_map.hpp | 4 +- .../board/stm32f767zi_nucleo/syscalls.cpp | 150 +++++++++ .../unimplemented_peripherals.hpp | 29 +- src/libs/mcu/arm_cm7/CMakeLists.txt | 5 +- src/libs/mcu/arm_cm7/gpio_pin.cpp | 31 +- src/libs/mcu/arm_cm7/gpio_port.cpp | 45 +++ src/libs/mcu/arm_cm7/gpio_port.hpp | 47 +++ src/libs/mcu/arm_cm7/usart.cpp | 294 ++++++++++++++++++ src/libs/mcu/arm_cm7/usart.hpp | 71 +++++ 14 files changed, 681 insertions(+), 71 deletions(-) create mode 100644 src/libs/board/stm32f767zi_nucleo/syscalls.cpp create mode 100644 src/libs/mcu/arm_cm7/usart.cpp create mode 100644 src/libs/mcu/arm_cm7/usart.hpp diff --git a/cmake/toolchain/armgcc.cmake b/cmake/toolchain/armgcc.cmake index a461e80..1b6e050 100644 --- a/cmake/toolchain/armgcc.cmake +++ b/cmake/toolchain/armgcc.cmake @@ -44,13 +44,16 @@ set(CMAKE_ASM_OPTIONS "-x assembler-with-cpp") set(CMAKE_C_FLAGS_INIT "${CMAKE_COMMON_FLAGS}") set(CMAKE_CXX_FLAGS_INIT "${CMAKE_COMMON_FLAGS}") set(CMAKE_ASM_FLAGS_INIT "${CMAKE_COMMON_FLAGS} ${CMAKE_ASM_OPTIONS}") -# nano.specs selects newlib-nano; nosys.specs supplies stub implementations of -# the syscalls it expects (_sbrk, _write, _close, ...) so a board backend links -# before it has written its own. Note _sbrk is not optional even in a design -# that never calls new: a polymorphic class's vtable references its deleting -# destructor, which references operator delete. A board replaces nosys with a -# real syscalls translation unit once it has a UART to write to. -set(CMAKE_EXE_LINKER_FLAGS_INIT "--specs=nano.specs --specs=nosys.specs -Wl,--gc-sections,-print-memory-usage,--no-warn-rwx-segments") +# nano.specs selects newlib-nano. The syscalls it expects (_sbrk, _write, ...) +# are the board's to provide -- see stm32f767zi_nucleo/syscalls.cpp. nosys.specs +# would supply stubs that link but always fail, which is the right scaffolding +# for a board that has no console yet and the wrong thing to leave in place +# once it does: a _write that silently discards output is worse than none. +# +# Note _sbrk is not optional even in a design that never calls new: a +# polymorphic class's vtable references its deleting destructor, which +# references operator delete, which pulls in newlib's malloc arena. +set(CMAKE_EXE_LINKER_FLAGS_INIT "--specs=nano.specs -Wl,--gc-sections,-print-memory-usage,--no-warn-rwx-segments") # Firmware images, not host executables. Makes blinky.elf and blinky.bin # unambiguous in the build tree and in flashing instructions. diff --git a/docs/HARDWARE.md b/docs/HARDWARE.md index 14ecd64..c530c30 100644 --- a/docs/HARDWARE.md +++ b/docs/HARDWARE.md @@ -107,9 +107,35 @@ expect **25**. Each ON is a full ON→OFF→ON cycle, which is *two* toggles of 200 ms each — so 50 would mean the delay is running at half its intended length, not that it is correct. -`Uart1()` and `I2C1()` are still placeholders that return -`Error::kInvalidOperation`, so `uart_echo` and `i2c_demo` link and run but -fail at their first peripheral call. See `unimplemented_peripherals.hpp`. +`uart_echo` is the second: it greets over the ST-LINK virtual COM port and +echoes what you type, toggling LD1 per byte received. + +```bash +st-flash --reset write build/nucleo-f767zi/bin/Debug/uart_echo.bin 0x8000000 +screen /dev/ttyACM0 115200 # or: picocom -b 115200 /dev/ttyACM0 +``` + +`printf` and friends also reach this port: the board's `_write` (see +`syscalls.cpp`) retargets stdout and stderr to USART3, expanding `\n` to CRLF. +Output written before `Uart1().Init()` is discarded rather than blocking. + +`I2C1()` is still a placeholder that returns `Error::kInvalidOperation`, so +`i2c_demo` links and runs but fails at its first peripheral call. See +`unimplemented_peripherals.hpp`. + +### A note on allocation in interrupt handlers + +`uart_echo`'s receive handler builds a `std::vector` — that is, it allocates, +in interrupt context. newlib-nano ships `__malloc_lock` as a no-op, so this +would re-enter the allocator whenever an interrupt arrived while the main +context was inside `malloc`, corrupting the arena in a way that surfaces as a +fault somewhere unrelated much later. `syscalls.cpp` overrides both lock +functions to mask interrupts around the allocator, which makes the pattern +safe here. + +The handler also calls `Send()`, which busy-waits for the transmitter. At +115200 baud that is roughly 87 µs per byte spent inside an ISR. Fine for an +echo demo; not a pattern to copy into anything with latency requirements. ## Clock configuration diff --git a/docs/PROJECT_PLAN.md b/docs/PROJECT_PLAN.md index 89f4c95..100b1fd 100644 --- a/docs/PROJECT_PLAN.md +++ b/docs/PROJECT_PLAN.md @@ -66,11 +66,13 @@ is the open part. - [x] Verify blinky flashes and runs on the physical board — LD1 toggles at a measured 200 ms and B1 latches LD2 from an EXTI handler, from unmodified `blinky.cpp` (2026-09-04) -- [ ] USART3 to the ST-LINK virtual COM port, replacing the placeholder +- [x] USART3 to the ST-LINK virtual COM port, replacing the placeholder - [ ] I2C1 on PB8/PB9, replacing the placeholder -- [ ] Replace `nosys.specs` with real newlib syscalls once there is a UART to - retarget `_write` to, and a `_sbrk` bounded by the linker script's - `__heap_limit` +- [x] Replace `nosys.specs` with real newlib syscalls: `_write` retargeted to + USART3, `_sbrk` bounded by the linker script's `__heap_limit`, and + `__malloc_lock` overridden so an allocating interrupt handler cannot + re-enter the allocator +- [ ] Verify `uart_echo` over the virtual COM port on hardware **Success Criteria**: - Blinky runs on a physical STM32F7 Nucleo board diff --git a/src/libs/board/stm32f767zi_nucleo/CMakeLists.txt b/src/libs/board/stm32f767zi_nucleo/CMakeLists.txt index 65f54c3..87446cd 100644 --- a/src/libs/board/stm32f767zi_nucleo/CMakeLists.txt +++ b/src/libs/board/stm32f767zi_nucleo/CMakeLists.txt @@ -12,8 +12,8 @@ target_link_libraries(nucleo_board # each application directly: pulling them out of a static archive depends on # link-order symbol resolution and breaks under --gc-sections and LTO, and a # vector table nothing references is exactly what --gc-sections removes. -add_library(platform_entry OBJECT main.cpp startup.s) -target_link_libraries(platform_entry PRIVATE project_options app board nucleo_board) +add_library(platform_entry OBJECT main.cpp startup.s syscalls.cpp) +target_link_libraries(platform_entry PRIVATE project_options app board nucleo_board arm_cm7_mcu) set(LINKER_SCRIPT "${CMAKE_CURRENT_SOURCE_DIR}/stm32f767zi.ld") diff --git a/src/libs/board/stm32f767zi_nucleo/nucleo_board.hpp b/src/libs/board/stm32f767zi_nucleo/nucleo_board.hpp index 6f28367..53c0c80 100644 --- a/src/libs/board/stm32f767zi_nucleo/nucleo_board.hpp +++ b/src/libs/board/stm32f767zi_nucleo/nucleo_board.hpp @@ -7,6 +7,7 @@ #include "libs/board/stm32f767zi_nucleo/unimplemented_peripherals.hpp" #include "libs/common/error.hpp" #include "libs/mcu/arm_cm7/gpio_pin.hpp" +#include "libs/mcu/arm_cm7/usart.hpp" #include "libs/mcu/i2c.hpp" #include "libs/mcu/pin.hpp" #include "libs/mcu/uart.hpp" @@ -38,10 +39,18 @@ class NucleoF767ZiBoard final : public Board { mcu::GpioPin user_button_1_{pin_map::kUserButton1.port, pin_map::kUserButton1.pin}; - // Placeholders until each peripheral's hardware implementation lands. See + mcu::Usart uart_1_{mcu::UsartId::kUsart3, + { + .tx_port = pin_map::kUart1Tx.port, + .tx_pin = pin_map::kUart1Tx.pin, + .rx_port = pin_map::kUart1Rx.port, + .rx_pin = pin_map::kUart1Rx.pin, + .alternate_function = pin_map::kUart1AlternateFunction, + }}; + + // Placeholder until the I2C implementation lands. See // unimplemented_peripherals.hpp. UnimplementedI2CController i2c_1_; - UnimplementedUart uart_1_; }; } // namespace board diff --git a/src/libs/board/stm32f767zi_nucleo/pin_map.hpp b/src/libs/board/stm32f767zi_nucleo/pin_map.hpp index 5c625b1..5f04e69 100644 --- a/src/libs/board/stm32f767zi_nucleo/pin_map.hpp +++ b/src/libs/board/stm32f767zi_nucleo/pin_map.hpp @@ -30,11 +30,11 @@ constexpr PinLocation kUserButton1{mcu::GpioPort::kC, 13}; // output shows up on /dev/ttyACM0. Alternate function 7. constexpr PinLocation kUart1Tx{mcu::GpioPort::kD, 8}; constexpr PinLocation kUart1Rx{mcu::GpioPort::kD, 9}; -constexpr std::uint32_t kUart1AlternateFunction = 7; +constexpr std::uint8_t kUart1AlternateFunction = 7; // I2C1 on the Arduino connector: D15 (SCL) and D14 (SDA). Alternate function 4. constexpr PinLocation kI2C1Scl{mcu::GpioPort::kB, 8}; constexpr PinLocation kI2C1Sda{mcu::GpioPort::kB, 9}; -constexpr std::uint32_t kI2C1AlternateFunction = 4; +constexpr std::uint8_t kI2C1AlternateFunction = 4; } // namespace board::pin_map diff --git a/src/libs/board/stm32f767zi_nucleo/syscalls.cpp b/src/libs/board/stm32f767zi_nucleo/syscalls.cpp new file mode 100644 index 0000000..75ecf95 --- /dev/null +++ b/src/libs/board/stm32f767zi_nucleo/syscalls.cpp @@ -0,0 +1,150 @@ +/// @file +/// The newlib syscalls this board provides, replacing --specs=nosys.specs. +/// +/// newlib-nano expects a small set of POSIX-shaped functions to exist. Most of +/// them have no meaning on a board with no filesystem and no processes, and +/// exist only so the C library links; the two that do real work are _write, +/// which reaches the console USART, and _sbrk, which hands out the heap. +/// +/// A heap is not optional here even though nothing in this project calls new: +/// a polymorphic class's vtable references its deleting destructor, which +/// references operator delete, which pulls in free and newlib's malloc arena. +/// Some applications do allocate -- uart_echo builds a std::vector in its +/// receive handler and a std::string for its greeting. + +#include +#include +#include + +#include "libs/mcu/arm_cm7/cmsis.hpp" +#include "libs/mcu/arm_cm7/usart.hpp" + +namespace { + +// Defined by the linker script (stm32f767zi.ld). `end` is the first address +// past .bss; __heap_limit leaves the stack its reserved space at the top of +// RAM. Declared as arrays so their *addresses* are the values -- a linker +// symbol has no storage to load from. +extern "C" char end[]; // NOLINT(modernize-avoid-c-arrays) +extern "C" char __heap_limit[]; // NOLINT(modernize-avoid-c-arrays) + +char* g_heap_break = nullptr; + +// Guards newlib's allocator against re-entry from an interrupt handler; see +// __malloc_lock below. +std::uint32_t g_malloc_lock_depth = 0; +std::uint32_t g_saved_primask = 0; + +} // namespace + +extern "C" { + +/// Grow the heap. Bounded, unlike the usual vendor implementation: exceeding +/// the limit returns an allocation failure rather than quietly handing out +/// addresses that the stack is about to grow down into, which corrupts memory +/// far from the code responsible and long after the fact. +auto _sbrk(ptrdiff_t increment) -> void* { + if (g_heap_break == nullptr) { + g_heap_break = static_cast(end); + } + + char* const previous = g_heap_break; + if (increment > 0 && (__heap_limit - g_heap_break) < increment) { + errno = ENOMEM; + return reinterpret_cast(-1); // NOLINT(performance-no-int-to-ptr) + } + g_heap_break += increment; + return previous; +} + +/// stdout and stderr go to the console USART; anything else is a bad file +/// descriptor. Newlines are expanded to CRLF because terminal emulators on the +/// other end of the virtual COM port expect it. +auto _write(int file, const char* data, int length) -> int { + if (file != 1 && file != 2) { + errno = EBADF; + return -1; + } + for (int i = 0; i < length; ++i) { + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic) + const char value = data[i]; + if (value == '\n') { + static_cast(mcu::PutcharToConsole('\r')); + } + if (!mcu::PutcharToConsole(value)) { + // No console yet. Report the write as complete rather than failing: + // output before Uart1().Init() is diagnostics, and a failing write there + // turns startup logging into an error path. + return length; + } + } + return length; +} + +// No filesystem. These exist so newlib links; every one reports the honest +// answer for a target with no files, rather than pretending to succeed. +auto _read(int /*file*/, char* /*data*/, int /*length*/) -> int { return 0; } +auto _close(int /*file*/) -> int { return -1; } +auto _lseek(int /*file*/, int /*offset*/, int /*whence*/) -> int { return 0; } + +/// Claiming character-device status keeps newlib's stdout unbuffered, which is +/// what you want when the next thing after a printf may be a hard fault. +auto _isatty(int /*file*/) -> int { return 1; } + +auto _fstat(int /*file*/, struct stat* /*st*/) -> int { + return 0; // st_mode is left alone; _isatty is what newlib actually consults. +} + +auto _getpid() -> int { return 1; } + +auto _kill(int /*pid*/, int /*sig*/) -> int { + errno = EINVAL; + return -1; +} + +/// Nothing to exit to. Stop with interrupts off, so a debugger attaches to a +/// halted core rather than one still servicing timers. +[[noreturn]] auto _exit(int /*status*/) -> void { + __disable_irq(); + while (true) { + } +} + +/// newlib-nano ships __malloc_lock and __malloc_unlock as `bx lr` -- correct +/// for a single-threaded program, and wrong the moment an interrupt handler +/// allocates. uart_echo's receive handler builds a std::vector, so a USART +/// interrupt arriving while the main context is inside malloc would re-enter +/// the allocator and corrupt its arena: a fault later, somewhere unrelated. +/// +/// Masking interrupts around the allocator is the standard fix and costs +/// nothing when uncontended. Nesting is counted, and the previous PRIMASK +/// restored rather than assumed clear, so a call from a context that already +/// had interrupts disabled does not silently enable them on the way out. +auto __malloc_lock(struct _reent* /*reent*/) -> void { + const std::uint32_t primask = __get_PRIMASK(); + __disable_irq(); + if (g_malloc_lock_depth == 0) { + g_saved_primask = primask; + } + ++g_malloc_lock_depth; +} + +auto __malloc_unlock(struct _reent* /*reent*/) -> void { + if (g_malloc_lock_depth > 0) { + --g_malloc_lock_depth; + } + if (g_malloc_lock_depth == 0 && (g_saved_primask & 1U) == 0U) { + __enable_irq(); + } +} + +/// Called when a pure virtual is invoked -- during construction or after +/// destruction of a base. newlib's default drags in std::terminate's +/// machinery; this is smaller and stops at a known address. +[[noreturn]] auto __cxa_pure_virtual() -> void { + __disable_irq(); + while (true) { + } +} + +} // extern "C" diff --git a/src/libs/board/stm32f767zi_nucleo/unimplemented_peripherals.hpp b/src/libs/board/stm32f767zi_nucleo/unimplemented_peripherals.hpp index 234a6ca..439704c 100644 --- a/src/libs/board/stm32f767zi_nucleo/unimplemented_peripherals.hpp +++ b/src/libs/board/stm32f767zi_nucleo/unimplemented_peripherals.hpp @@ -25,39 +25,14 @@ /// application that reaches for an unimplemented peripheral gets an error at /// the call, not silence. They are deleted as the real implementations land. /// -/// These have a delete-by date: B7 removes UnimplementedUart, B8 -/// UnimplementedI2CController. (UnimplementedPin is gone: B4 landed.) A stub +/// These have a delete-by date: B8 removes UnimplementedI2CController, the +/// last one. (UnimplementedPin went with B4, UnimplementedUart with B7.) A stub /// still here after B8 has stopped being bring-up scaffolding and become /// evidence of a separate problem -- that board::Board cannot express "this /// board does not have that peripheral" (see Milestone 3 in /// docs/PROJECT_PLAN.md). namespace board { -class UnimplementedUart final : public mcu::Uart { - public: - [[nodiscard]] auto Init(const mcu::UartConfig& /*config*/) - -> std::expected override { - return std::unexpected(common::Error::kInvalidOperation); - } - - [[nodiscard]] auto Send(std::span /*data*/) - -> std::expected override { - return std::unexpected(common::Error::kInvalidOperation); - } - - [[nodiscard]] auto Receive(std::span /*buffer*/, - std::uint32_t /*timeout_ms*/) - -> std::expected override { - return std::unexpected(common::Error::kInvalidOperation); - } - - [[nodiscard]] auto SetRxHandler( - std::function /*handler*/) - -> std::expected override { - return std::unexpected(common::Error::kInvalidOperation); - } -}; - class UnimplementedI2CController final : public mcu::I2CController { public: [[nodiscard]] auto SendData(std::uint16_t /*address*/, diff --git a/src/libs/mcu/arm_cm7/CMakeLists.txt b/src/libs/mcu/arm_cm7/CMakeLists.txt index 4bd80c5..41fe65a 100644 --- a/src/libs/mcu/arm_cm7/CMakeLists.txt +++ b/src/libs/mcu/arm_cm7/CMakeLists.txt @@ -6,12 +6,13 @@ # The filenames carry the distinction; split the directory when an arm_cm4 # backend arrives and wants to share the GPIO code. add_library(arm_cm7_mcu - cortex_m7.cpp systick.cpp delay.cpp gpio_pin.cpp gpio_port.cpp exti.cpp) + cortex_m7.cpp systick.cpp delay.cpp gpio_pin.cpp gpio_port.cpp exti.cpp usart.cpp) target_sources(arm_cm7_mcu PUBLIC FILE_SET HEADERS BASE_DIRS ${PROJECT_SOURCE_DIR}/src FILES cmsis.hpp cortex_m7.hpp systick.hpp - gpio_pin.hpp gpio_port.hpp gpio_registers.hpp exti.hpp) + gpio_pin.hpp gpio_port.hpp gpio_registers.hpp exti.hpp + usart.hpp) # cmsis_f767 is PUBLIC because cmsis.hpp, a public header, includes it. target_link_libraries(arm_cm7_mcu PUBLIC mcu cmsis_f767 diff --git a/src/libs/mcu/arm_cm7/gpio_pin.cpp b/src/libs/mcu/arm_cm7/gpio_pin.cpp index 1847f83..36b1b7c 100644 --- a/src/libs/mcu/arm_cm7/gpio_pin.cpp +++ b/src/libs/mcu/arm_cm7/gpio_pin.cpp @@ -1,6 +1,5 @@ #include "libs/mcu/arm_cm7/gpio_pin.hpp" -#include #include #include #include @@ -14,29 +13,17 @@ namespace mcu { -namespace { - -constexpr std::uint32_t kModeInput = 0b00; -constexpr std::uint32_t kModeOutput = 0b01; -constexpr std::uint32_t kModeFieldWidth = 2; - -} // namespace - auto GpioPin::Configure(PinDirection direction) -> std::expected { - // Before the port clock is running, every register here reads as zero and - // ignores writes, without faulting. - EnablePortClock(port_); - - const std::uint32_t shift = pin_ * kModeFieldWidth; - const std::uint32_t mode = - direction == PinDirection::kOutput ? kModeOutput : kModeInput; - - auto* registers = PortRegisters(port_); - auto moder = registers->MODER; - moder &= ~(0b11U << shift); - moder |= mode << shift; - registers->MODER = moder; + // ConfigurePin enables the port clock first, which matters: before it is + // running every register here reads as zero and ignores writes, silently. + ConfigurePin(port_, pin_, + { + .mode = direction == PinDirection::kOutput ? PinMode::kOutput + : PinMode::kInput, + .speed = PinSpeed::kLow, + .pull = PinPull::kNone, + }); direction_ = direction; configured_ = true; diff --git a/src/libs/mcu/arm_cm7/gpio_port.cpp b/src/libs/mcu/arm_cm7/gpio_port.cpp index 133bfb7..bd09697 100644 --- a/src/libs/mcu/arm_cm7/gpio_port.cpp +++ b/src/libs/mcu/arm_cm7/gpio_port.cpp @@ -15,6 +15,51 @@ auto PortRegisters(GpioPort port) -> GPIO_TypeDef* { return reinterpret_cast(base); } +namespace { + +/// Write a `width`-bit field for `pin` into a register whose fields are packed +/// one per pin. MODER, OSPEEDR and PUPDR are two bits per pin; AFR is four, +/// split across two words. +auto WriteField(volatile std::uint32_t& reg, std::uint32_t shift, + std::uint32_t width, std::uint32_t value) -> void { + const std::uint32_t mask = ((1UL << width) - 1UL) << shift; + reg = (reg & ~mask) | ((value << shift) & mask); +} + +} // namespace + +auto ConfigurePin(GpioPort port, std::uint32_t pin, + const PinConfig& config) -> void { + EnablePortClock(port); + auto* registers = PortRegisters(port); + + const std::uint32_t two_bit_shift = pin * 2UL; + + WriteField(registers->OSPEEDR, two_bit_shift, 2, + static_cast(config.speed)); + WriteField(registers->PUPDR, two_bit_shift, 2, + static_cast(config.pull)); + + if (config.output_type == OutputType::kOpenDrain) { + registers->OTYPER |= 1UL << pin; + } else { + registers->OTYPER &= ~(1UL << pin); + } + + if (config.mode == PinMode::kAlternate) { + // AFR[0] covers pins 0-7, AFR[1] pins 8-15, four bits each. Must be set + // before MODER selects alternate mode, or the pin briefly drives through + // whichever peripheral AF0 happens to be. + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-constant-array-index) + WriteField(registers->AFR[pin / 8UL], (pin % 8UL) * 4UL, 4, + config.alternate_function); + } + + // MODER last: it is what actually hands the pin over. + WriteField(registers->MODER, two_bit_shift, 2, + static_cast(config.mode)); +} + auto EnablePortClock(GpioPort port) -> void { RCC->AHB1ENR |= 1UL << static_cast(port); diff --git a/src/libs/mcu/arm_cm7/gpio_port.hpp b/src/libs/mcu/arm_cm7/gpio_port.hpp index 273a69a..d559280 100644 --- a/src/libs/mcu/arm_cm7/gpio_port.hpp +++ b/src/libs/mcu/arm_cm7/gpio_port.hpp @@ -29,6 +29,53 @@ enum class GpioPort : std::uint8_t { kK, }; +/// @brief What a pin is wired to. MODER field values. +/// +/// The portable mcu::PinDirection has only input and output, which is the +/// right vocabulary for an application. A peripheral driver needs more: a +/// USART or I2C pin is in alternate-function mode, driven by the peripheral +/// rather than by software. That distinction is backend-private, so it lives +/// here rather than in libs/mcu/pin.hpp. +enum class PinMode : std::uint8_t { + kInput = 0b00, + kOutput = 0b01, + kAlternate = 0b10, + kAnalog = 0b11, +}; + +enum class OutputType : std::uint8_t { kPushPull = 0, kOpenDrain = 1 }; + +enum class PinSpeed : std::uint8_t { + kLow = 0b00, + kMedium = 0b01, + kHigh = 0b10, + kVeryHigh = 0b11, +}; + +enum class PinPull : std::uint8_t { kNone = 0b00, kUp = 0b01, kDown = 0b10 }; + +/// @brief Everything MODER/OTYPER/OSPEEDR/PUPDR/AFR say about one pin. +/// +/// Defaults are a plain input, which is also the chip's reset state, so a +/// caller only names what it needs to differ. +struct PinConfig { + PinMode mode = PinMode::kInput; + OutputType output_type = OutputType::kPushPull; + PinSpeed speed = PinSpeed::kLow; + PinPull pull = PinPull::kNone; + /// Only consulted when mode is kAlternate. AF0-AF15; see the part datasheet's + /// alternate function mapping table, not the reference manual. + std::uint8_t alternate_function = 0; +}; + +/// @brief Apply a configuration to one pin, enabling its port clock first. +/// +/// Writes the five registers in the order the reference manual's examples use: +/// everything else before MODER, so the pin never spends a cycle driving with +/// a stale output type or speed. +auto ConfigurePin(GpioPort port, std::uint32_t pin, + const PinConfig& config) -> void; + /// @brief Enable the AHB1 clock for one GPIO port, and wait for it to land. /// /// Every GPIO register reads as zero and ignores writes until its port clock diff --git a/src/libs/mcu/arm_cm7/usart.cpp b/src/libs/mcu/arm_cm7/usart.cpp new file mode 100644 index 0000000..839fc92 --- /dev/null +++ b/src/libs/mcu/arm_cm7/usart.cpp @@ -0,0 +1,294 @@ +#include "libs/mcu/arm_cm7/usart.hpp" + +#include +#include +#include +#include +#include +#include +#include + +#include "libs/common/error.hpp" +#include "libs/mcu/arm_cm7/cmsis.hpp" +#include "libs/mcu/arm_cm7/gpio_port.hpp" +#include "libs/mcu/arm_cm7/systick.hpp" +#include "libs/mcu/uart.hpp" + +namespace mcu { + +namespace { + +// APB1 and APB2 both run at the core clock in the reset configuration (HSI +// 16 MHz, all prescalers at 1). USART1 and USART6 are on APB2, the rest on +// APB1; the distinction only starts mattering once the PLL and the bus +// prescalers are configured. +constexpr std::uint32_t kPeripheralClockHz = 16'000'000; + +constexpr std::size_t kUsartCount = 4; + +[[nodiscard]] auto Index(UsartId id) -> std::size_t { + return static_cast(id); +} + +[[nodiscard]] auto Registers(UsartId id) -> USART_TypeDef* { + switch (id) { + case UsartId::kUsart1: + return USART1; + case UsartId::kUsart2: + return USART2; + case UsartId::kUsart3: + return USART3; + case UsartId::kUsart6: + return USART6; + } + return USART3; +} + +[[nodiscard]] auto IrqNumber(UsartId id) -> IRQn_Type { + switch (id) { + case UsartId::kUsart1: + return USART1_IRQn; + case UsartId::kUsart2: + return USART2_IRQn; + case UsartId::kUsart3: + return USART3_IRQn; + case UsartId::kUsart6: + return USART6_IRQn; + } + return USART3_IRQn; +} + +auto EnablePeripheralClock(UsartId id) -> void { + switch (id) { + case UsartId::kUsart1: + RCC->APB2ENR |= RCC_APB2ENR_USART1EN; + static_cast(RCC->APB2ENR); + break; + case UsartId::kUsart6: + RCC->APB2ENR |= RCC_APB2ENR_USART6EN; + static_cast(RCC->APB2ENR); + break; + case UsartId::kUsart2: + RCC->APB1ENR |= RCC_APB1ENR_USART2EN; + static_cast(RCC->APB1ENR); + break; + case UsartId::kUsart3: + RCC->APB1ENR |= RCC_APB1ENR_USART3EN; + static_cast(RCC->APB1ENR); + break; + } +} + +/// Word length is M1:M0, and it counts the parity bit. Enabling parity on an +/// 8-bit format therefore needs a 9-bit word, or the parity bit would eat the +/// top data bit -- the single easiest thing to get wrong here. +[[nodiscard]] auto WordLengthBits(const UartConfig& config) + -> std::expected { + auto data_bits = 0U; + switch (config.data_bits) { + case UartConfig::DataBits::k7Bits: + data_bits = 7; + break; + case UartConfig::DataBits::k8Bits: + data_bits = 8; + break; + case UartConfig::DataBits::k9Bits: + data_bits = 9; + break; + } + const auto total = + data_bits + (config.parity == UartConfig::Parity::kNone ? 0U : 1U); + + switch (total) { + case 7: + return USART_CR1_M1; // M1:M0 = 10 + case 8: + return 0U; // M1:M0 = 00 + case 9: + return USART_CR1_M0; // M1:M0 = 01 + default: + // 9 data bits plus parity is 10, which the peripheral cannot express. + return std::unexpected(common::Error::kInvalidArgument); + } +} + +// The USART newlib's _write talks to, and a flag for whether it is usable. +// Set by Init; read from _write, which cannot be handed a C++ object. +USART_TypeDef* g_console = nullptr; + +struct RxSlot { + std::function handler; +}; + +// One per instance, namespace scope so an interrupt can reach it before any +// function-local static would have been initialized. +std::array g_rx_slots; + +auto ServiceRx(UsartId id) -> void { + auto* registers = Registers(id); + + // Overrun sets RXNE's companion flag and, left alone, wedges the receiver. + // Clearing it discards the byte that was lost, which has already happened. + if ((registers->ISR & USART_ISR_ORE) != 0U) { + registers->ICR = USART_ICR_ORECF; + } + + while ((registers->ISR & USART_ISR_RXNE) != 0U) { + // Reading RDR is what clears RXNE. + const auto value = static_cast(registers->RDR & 0xFFU); + const auto& slot = g_rx_slots.at(Index(id)); + if (slot.handler) { + slot.handler(&value, 1); + } + } +} + +} // namespace + +auto Usart::Init(const UartConfig& config) + -> std::expected { + if (initialized_) { + return std::unexpected(common::Error::kInvalidState); + } + if (config.baud_rate == 0) { + return std::unexpected(common::Error::kInvalidArgument); + } + // XON/XOFF is a software protocol; the peripheral has no bit for it, and + // silently ignoring the request would be worse than refusing it. + if (config.flow_control == UartConfig::FlowControl::kXonXoff) { + return std::unexpected(common::Error::kInvalidArgument); + } + + const auto word_length = WordLengthBits(config); + if (!word_length) { + return std::unexpected(word_length.error()); + } + + EnablePeripheralClock(id_); + + const PinConfig pin{ + .mode = PinMode::kAlternate, + .output_type = OutputType::kPushPull, + .speed = PinSpeed::kVeryHigh, + .pull = PinPull::kUp, + .alternate_function = pins_.alternate_function, + }; + ConfigurePin(pins_.tx_port, pins_.tx_pin, pin); + ConfigurePin(pins_.rx_port, pins_.rx_pin, pin); + + auto* registers = Registers(id_); + + // Every control bit but UE must be written while the USART is disabled. + registers->CR1 = 0; + + registers->BRR = + (kPeripheralClockHz + (config.baud_rate / 2U)) / config.baud_rate; + + registers->CR2 = + config.stop_bits == UartConfig::StopBits::k2Bits ? USART_CR2_STOP_1 : 0U; + + registers->CR3 = config.flow_control == UartConfig::FlowControl::kRtsCts + ? (USART_CR3_RTSE | USART_CR3_CTSE) + : 0U; + + std::uint32_t cr1 = *word_length | USART_CR1_TE | USART_CR1_RE; + if (config.parity != UartConfig::Parity::kNone) { + cr1 |= USART_CR1_PCE; + if (config.parity == UartConfig::Parity::kOdd) { + cr1 |= USART_CR1_PS; + } + } + registers->CR1 = cr1 | USART_CR1_UE; + + initialized_ = true; + g_console = registers; + return {}; +} + +auto Usart::Send(std::span data) + -> std::expected { + if (!initialized_) { + return std::unexpected(common::Error::kInvalidState); + } + + auto* registers = Registers(id_); + for (const auto value : data) { + while ((registers->ISR & USART_ISR_TXE) == 0U) { + } + registers->TDR = std::to_integer(value); + } + + // Wait for the last byte to leave the shift register, not just the holding + // register. Without this, returning from Send and immediately resetting or + // reconfiguring the peripheral truncates the final character. + while ((registers->ISR & USART_ISR_TC) == 0U) { + } + return {}; +} + +auto Usart::Receive(std::span buffer, std::uint32_t timeout_ms) + -> std::expected { + if (!initialized_) { + return std::unexpected(common::Error::kInvalidState); + } + if (buffer.empty()) { + return 0U; + } + + auto* registers = Registers(id_); + const std::uint32_t start = Millis(); + std::size_t received = 0; + + while (received < buffer.size()) { + if ((registers->ISR & USART_ISR_ORE) != 0U) { + registers->ICR = USART_ICR_ORECF; + } + + if ((registers->ISR & USART_ISR_RXNE) != 0U) { + buffer[received] = static_cast(registers->RDR & 0xFFU); + ++received; + continue; + } + + // timeout_ms == 0 means wait forever, per the interface contract. + if (timeout_ms != 0 && (Millis() - start) > timeout_ms) { + return received > 0 ? std::expected{received} + : std::unexpected(common::Error::kTimeout); + } + } + + return received; +} + +auto Usart::SetRxHandler(std::function + handler) -> std::expected { + if (!initialized_) { + return std::unexpected(common::Error::kInvalidState); + } + + auto* registers = Registers(id_); + g_rx_slots.at(Index(id_)).handler = std::move(handler); + + registers->CR1 |= USART_CR1_RXNEIE; + NVIC_SetPriority(IrqNumber(id_), 5); + NVIC_EnableIRQ(IrqNumber(id_)); + return {}; +} + +auto PutcharToConsole(char value) -> bool { + if (g_console == nullptr) { + return false; + } + while ((g_console->ISR & USART_ISR_TXE) == 0U) { + } + g_console->TDR = + static_cast(static_cast(value)); + return true; +} + +extern "C" auto USART1_IRQHandler() -> void { ServiceRx(UsartId::kUsart1); } +extern "C" auto USART2_IRQHandler() -> void { ServiceRx(UsartId::kUsart2); } +extern "C" auto USART3_IRQHandler() -> void { ServiceRx(UsartId::kUsart3); } +extern "C" auto USART6_IRQHandler() -> void { ServiceRx(UsartId::kUsart6); } + +} // namespace mcu diff --git a/src/libs/mcu/arm_cm7/usart.hpp b/src/libs/mcu/arm_cm7/usart.hpp new file mode 100644 index 0000000..d081409 --- /dev/null +++ b/src/libs/mcu/arm_cm7/usart.hpp @@ -0,0 +1,71 @@ +#pragma once + +#include +#include +#include +#include +#include + +#include "libs/common/error.hpp" +#include "libs/mcu/arm_cm7/gpio_port.hpp" +#include "libs/mcu/uart.hpp" + +namespace mcu { + +/// @brief Which USART/UART instance. Values are indices into the driver's +/// internal tables, not addresses. +/// +/// Named `kUsart3` rather than `USART3` deliberately: CMSIS defines `USART3` +/// as an object-like macro, so a public header that used the bare name would +/// rewrite it wherever it appeared downstream. Nothing in this header names a +/// vendor type; see the note in gpio_port.hpp. +enum class UsartId : std::uint8_t { kUsart1, kUsart2, kUsart3, kUsart6 }; + +/// @brief Where a USART's two pins are, and which alternate function selects +/// the peripheral on them. +struct UsartPins { + GpioPort tx_port; + std::uint32_t tx_pin; + GpioPort rx_port; + std::uint32_t rx_pin; + std::uint8_t alternate_function; +}; + +/// @brief Blocking USART, with an optional receive interrupt. +/// +/// Send and Receive poll the status register; there is no transmit buffering, +/// so Send returns once the last byte has left the shift register and the line +/// is idle. That is the honest shape for this peripheral until there is a +/// reason for more -- see the note on async modes in libs/mcu/uart.hpp. +class Usart final : public Uart { + public: + Usart(UsartId id, const UsartPins& pins) : id_(id), pins_(pins) {} + + [[nodiscard]] auto Init(const UartConfig& config) + -> std::expected override; + + [[nodiscard]] auto Send(std::span data) + -> std::expected override; + + [[nodiscard]] auto Receive(std::span buffer, + std::uint32_t timeout_ms) + -> std::expected override; + + [[nodiscard]] auto SetRxHandler( + std::function handler) + -> std::expected override; + + private: + UsartId id_; + UsartPins pins_; + bool initialized_ = false; +}; + +/// @brief Write one byte to whichever USART was last initialized. +/// +/// Exists for newlib's _write, which has no way to reach a C++ object. Returns +/// false if no USART is up yet, so early output is dropped rather than +/// hanging on a peripheral that will never assert TXE. +auto PutcharToConsole(char value) -> bool; + +} // namespace mcu From 4509a459cf768fcbb21b6db7fd6d0ad13252bbe9 Mon Sep 17 00:00:00 2001 From: Nehal Patel Date: Fri, 4 Sep 2026 14:25:03 +0000 Subject: [PATCH 10/12] fix(arm): make Usart::Send atomic against its own receive handler A handler that echoes calls Send from interrupt context -- which is exactly what uart_echo does. If that preempts a Send already in progress, the two byte streams interleave on the wire, and the outer Send's next TDR write clears TC, so the inner one's completion wait can outlast the byte it was waiting for. Typing during the greeting is enough to hit it. Masks this USART's receive interrupt for the duration rather than all interrupts: other peripherals keep interrupting, and a byte arriving meanwhile still sets RXNE, so it is delivered when the flag is restored rather than lost. Called from the handler itself it is a no-op, which is correct -- an interrupt cannot preempt itself. Also documents two things about the terminal that look like faults and are not: local echo is off by default, so everything on screen came from the board and one "hello" for a typed "hello" is a working echo; and the greeting usually appears twice, because st-flash --reset boots the board into the ST-LINK's USB buffer and opening the port resets it again. uart_echo is verified on hardware: greeting and per-character echo at 115200. Co-Authored-By: Claude Opus 5 (1M context) --- docs/HARDWARE.md | 18 +++++++++++++++++- docs/PROJECT_PLAN.md | 3 ++- src/libs/mcu/arm_cm7/usart.cpp | 21 +++++++++++++++++++++ 3 files changed, 40 insertions(+), 2 deletions(-) diff --git a/docs/HARDWARE.md b/docs/HARDWARE.md index c530c30..425470e 100644 --- a/docs/HARDWARE.md +++ b/docs/HARDWARE.md @@ -112,9 +112,25 @@ echoes what you type, toggling LD1 per byte received. ```bash st-flash --reset write build/nucleo-f767zi/bin/Debug/uart_echo.bin 0x8000000 -screen /dev/ttyACM0 115200 # or: picocom -b 115200 /dev/ttyACM0 +picocom -b 115200 --imap lfcrlf,crcrlf /dev/ttyACM0 ``` +Two things about reading that terminal, both of which look like faults and are +not: + +- **Terminal emulators default to local echo off**, so what you type is not + displayed. Every character on screen came from the board. Typing `hello` and + seeing `hello` once means all five made the round trip — seeing it *twice* + would mean something is echoing twice. +- **The greeting will often appear twice.** `st-flash --reset` boots the board + and its greeting sits in the ST-LINK's USB buffer with nobody attached; + opening the port then toggles DTR, resetting the board, so you get the + buffered greeting and a fresh one. + +`--imap` is worth the typing: the board sends bytes verbatim, so the greeting's +bare `\n` walks the cursor diagonally and an echoed Enter (a bare CR) makes +later output overwrite the same line. + `printf` and friends also reach this port: the board's `_write` (see `syscalls.cpp`) retargets stdout and stderr to USART3, expanding `\n` to CRLF. Output written before `Uart1().Init()` is discarded rather than blocking. diff --git a/docs/PROJECT_PLAN.md b/docs/PROJECT_PLAN.md index 100b1fd..63b1016 100644 --- a/docs/PROJECT_PLAN.md +++ b/docs/PROJECT_PLAN.md @@ -72,7 +72,8 @@ is the open part. USART3, `_sbrk` bounded by the linker script's `__heap_limit`, and `__malloc_lock` overridden so an allocating interrupt handler cannot re-enter the allocator -- [ ] Verify `uart_echo` over the virtual COM port on hardware +- [x] Verify `uart_echo` over the virtual COM port on hardware — greeting + and per-character echo confirmed at 115200 (2026-09-04) **Success Criteria**: - Blinky runs on a physical STM32F7 Nucleo board diff --git a/src/libs/mcu/arm_cm7/usart.cpp b/src/libs/mcu/arm_cm7/usart.cpp index 839fc92..d3a3068 100644 --- a/src/libs/mcu/arm_cm7/usart.cpp +++ b/src/libs/mcu/arm_cm7/usart.cpp @@ -212,6 +212,23 @@ auto Usart::Send(std::span data) } auto* registers = Registers(id_); + + // Send must be atomic with respect to its own receive handler. A handler + // that echoes -- which is exactly what uart_echo does -- calls Send from + // interrupt context, and if that preempts a Send already in progress the two + // byte streams interleave on the wire. Worse, the outer Send's next TDR + // write clears TC, so the inner one's completion wait can outlast the byte + // it was waiting for. + // + // Masking just this USART's interrupt, rather than all of them, keeps the + // window narrow: other peripherals keep interrupting, and a byte arriving + // meanwhile still sets RXNE, so it is delivered as soon as the flag is + // restored rather than lost. Called from the handler itself this is a no-op, + // which is correct -- an interrupt cannot preempt itself. + const bool rx_interrupt_was_enabled = + (registers->CR1 & USART_CR1_RXNEIE) != 0U; + registers->CR1 &= ~USART_CR1_RXNEIE; + for (const auto value : data) { while ((registers->ISR & USART_ISR_TXE) == 0U) { } @@ -223,6 +240,10 @@ auto Usart::Send(std::span data) // reconfiguring the peripheral truncates the final character. while ((registers->ISR & USART_ISR_TC) == 0U) { } + + if (rx_interrupt_was_enabled) { + registers->CR1 |= USART_CR1_RXNEIE; + } return {}; } From bc412f3c9755b2b077992f11acd2fad7eb90685e Mon Sep 17 00:00:00 2001 From: Nehal Patel Date: Fri, 4 Sep 2026 14:28:18 +0000 Subject: [PATCH 11/12] feat(arm): I2C1 on PB8/PB9, and the last placeholder is gone Implements mcu::I2CBus against the F7's v2 I2C peripheral and wires I2C1() to PB8/PB9 AF4 (Arduino D15/D14). unimplemented_peripherals.hpp is deleted: every board::Board accessor now returns real hardware. Timing is TIMINGR's five fields, not the old CCR divisor. The constant for 100 kHz from a 16 MHz I2CCLK is ST's, but the derivation is in the comment so it can be checked rather than trusted: PRESC 3 gives a 250 ns tick, SCLL 0x13 and SCLH 0x0F give 5.00 and 4.00 us, and 9 us plus rise and fall is a 10 us period. It is invalidated by raising the core clock, which is said where the constant is. Transfers use autoend, so NBYTES is programmed up front and the hardware issues STOP itself. Waits are bounded by a 25 ms timeout, because a bus held low by a stuck device never sets any completion flag -- and a NACK is reported as kOperationFailed distinctly from a timeout, since an unanswered address means a missing device rather than a wedged bus. Pins are open drain: I2C is wire-AND, and a push-pull driver would fight anything else pulling low. The board brings the bus up in Init(), unlike Uart which the application configures itself, because mcu::I2CController has no Init() in the portable interface. Hardware verification pending. Without a device on the bus the NACK path is still a real test: SendData should return kOperationFailed inside the timeout and i2c_demo should retry rather than hang. Co-Authored-By: Claude Opus 5 (1M context) --- docs/HARDWARE.md | 22 +- docs/PROJECT_PLAN.md | 4 +- .../board/stm32f767zi_nucleo/CMakeLists.txt | 2 +- .../board/stm32f767zi_nucleo/nucleo_board.cpp | 5 +- .../board/stm32f767zi_nucleo/nucleo_board.hpp | 13 +- .../unimplemented_peripherals.hpp | 51 ----- src/libs/mcu/arm_cm7/CMakeLists.txt | 4 +- src/libs/mcu/arm_cm7/i2c.cpp | 200 ++++++++++++++++++ src/libs/mcu/arm_cm7/i2c.hpp | 57 +++++ 9 files changed, 295 insertions(+), 63 deletions(-) delete mode 100644 src/libs/board/stm32f767zi_nucleo/unimplemented_peripherals.hpp create mode 100644 src/libs/mcu/arm_cm7/i2c.cpp create mode 100644 src/libs/mcu/arm_cm7/i2c.hpp diff --git a/docs/HARDWARE.md b/docs/HARDWARE.md index 425470e..21ece4b 100644 --- a/docs/HARDWARE.md +++ b/docs/HARDWARE.md @@ -135,9 +135,25 @@ later output overwrite the same line. `syscalls.cpp`) retargets stdout and stderr to USART3, expanding `\n` to CRLF. Output written before `Uart1().Init()` is discarded rather than blocking. -`I2C1()` is still a placeholder that returns `Error::kInvalidOperation`, so -`i2c_demo` links and runs but fails at its first peripheral call. See -`unimplemented_peripherals.hpp`. +`i2c_demo` is the third: it writes `DE AD BE EF` to address 0x50, reads it +back, and drives LD1 by whether the round trip matched. + +```bash +st-flash --reset write build/nucleo-f767zi/bin/Debug/i2c_demo.bin 0x8000000 +``` + +With nothing on the bus this is still a real test of the driver: an +unanswered address NACKs, `SendData` returns `kOperationFailed` within the +25 ms transfer timeout, and the demo turns LD1 off and retries rather than +hanging. A scope or analyser on PB8/PB9 shows START, the address, and the NACK. + +With a device at 0x50 (a 24Cxx EEPROM, say) the round trip completes. Note the +demo writes and reads as two separate transactions rather than the +register-addressed read a real EEPROM driver would use — the bus is exercised, +the device's addressing is not. + +The bus runs at 100 kHz on the internal pull-ups, which are weak (~40 kΩ). +Adequate for short wiring; a real bus wants external resistors. ### A note on allocation in interrupt handlers diff --git a/docs/PROJECT_PLAN.md b/docs/PROJECT_PLAN.md index 63b1016..d2fe480 100644 --- a/docs/PROJECT_PLAN.md +++ b/docs/PROJECT_PLAN.md @@ -67,7 +67,9 @@ is the open part. measured 200 ms and B1 latches LD2 from an EXTI handler, from unmodified `blinky.cpp` (2026-09-04) - [x] USART3 to the ST-LINK virtual COM port, replacing the placeholder -- [ ] I2C1 on PB8/PB9, replacing the placeholder +- [x] I2C1 on PB8/PB9, replacing the last placeholder +- [ ] Verify `i2c_demo` on hardware (NACK path without a device; round trip + with one) - [x] Replace `nosys.specs` with real newlib syscalls: `_write` retargeted to USART3, `_sbrk` bounded by the linker script's `__heap_limit`, and `__malloc_lock` overridden so an allocating interrupt handler cannot diff --git a/src/libs/board/stm32f767zi_nucleo/CMakeLists.txt b/src/libs/board/stm32f767zi_nucleo/CMakeLists.txt index 87446cd..f924b09 100644 --- a/src/libs/board/stm32f767zi_nucleo/CMakeLists.txt +++ b/src/libs/board/stm32f767zi_nucleo/CMakeLists.txt @@ -2,7 +2,7 @@ add_library(nucleo_board nucleo_board.cpp) target_sources(nucleo_board PUBLIC FILE_SET HEADERS BASE_DIRS ${PROJECT_SOURCE_DIR}/src - FILES nucleo_board.hpp pin_map.hpp unimplemented_peripherals.hpp) + FILES nucleo_board.hpp pin_map.hpp) target_link_libraries(nucleo_board PUBLIC board mcu arm_cm7_mcu PRIVATE project_options) diff --git a/src/libs/board/stm32f767zi_nucleo/nucleo_board.cpp b/src/libs/board/stm32f767zi_nucleo/nucleo_board.cpp index 7090b7e..a6f6119 100644 --- a/src/libs/board/stm32f767zi_nucleo/nucleo_board.cpp +++ b/src/libs/board/stm32f767zi_nucleo/nucleo_board.cpp @@ -23,7 +23,10 @@ auto NucleoF767ZiBoard::Init() -> std::expected { [this] { return user_led_2_.Configure(mcu::PinDirection::kOutput); }) .and_then([this] { return user_button_1_.Configure(mcu::PinDirection::kInput); - }); + }) + // I2C has no Init() in the portable interface -- unlike Uart, which the + // application configures itself -- so the board brings the bus up here. + .and_then([this] { return i2c_1_.Init(); }); } auto NucleoF767ZiBoard::UserLed1() -> mcu::OutputPin& { return user_led_1_; } diff --git a/src/libs/board/stm32f767zi_nucleo/nucleo_board.hpp b/src/libs/board/stm32f767zi_nucleo/nucleo_board.hpp index 53c0c80..0800cf6 100644 --- a/src/libs/board/stm32f767zi_nucleo/nucleo_board.hpp +++ b/src/libs/board/stm32f767zi_nucleo/nucleo_board.hpp @@ -4,9 +4,9 @@ #include "libs/board/board.hpp" #include "libs/board/stm32f767zi_nucleo/pin_map.hpp" -#include "libs/board/stm32f767zi_nucleo/unimplemented_peripherals.hpp" #include "libs/common/error.hpp" #include "libs/mcu/arm_cm7/gpio_pin.hpp" +#include "libs/mcu/arm_cm7/i2c.hpp" #include "libs/mcu/arm_cm7/usart.hpp" #include "libs/mcu/i2c.hpp" #include "libs/mcu/pin.hpp" @@ -48,9 +48,14 @@ class NucleoF767ZiBoard final : public Board { .alternate_function = pin_map::kUart1AlternateFunction, }}; - // Placeholder until the I2C implementation lands. See - // unimplemented_peripherals.hpp. - UnimplementedI2CController i2c_1_; + mcu::I2CBus i2c_1_{mcu::I2CId::kI2C1, + { + .scl_port = pin_map::kI2C1Scl.port, + .scl_pin = pin_map::kI2C1Scl.pin, + .sda_port = pin_map::kI2C1Sda.port, + .sda_pin = pin_map::kI2C1Sda.pin, + .alternate_function = pin_map::kI2C1AlternateFunction, + }}; }; } // namespace board diff --git a/src/libs/board/stm32f767zi_nucleo/unimplemented_peripherals.hpp b/src/libs/board/stm32f767zi_nucleo/unimplemented_peripherals.hpp deleted file mode 100644 index 439704c..0000000 --- a/src/libs/board/stm32f767zi_nucleo/unimplemented_peripherals.hpp +++ /dev/null @@ -1,51 +0,0 @@ -#pragma once - -#include -#include -#include -#include -#include -#include - -#include "libs/common/error.hpp" -#include "libs/mcu/i2c.hpp" -#include "libs/mcu/pin.hpp" -#include "libs/mcu/uart.hpp" - -/// @file -/// Placeholders for the peripherals this board has not implemented yet. -/// -/// board::Board is an all-or-nothing interface: a board that implements none -/// of it does not compile, and one that implements only pins cannot be -/// constructed. These stubs let the board satisfy the interface from the -/// first commit, so every application links and the hardware bring-up can -/// proceed one peripheral at a time instead of all at once. -/// -/// Each returns kInvalidOperation rather than pretending to succeed: an -/// application that reaches for an unimplemented peripheral gets an error at -/// the call, not silence. They are deleted as the real implementations land. -/// -/// These have a delete-by date: B8 removes UnimplementedI2CController, the -/// last one. (UnimplementedPin went with B4, UnimplementedUart with B7.) A stub -/// still here after B8 has stopped being bring-up scaffolding and become -/// evidence of a separate problem -- that board::Board cannot express "this -/// board does not have that peripheral" (see Milestone 3 in -/// docs/PROJECT_PLAN.md). -namespace board { - -class UnimplementedI2CController final : public mcu::I2CController { - public: - [[nodiscard]] auto SendData(std::uint16_t /*address*/, - std::span /*data*/) - -> std::expected override { - return std::unexpected(common::Error::kInvalidOperation); - } - - [[nodiscard]] auto ReceiveData(std::uint16_t /*address*/, - std::span /*buffer*/) - -> std::expected override { - return std::unexpected(common::Error::kInvalidOperation); - } -}; - -} // namespace board diff --git a/src/libs/mcu/arm_cm7/CMakeLists.txt b/src/libs/mcu/arm_cm7/CMakeLists.txt index 41fe65a..8fc0b8b 100644 --- a/src/libs/mcu/arm_cm7/CMakeLists.txt +++ b/src/libs/mcu/arm_cm7/CMakeLists.txt @@ -6,13 +6,13 @@ # The filenames carry the distinction; split the directory when an arm_cm4 # backend arrives and wants to share the GPIO code. add_library(arm_cm7_mcu - cortex_m7.cpp systick.cpp delay.cpp gpio_pin.cpp gpio_port.cpp exti.cpp usart.cpp) + cortex_m7.cpp systick.cpp delay.cpp gpio_pin.cpp gpio_port.cpp exti.cpp usart.cpp i2c.cpp) target_sources(arm_cm7_mcu PUBLIC FILE_SET HEADERS BASE_DIRS ${PROJECT_SOURCE_DIR}/src FILES cmsis.hpp cortex_m7.hpp systick.hpp gpio_pin.hpp gpio_port.hpp gpio_registers.hpp exti.hpp - usart.hpp) + usart.hpp i2c.hpp) # cmsis_f767 is PUBLIC because cmsis.hpp, a public header, includes it. target_link_libraries(arm_cm7_mcu PUBLIC mcu cmsis_f767 diff --git a/src/libs/mcu/arm_cm7/i2c.cpp b/src/libs/mcu/arm_cm7/i2c.cpp new file mode 100644 index 0000000..8737d7d --- /dev/null +++ b/src/libs/mcu/arm_cm7/i2c.cpp @@ -0,0 +1,200 @@ +#include "libs/mcu/arm_cm7/i2c.hpp" + +#include +#include +#include +#include + +#include "libs/common/error.hpp" +#include "libs/mcu/arm_cm7/cmsis.hpp" +#include "libs/mcu/arm_cm7/gpio_port.hpp" +#include "libs/mcu/arm_cm7/systick.hpp" + +namespace mcu { + +namespace { + +/// Timing for 100 kHz standard mode from a 16 MHz I2CCLK (the reset +/// configuration: HSI, no PLL, APB1 prescaler 1). The F7's I2C is the v2 +/// peripheral: timing is these five fields, not the old CCR divisor, and the +/// value is normally taken from ST's tool rather than derived. Matches +/// AN4235's table for 16 MHz; the arithmetic, so the number is checkable: +/// +/// PRESC = 3 -> t_PRESC = (3+1) x 62.5 ns = 250 ns +/// SCLL = 0x13 -> (19+1) x 250 ns = 5.00 us low period +/// SCLH = 0x0F -> (15+1) x 250 ns = 4.00 us high period +/// SDADEL = 0x2 -> 2 x 250 ns = 500 ns data hold +/// SCLDEL = 0x4 -> (4+1) x 250 ns = 1.25 us data setup +/// +/// 5.00 + 4.00 us plus rise and fall gives a ~10 us period: 100 kHz. +/// Raising the core clock invalidates this constant. +constexpr std::uint32_t kTiming100kHzAt16MHz = 0x3042'0F13U; + +/// A transfer that makes no progress must fail rather than spin forever: a +/// bus held low by a stuck device never sets any completion flag. +constexpr std::uint32_t kTransferTimeoutMs = 25; + +[[nodiscard]] auto Registers(I2CId id) -> I2C_TypeDef* { + switch (id) { + case I2CId::kI2C1: + return I2C1; + case I2CId::kI2C2: + return I2C2; + case I2CId::kI2C3: + return I2C3; + case I2CId::kI2C4: + return I2C4; + } + return I2C1; +} + +auto EnablePeripheralClock(I2CId id) -> void { + switch (id) { + case I2CId::kI2C1: + RCC->APB1ENR |= RCC_APB1ENR_I2C1EN; + break; + case I2CId::kI2C2: + RCC->APB1ENR |= RCC_APB1ENR_I2C2EN; + break; + case I2CId::kI2C3: + RCC->APB1ENR |= RCC_APB1ENR_I2C3EN; + break; + case I2CId::kI2C4: + RCC->APB1ENR |= RCC_APB1ENR_I2C4EN; + break; + } + static_cast(RCC->APB1ENR); +} + +/// Spin until `flag` appears in ISR, giving up after kTransferTimeoutMs. +/// Reports a NACK distinctly from a timeout: an unanswered address is a +/// missing device, which is worth telling apart from a wedged bus. +[[nodiscard]] auto WaitForFlag(I2C_TypeDef* registers, std::uint32_t flag) + -> std::expected { + const std::uint32_t start = Millis(); + while ((registers->ISR & flag) == 0U) { + if ((registers->ISR & I2C_ISR_NACKF) != 0U) { + registers->ICR = I2C_ICR_NACKCF | I2C_ICR_STOPCF; + return std::unexpected(common::Error::kOperationFailed); + } + if ((Millis() - start) > kTransferTimeoutMs) { + return std::unexpected(common::Error::kTimeout); + } + } + return {}; +} + +/// Program CR2 for one autoend transfer and issue START. The peripheral takes +/// the 7-bit address left-shifted by one, in the position the R/W bit occupies +/// on the wire. +auto StartTransfer(I2C_TypeDef* registers, std::uint16_t address, + std::size_t byte_count, bool reading) -> void { + std::uint32_t cr2 = + (static_cast(address) << 1U) & I2C_CR2_SADD_Msk; + cr2 |= (static_cast(byte_count) << I2C_CR2_NBYTES_Pos) & + I2C_CR2_NBYTES_Msk; + cr2 |= I2C_CR2_AUTOEND; + if (reading) { + cr2 |= I2C_CR2_RD_WRN; + } + registers->CR2 = cr2 | I2C_CR2_START; +} + +/// Autoend issues STOP itself; STOPF must still be cleared or the next +/// transfer starts against a stale flag. +auto FinishTransfer(I2C_TypeDef* registers) + -> std::expected { + auto stopped = WaitForFlag(registers, I2C_ISR_STOPF); + registers->ICR = I2C_ICR_STOPCF; + registers->CR2 = 0; + return stopped; +} + +} // namespace + +auto I2CBus::Init() -> std::expected { + if (initialized_) { + return std::unexpected(common::Error::kInvalidState); + } + + EnablePeripheralClock(id_); + + // Open drain, because I2C signals are wire-AND: a push-pull driver would + // fight any other device pulling the line low. The internal pull-ups are + // weak (~40k) and adequate at 100 kHz for short wiring; a real bus wants + // external resistors. + const PinConfig pin{ + .mode = PinMode::kAlternate, + .output_type = OutputType::kOpenDrain, + .speed = PinSpeed::kVeryHigh, + .pull = PinPull::kUp, + .alternate_function = pins_.alternate_function, + }; + ConfigurePin(pins_.scl_port, pins_.scl_pin, pin); + ConfigurePin(pins_.sda_port, pins_.sda_pin, pin); + + auto* registers = Registers(id_); + + // TIMINGR is writable only while the peripheral is disabled. + registers->CR1 &= ~I2C_CR1_PE; + registers->TIMINGR = kTiming100kHzAt16MHz; + registers->CR1 |= I2C_CR1_PE; + + initialized_ = true; + return {}; +} + +auto I2CBus::SendData(std::uint16_t address, std::span data) + -> std::expected { + if (!initialized_) { + return std::unexpected(common::Error::kInvalidState); + } + if (data.empty()) { + return {}; + } + + auto* registers = Registers(id_); + StartTransfer(registers, address, data.size(), /*reading=*/false); + + for (const auto value : data) { + // TXIS, not TXE: TXIS means the peripheral is asking for the next byte, + // which is also where an unanswered address surfaces as NACKF. + if (auto ready = WaitForFlag(registers, I2C_ISR_TXIS); !ready) { + static_cast(FinishTransfer(registers)); + return std::unexpected(ready.error()); + } + registers->TXDR = std::to_integer(value); + } + + return FinishTransfer(registers); +} + +auto I2CBus::ReceiveData(std::uint16_t address, std::span buffer) + -> std::expected { + if (!initialized_) { + return std::unexpected(common::Error::kInvalidState); + } + if (buffer.empty()) { + return 0U; + } + + auto* registers = Registers(id_); + StartTransfer(registers, address, buffer.size(), /*reading=*/true); + + std::size_t received = 0; + for (auto& slot : buffer) { + if (auto ready = WaitForFlag(registers, I2C_ISR_RXNE); !ready) { + static_cast(FinishTransfer(registers)); + return std::unexpected(ready.error()); + } + slot = static_cast(registers->RXDR & 0xFFU); + ++received; + } + + if (auto finished = FinishTransfer(registers); !finished) { + return std::unexpected(finished.error()); + } + return received; +} + +} // namespace mcu diff --git a/src/libs/mcu/arm_cm7/i2c.hpp b/src/libs/mcu/arm_cm7/i2c.hpp new file mode 100644 index 0000000..d1e0681 --- /dev/null +++ b/src/libs/mcu/arm_cm7/i2c.hpp @@ -0,0 +1,57 @@ +#pragma once + +#include +#include +#include +#include + +#include "libs/common/error.hpp" +#include "libs/mcu/arm_cm7/gpio_port.hpp" +#include "libs/mcu/i2c.hpp" + +namespace mcu { + +/// @brief Which I2C instance. As with UsartId, the enumerators are indices, +/// not addresses: `I2C1` is a CMSIS macro, so no public header here may name +/// it. `kI2C1` is a distinct token and safe. +enum class I2CId : std::uint8_t { kI2C1, kI2C2, kI2C3, kI2C4 }; + +/// @brief Where an I2C instance's two pins are, and their alternate function. +struct I2CPins { + GpioPort scl_port; + std::uint32_t scl_pin; + GpioPort sda_port; + std::uint32_t sda_pin; + std::uint8_t alternate_function; +}; + +/// @brief Blocking I2C bus master. +/// +/// Transfers use the peripheral's autoend mode: NBYTES is programmed up front +/// and the hardware issues STOP on its own. That covers a whole transfer in +/// one setup, and rules out the repeated-start sequences a register-level +/// read of an addressed device needs -- which is why the demo application +/// writes and reads as two separate transactions. +class I2CBus final : public I2CController { + public: + I2CBus(I2CId id, const I2CPins& pins) : id_(id), pins_(pins) {} + + /// @brief Enable the peripheral and configure its pins. Called by the board; + /// SendData and ReceiveData report kInvalidState until it has run. + [[nodiscard]] auto Init() -> std::expected; + + [[nodiscard]] auto SendData(std::uint16_t address, + std::span data) + -> std::expected override; + + [[nodiscard]] auto ReceiveData(std::uint16_t address, + std::span buffer) + -> std::expected override; + + private: + I2CId id_; + I2CPins pins_; + bool initialized_ = false; +}; + +} // namespace mcu From 7e0545d4e0ccc1d12982ea3016e124e8b0e54c69 Mon Sep 17 00:00:00 2001 From: Nehal Patel Date: Fri, 4 Sep 2026 14:43:05 +0000 Subject: [PATCH 12/12] docs: anchor the deferred peripheral-init decisions to #37 and #38 Both decisions were reachable only from an issue tracker, which is not where anyone is standing when they matter. The code now points at them from the places that would otherwise look like arbitrary choices: the two-phase construction in gpio_pin.hpp, usart.hpp and i2c.hpp, and the hardcoded TIMINGR constant. Writing #37 up surfaced that the unenforced Init() is not an I2C problem. GpioPin::Configure(), Usart::Init() and I2CBus::Init() are the same shape -- a second phase guarded by a bool and a runtime kInvalidState. The split itself is necessary, because the board is a namespace-scope object whose members are constructed before SystemInit brings clocks up; what is wrong is that the type permits the intermediate state. Also records in PROJECT_PLAN that Milestone 3's board::Board narrowing is entangled with #37: an accessor that can say "this peripheral did not come up" is exactly what the factory-based fixes there need. Co-Authored-By: Claude Opus 5 (1M context) --- docs/PROJECT_PLAN.md | 4 +++- src/libs/mcu/arm_cm7/gpio_pin.hpp | 3 +++ src/libs/mcu/arm_cm7/i2c.cpp | 3 ++- src/libs/mcu/arm_cm7/i2c.hpp | 7 +++++++ src/libs/mcu/arm_cm7/usart.hpp | 4 ++++ 5 files changed, 19 insertions(+), 2 deletions(-) diff --git a/docs/PROJECT_PLAN.md b/docs/PROJECT_PLAN.md index d2fe480..6b45d74 100644 --- a/docs/PROJECT_PLAN.md +++ b/docs/PROJECT_PLAN.md @@ -92,7 +92,9 @@ is the open part. hardware board exists: optional accessors (`std::expected`) vs. capability mix-ins vs. compile-time board traits. Deferred from Milestone 2 deliberately — with - one board there was nothing to design against. + one board there was nothing to design against. Entangled with #37: an + accessor that can report "this peripheral did not come up" is what the + factory-based fixes there need. - [ ] Additional example application exercising more complex behavior - [ ] Cross-board validation: blinky runs on both boards unmodified diff --git a/src/libs/mcu/arm_cm7/gpio_pin.hpp b/src/libs/mcu/arm_cm7/gpio_pin.hpp index 9476cb3..d518c5e 100644 --- a/src/libs/mcu/arm_cm7/gpio_pin.hpp +++ b/src/libs/mcu/arm_cm7/gpio_pin.hpp @@ -17,6 +17,9 @@ namespace mcu { /// tree is up. Configure() is what makes the pin real, and every operation /// before it returns kInvalidState rather than writing into a dead register /// block. +/// +/// The split is deliberate; that nothing enforces the second half of it is +/// not. Every peripheral in this backend has the same shape. See issue #37. class GpioPin final : public BidirectionalPin { public: GpioPin(GpioPort port, std::uint32_t pin) : port_(port), pin_(pin) {} diff --git a/src/libs/mcu/arm_cm7/i2c.cpp b/src/libs/mcu/arm_cm7/i2c.cpp index 8737d7d..7f5b441 100644 --- a/src/libs/mcu/arm_cm7/i2c.cpp +++ b/src/libs/mcu/arm_cm7/i2c.cpp @@ -27,7 +27,8 @@ namespace { /// SCLDEL = 0x4 -> (4+1) x 250 ns = 1.25 us data setup /// /// 5.00 + 4.00 us plus rise and fall gives a ~10 us period: 100 kHz. -/// Raising the core clock invalidates this constant. +/// Raising the core clock invalidates this constant. Issue #38 covers turning +/// this into a table the board selects from, rather than one hardcoded rate. constexpr std::uint32_t kTiming100kHzAt16MHz = 0x3042'0F13U; /// A transfer that makes no progress must fail rather than spin forever: a diff --git a/src/libs/mcu/arm_cm7/i2c.hpp b/src/libs/mcu/arm_cm7/i2c.hpp index d1e0681..f3d928e 100644 --- a/src/libs/mcu/arm_cm7/i2c.hpp +++ b/src/libs/mcu/arm_cm7/i2c.hpp @@ -38,6 +38,13 @@ class I2CBus final : public I2CController { /// @brief Enable the peripheral and configure its pins. Called by the board; /// SendData and ReceiveData report kInvalidState until it has run. + /// + /// That the board must call this is a convention, not something the type + /// enforces -- and unlike Uart, mcu::I2CController has no Init(), so nothing + /// in board::Board's shape hints that the call is required. See issue #37. + /// + /// The bus runs at 100 kHz; making the speed a board-supplied parameter is + /// issue #38. [[nodiscard]] auto Init() -> std::expected; [[nodiscard]] auto SendData(std::uint16_t address, diff --git a/src/libs/mcu/arm_cm7/usart.hpp b/src/libs/mcu/arm_cm7/usart.hpp index d081409..c38e859 100644 --- a/src/libs/mcu/arm_cm7/usart.hpp +++ b/src/libs/mcu/arm_cm7/usart.hpp @@ -33,6 +33,10 @@ struct UsartPins { /// @brief Blocking USART, with an optional receive interrupt. /// +/// Like every peripheral here, this is constructed inert and made real by a +/// separate call -- Init(), which the application makes. Nothing enforces +/// that; see issue #37. +/// /// Send and Receive poll the status register; there is no transmit buffering, /// so Send returns once the last byte has left the shift register and the line /// is idle. That is the honest shape for this peripheral until there is a