fix!: second assessment round — driver bugs, family #ifdef refactor, host tests, docs

[gabrielfrasantos]

This fixes the findings from a second, independent review of main after #117. Every finding was checked against the code before being fixed. I also re-checked the three High findings myself.

Since then, the HIL bench (#120) has run these drivers on an EK-TM4C1294XL and an EK-TM4C123GXL. The hardware fixes it led to are listed under Found on hardware, several of them TM4C123 errata.

High

• PWM: generator interrupts were never enabled at the module level (PWMINTEN.INTPWMn), so onNormalInterrupt never fired.
• SPI on TM4C129: MOSI and MISO were swapped on every SSI instance. In normal SPI mode SSInXDAT0 is TX and SSInXDAT1 is RX.
• UartWithDma: on an RX timeout, the half-buffer currently being filled was detected wrongly once the primary half had been re-armed, so received bytes were dropped. It is now read from the uDMA ALTSET state.

Medium

• GPIO pin release: releasing a pin (and analog setup) left it as a push-pull output driving the stale data latch. It now returns to its reset state (input, digital function off).
• Pwm/SynchronousPwm: Config is kept by value, not by a reference that dangles for temporaries. 0% and 100% duty now drive the output constant low/high; before, 0% could come out as roughly 100%.
• Interrupt table: sized 155 so TM4C123 PWM module 1 interrupts (IRQ 134–138) fit.
• ADC: channels AIN16–19 are encoded through SSEMUX. Before, they sampled AIN0–3 in differential mode.
• Comparator: ACCTL.TOEN is bit 11 and is set when the ADC trigger is enabled.
• TM4C129 clock: SystemCoreClock reports the frequency actually produced, not the requested one. The oscillator source is no longer forced to PIOSC after setup.
• CAN: the accept-all filter no longer rejects 29-bit frames.
• GPIO interrupts: they now work on every port, including TM4C129 ports G–N and the per-pin P/Q vectors. InterruptType::immediate runs the action directly in the ISR.
• FreeRTOS example: the syscall priority ceiling now lets EMIL's normal/high interrupt priorities call FreeRTOS APIs from an ISR.

Low

• Startup SystemCoreClock is 16 MHz (the reset clock).
• EEPROM callbacks always run from the event dispatcher.
• SynchronousSpiMaster drains stale RX data correctly in its constructor.
• MAC address filter: no more out-of-bounds read.
• SPI clock-divider search checks its inputs and never picks a rate above the one requested.
• SpiMaster's interrupt handler is no longer destroyed from inside its own interrupt handler.
• The two watchdog timers share one vector through a single handler.
• DmaChannel stores its channel by value.
• GpioPin::Set writes through the masked data address.
• Pin reservation checks use really_assert, so they stay in release builds.
• QEI is reset before configuration, and Resolution() returns the configured value.
• ADC completion polling only checks the sequencer's own bit.
• CAN: the ISR only uses IF2 (the thread uses IF1), and the TX request is cleared on bus-off.
• Unused config fields are now applied (PWM output inversion, LaunchPad clock divider and PLL). The UART word-length field only supports 8 bits and now asserts on anything else.
• The instantiations namespace typo is fixed.
• Ethernet uses DEVICE_HEADER and reports the selected PHY address.

Found on hardware (HIL, #120)

• UartWithDma:
  • RX with burst arbitration keeps a residue so the receive time-out fires. USEBURST is restored on every arm, and the whole FIFO is drained at time-out.
  • RX now moves one item per request. At a ping-pong half switch, the uDMA could serve a burst request the UART had not yet withdrawn. That read the empty FIFO and inserted stale characters at 921600 Bd.
  • TX is burst-only, so a single request can no longer push 4 items into a nearly full FIFO. This dropped bytes on the TM4C123.
• Uart (interrupt): the receive interrupt fires at half full instead of 7/8, giving more latency headroom.
• SpiMaster: batches complete from the receive FIFO. The TM4C123 does not latch TXRIS in EOT mode (erratum SSI#07), so asynchronous transfers timed out and then overran into an assert.
• PWM fault: PWMFAULT/PWMFAULTVAL force the outputs inactive on a fault. The ISR reports one event, and Stop() clears the latch and re-arms.
• GPIO: dispatched interrupts are coalesced per pin with a pending-edge counter, so fast edges can't flood the event queue.
• TM4C129 comparator: outputs C0o–C2o on PD0–PD2 use PCTL function 5.
• Watchdog: without reset, the ISR clears the time-out itself. Watchdog 1 otherwise resets on the second time-out even with RESEN clear (erratum WDT#03).
• ADC:
  • The module is reset through SRADC when its clock is first enabled (erratum ADC#14).
  • A sequence whose END step goes to a digital comparator ends on a dummy FIFO step instead, and that sample is discarded. On the TM4C123 the comparator END step lost one FIFO sample per run (erratum ADC#03).
• Build: the Tiva configure presets set CMAKE_BUILD_TYPE=RelWithDebInfo. Before, the single-config generator built firmware at -O0, CI included, and the build presets' configuration had no effect.
• EMIL: pinned to 0059646, which includes the HIL watchdog auto-feed fix (embedded-infra-lib#155).

Repo rules and consistency

• No family #ifdefs in C++ (AGENTS.md rule). Family-specific constants, types and register helpers moved to hal_tiva/tiva/family/<family>/<Driver>Family.hpp, and the BSP to hal_tiva/instantiations/family/<family>/ (<family> is tm4c123 or tm4c129). CMake adds only family/$<LOWER_CASE:${TARGET_MCU_FAMILY}> to the include path.
• Interrupt handler order: ADC, CAN, EEPROM, watchdog, Ethernet and SpiMaster now register their EMIL interrupt handler at the end of the constructor, after clearing the pending interrupt. Destructors release it first instead of calling NVIC_* directly.
• Host tests: the placeholder test is replaced by 47 real tests. CAN bit timing and the SPI clock-divider search moved into device-independent headers (CanBitTiming.hpp, SpiClockDivisor.hpp); the two SPI drivers now share one implementation.
• Examples: terminal_uart_with_dma now actually uses Dma + UartWithDma. The new HAL_TI_INCLUDE_LWIP option lets the lwIP instantiation build standalone.
• Docs (README, AGENTS.md, CLAUDE.md, Copilot/Claude agent instructions): interrupt dispatch goes through EMIL's InterruptTable, so there are no per-interrupt startup-file edits. Also corrected: the handler registration order, CTest preset name, hal::cortex namespace, CAN status write-back, CMake 3.24, the hal_tiva.tiva target name and the broken CODEOWNERS links. Added doc/EK-TM4C1294XL.md.

Breaking changes

• AnalogComparator::Config::routeToPwmFault is removed; it had no effect.
• DmaChannel::IsCompletionPending() / ClearCompletion() (TM4C123 only) are replaced by DmaChannel::ChannelNumber().
• Can::BitTiming is now an alias of hal::tiva::CanBitTiming, with the same fields.
• UART NumberOfBytes::_16_bytes now asserts; it was silently ignored before.
• Tiva builds configured from the presets are now RelWithDebInfo; pass -DCMAKE_BUILD_TYPE=Debug for an unoptimised build.

Verification

• tm4c123gh6pm and tm4c1294ncpdt builds pass, examples included.
• The FreeRTOS example builds for both chips. With HAL_TI_INCLUDE_LWIP=ON, the TM4C129 build passes.
• Host build passes, and 47/47 host tests pass.
• No new clang-format deviations; ls-lint passes. The original line endings (CRLF/LF) are kept.
• HIL with feat(validation): add HIL validation app (firmware terminal + Python/AD3 test suite) #120's validation firmware:
  • EK-TM4C123GXL (optimised build, bundle1, full depth, async SPI included): 2652 passed and 1 failed. The failure was the UART 921600 DMA full-duplex duplicate. It is fixed by 016afa5 and confirmed clean afterwards (0/240 rounds, 40/40 runs), with no change in the full UART suite (160 passed).
  • EK-TM4C1294XL (bundle1 full, then bundle2 full, on an -O0 build): 2416 passed and 5 failed. Four were the watchdog failures fixed in EMIL; one was an SPI decode that passed on rerun. bundle2: 78 passed. The 1294 has not been re-run since the later UART, SPI, ADC and build-type changes.

🤖 Generated with Claude Code

https://claude.ai/code/session_014WbtsfrCF7vqzVkHsKYMoX

[gabrielfrasantos]

✅⚠️MegaLinter analysis: Success with warnings

Descriptor	Linter	Files	Fixed	Errors	Max errors	Warnings	Elapsed time
✅ ACTION	actionlint	5		0		0	0.05s
✅ CPP	clang-format	7	0	0		0	0.06s
✅ CPP	cppcheck	7		0		0	0.14s
✅ DOCKERFILE	hadolint	1		0		0	0.12s
✅ JSON	jsonlint	10		0		0	0.05s
✅ JSON	prettier	10	7	0		0	0.84s
⚠️ MARKDOWN	markdownlint	18	0	12		0	0.72s
✅ MARKDOWN	markdown-table-formatter	18	0	0		0	0.15s
✅ REPOSITORY	betterleaks	yes		no		no	1.03s
✅ REPOSITORY	checkov	yes		no		no	19.24s
✅ REPOSITORY	git_diff	yes		no		no	0.03s
✅ REPOSITORY	grype	yes		no		no	62.79s
✅ REPOSITORY	ls-lint	yes		no		no	0.0s
✅ REPOSITORY	secretlint	yes		no		no	0.61s
✅ REPOSITORY	syft	yes		no		no	1.11s
✅ REPOSITORY	trivy	yes		no		no	12.32s
✅ REPOSITORY	trivy-sbom	yes		no		no	0.09s
✅ REPOSITORY	trufflehog	yes		no		no	2.27s
✅ SPELL	lychee	44		0		0	1.22s
✅ YAML	prettier	8	2	0		0	0.59s
✅ YAML	v8r	8		0		0	3.87s
✅ YAML	yamllint	8		0		0	1.27s

Detailed Issues

⚠️ MARKDOWN / markdownlint - 12 errors

.claude/agents/executor.md:8 error MD041/first-line-heading/first-line-h1 First line in a file should be a top-level heading [Context: "You are the executor agent for..."]
.claude/agents/orchestrator.md:8 error MD041/first-line-heading/first-line-h1 First line in a file should be a top-level heading [Context: "You are the orchestrator agent..."]
.claude/agents/planner.md:8 error MD041/first-line-heading/first-line-h1 First line in a file should be a top-level heading [Context: "You are the planner agent for ..."]
.claude/agents/reviewer.md:8 error MD041/first-line-heading/first-line-h1 First line in a file should be a top-level heading [Context: "You are the reviewer agent for..."]
.github/agents/executor.agent.md:11 error MD041/first-line-heading/first-line-h1 First line in a file should be a top-level heading [Context: "You are the executor agent for..."]
.github/agents/orchestrator.agent.md:18 error MD041/first-line-heading/first-line-h1 First line in a file should be a top-level heading [Context: "You are the orchestrator agent..."]
.github/agents/planner.agent.md:11 error MD041/first-line-heading/first-line-h1 First line in a file should be a top-level heading [Context: "You are the planner agent for ..."]
.github/agents/reviewer.agent.md:14 error MD041/first-line-heading/first-line-h1 First line in a file should be a top-level heading [Context: "You are the reviewer agent for..."]
.github/prompts/orchestrate.prompt.md:8:401 error MD013/line-length Line length [Expected: 400; Actual: 615]
.github/prompts/orchestrate.prompt.md:8 error MD041/first-line-heading/first-line-h1 First line in a file should be a top-level heading [Context: "Analyze the following task for..."]
AGENTS.md:79:401 error MD013/line-length Line length [Expected: 400; Actual: 712]
AGENTS.md:81:401 error MD013/line-length Line length [Expected: 400; Actual: 557]

Notices

⚠️ Your configuration references items that have been removed from MegaLinter and are ignored: REPOSITORY_GITLEAKS, REPOSITORY_KICS. See Removed linters to find their replacements.

See detailed reports in MegaLinter artifacts

Your project could benefit from a custom flavor, which would allow you to run only the linters you need, and thus improve runtime performances. (Skip this info by defining FLAVOR_SUGGESTIONS: false)

• Documentation: Custom Flavors
• Command: npx mega-linter-runner@10.1.0 --custom-flavor-setup --custom-flavor-linters ACTION_ACTIONLINT,CPP_CPPCHECK,CPP_CLANG_FORMAT,DOCKERFILE_HADOLINT,JSON_JSONLINT,JSON_PRETTIER,MARKDOWN_MARKDOWNLINT,MARKDOWN_MARKDOWN_TABLE_FORMATTER,REPOSITORY_CHECKOV,REPOSITORY_GIT_DIFF,REPOSITORY_BETTERLEAKS,REPOSITORY_GRYPE,REPOSITORY_LS_LINT,REPOSITORY_SECRETLINT,REPOSITORY_SYFT,REPOSITORY_TRIVY,REPOSITORY_TRIVY_SBOM,REPOSITORY_TRUFFLEHOG,SPELL_LYCHEE,YAML_PRETTIER,YAML_YAMLLINT,YAML_V8R

Show us your support by starring ⭐ the repository

[Copilot]

Copilot review overview

🟡 Changes recommended

PWM channel updates can overwrite companion outputs, while several new shared IRQ registrations mishandle pending state or priority.

Review effort: Balanced
Findings: 5

Open (5)

• Duty updates overwrite the other channel's output mode · New
• Clear GPIO port IRQ pending state before handler registration · New
• Clear generator and NVIC pending state before handler registration · New
• Reconfigure shared IRQ priority for all active watchdogs · New
• Clear pending watchdog IRQ before shared-handler registration · New

What changed in this PR

Broad correctness sweep across TI peripheral drivers, family-specific code, host tests, examples, and documentation.

Changes:

• Fixes PWM, SPI, UART DMA, GPIO, ADC, CAN, watchdog, clock, and interrupt handling.
• Moves family-specific logic into CMake-selected headers.
• Adds 47 host tests and updates examples/documentation.

File	Description
tiva/​CMSIS/​Device/​TI/​TM4C129/​Source/​Templates/​system_TM4C129.c	Sets reset clock to 16 MHz.
tiva/​CMSIS/​Device/​TI/​TM4C123/​Source/​Templates/​system_TM4C123.c	Sets reset clock to 16 MHz.
README.md	Updates integration and testing docs.
integration_test/​test/​Test.cpp	Removes placeholder test.
integration_test/​test/​SpiClockDivisorTest.cpp	Tests SPI divisor selection.
integration_test/​test/​CMakeLists.txt	Registers new host tests.
integration_test/​test/​CanBitTimingTest.cpp	Tests CAN timing calculation.
hal_tiva/​tiva/​WatchDog.hpp	Adds shared-handler state.
hal_tiva/​tiva/​WatchDog.cpp	Shares the watchdog IRQ.
hal_tiva/​tiva/​UniqueDeviceId.cpp	Delegates family-specific behavior.
hal_tiva/​tiva/​UartWithDma.cpp	Fixes DMA completion and timeout handling.
hal_tiva/​tiva/​UartBase.cpp	Rejects unsupported word lengths.
hal_tiva/​tiva/​SpiMaster.cpp	Uses shared divisor logic and persistent IRQ registration.
hal_tiva/​tiva/​SpiClockDivisor.hpp	Adds device-independent divisor search.
hal_tiva/​tiva/​Pwm.hpp	Owns configuration and adds inversion setup.
hal_tiva/​tiva/​Pwm.cpp	Fixes interrupts, inversion, and endpoint duty cycles.
hal_tiva/​tiva/​PinoutTableDefaultTm4c129.cpp	Corrects TM4C129 SPI pins.
hal_tiva/​tiva/​PinoutTableDefault.hpp	Adds family-neutral declarations.
hal_tiva/​tiva/​Gpio.hpp	Expands interrupt routing support.
hal_tiva/​tiva/​Gpio.cpp	Fixes release, analog, masked writes, and IRQ dispatch.
hal_tiva/​tiva/​family/​tm4c129/​UniqueDeviceIdFamily.hpp	Provides TM4C129 unique ID access.
hal_tiva/​tiva/​family/​tm4c129/​UartWithDmaFamily.hpp	Defines TM4C129 DMA status handling.
hal_tiva/​tiva/​family/​tm4c129/​QuadratureEncoderFamily.hpp	Defines TM4C129 QEI resources.
hal_tiva/​tiva/​family/​tm4c129/​PwmFamily.hpp	Defines TM4C129 PWM resources.
hal_tiva/​tiva/​family/​tm4c129/​GpioFamily.hpp	Defines TM4C129 GPIO resources.
hal_tiva/​tiva/​family/​tm4c129/​AnalogComparatorFamily.hpp	Defines comparator registers and IRQs.
hal_tiva/​tiva/​family/​tm4c129/​AnalogComparatorFamily.cpp	Adds comparator 2 forwarding.
hal_tiva/​tiva/​family/​tm4c123/​UniqueDeviceIdFamily.hpp	Defines empty TM4C123 unique ID.
hal_tiva/​tiva/​family/​tm4c123/​UartWithDmaFamily.hpp	Defines TM4C123 DMA completion handling.
hal_tiva/​tiva/​family/​tm4c123/​QuadratureEncoderFamily.hpp	Defines TM4C123 QEI resources.
hal_tiva/​tiva/​family/​tm4c123/​PwmFamily.hpp	Defines TM4C123 PWM resources.
hal_tiva/​tiva/​family/​tm4c123/​PwmFamily.cpp	Adds PWM1 interrupt forwarding.
hal_tiva/​tiva/​family/​tm4c123/​GpioFamily.hpp	Defines TM4C123 GPIO resources.
hal_tiva/​tiva/​family/​tm4c123/​AnalogComparatorFamily.hpp	Defines TM4C123 comparator resources.
hal_tiva/​tiva/​Ethernet.hpp	Makes IRQ registration optional.
hal_tiva/​tiva/​Ethernet.cpp	Fixes lifecycle, MAC packing, and PHY reporting.
hal_tiva/​tiva/​Eeprom.hpp	Stores optional IRQ handler.
hal_tiva/​tiva/​Eeprom.cpp	Defers callbacks and fixes handler lifecycle.
hal_tiva/​tiva/​Dma.hpp	Owns channel data and exposes channel state.
hal_tiva/​tiva/​Dma.cpp	Implements alternate-state and channel queries.
hal_tiva/​tiva/​CMakeLists.txt	Selects family-specific implementation files.
hal_tiva/​tiva/​ClockTm4c129.cpp	Reports actual configured frequency.
hal_tiva/​tiva/​CanBitTiming.hpp	Extracts CAN timing calculation.
hal_tiva/​tiva/​Can.hpp	Uses shared timing type and optional handler.
hal_tiva/​tiva/​Can.cpp	Fixes filtering, IF usage, and lifecycle.
hal_tiva/​tiva/​AnalogComparator.hpp	Removes ineffective PWM routing API.
hal_tiva/​tiva/​AnalogComparator.cpp	Fixes trigger output configuration.
hal_tiva/​synchronous_tiva/​SynchronousUart.cpp	Rejects unsupported word lengths.
hal_tiva/​synchronous_tiva/​SynchronousSpiMaster.cpp	Shares divisor logic and drains RX data.
hal_tiva/​synchronous_tiva/​SynchronousQuadratureEncoder.hpp	Stores configured resolution.
hal_tiva/​synchronous_tiva/​SynchronousQuadratureEncoder.cpp	Resets QEI and fixes resolution reporting.
hal_tiva/​synchronous_tiva/​SynchronousPwm.hpp	Owns configuration and adds inversion setup.
hal_tiva/​synchronous_tiva/​SynchronousPwm.cpp	Fixes divisors, inversion, and endpoint duty cycles.
hal_tiva/​synchronous_tiva/​SynchronousAnalogComparator.hpp	Removes obsolete routing alias.
hal_tiva/​synchronous_tiva/​SynchronousAnalogComparator.cpp	Fixes trigger output configuration.
hal_tiva/​synchronous_tiva/​SynchronousAdc.cpp	Fixes channel encoding and result sizing.
hal_tiva/​instantiations/​TracingReset.hpp	Corrects namespace spelling.
hal_tiva/​instantiations/​TracingReset.cpp	Corrects namespace spelling.
hal_tiva/​instantiations/​LaunchPadBsp.hpp	Selects family-specific BSP.
hal_tiva/​instantiations/​family/​tm4c129/​LaunchPadFamily.hpp	Defines TM4C129 LaunchPad setup.
hal_tiva/​instantiations/​family/​tm4c123/​LaunchPadFamily.hpp	Defines TM4C123 LaunchPad setup.
hal_tiva/​instantiations/​CMakeLists.txt	Adds family BSP include paths.
hal_tiva/​bringup/​Bringup.cpp	Enlarges the interrupt table.
examples/​terminal_uart_with_dma/​Main.cpp	Demonstrates actual DMA UART usage.
examples/​freertos/​Main.cpp	Uses family-neutral second LED.
examples/​freertos/​config/​FreeRTOSConfig.h	Adjusts syscall interrupt priority.
doc/​EK-TM4C1294XL.md	Documents the TM4C129 LaunchPad.
CONTRIBUTING.md	Corrects project and reporting guidance.
CMakeLists.txt	Adds optional lwIP integration.
CLAUDE.md	Updates interrupt lifecycle guidance.
AGENTS.md	Updates architecture and workflow rules.
.github/​prompts/​orchestrate.prompt.md	Updates orchestration guidance.
.github/​instructions/​hal-ti-cpp.instructions.md	Updates C++ interrupt rules.
.github/​copilot-instructions.md	Updates repository patterns and commands.
.github/​agents/​reviewer.agent.md	Updates review checklist.
.github/​agents/​planner.agent.md	Updates planning guidance.
.github/​agents/​orchestrator.agent.md	Updates routing guidance.
.github/​agents/​executor.agent.md	Updates implementation guidance.
.claude/​agents/​reviewer.md	Updates review checklist.
.claude/​agents/​planner.md	Updates planning guidance.
.claude/​agents/​orchestrator.md	Updates routing guidance.
.claude/​agents/​executor.md	Updates implementation guidance.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.