feat(cli): warn when a UART's configured dmaopt loses its DMA claim

[nerdCopter]

AI Generated pull-request

Summary

serialUART() silently falls back to IRQ-driven mode when its DMA claim
(uartDmaClaim()/dmaAllocate()) loses a stream/channel to another peripheral at boot — no CLI
message, log entry, or other diagnostic exists anywhere (EF or BF, inherited design). PR #1384's
new dma CLI setter increased real user exposure to this by making it easy to configure a
dmaopt value that collides with another peripheral's claim.

Adds printDmaoptClaimStatus() in src/main/interface/cli.c, which cross-checks a resolved
dmaopt stream against dmaGetOwner()/dmaGetResourceIndex() — the same live bookkeeping
dmaAllocate()/uartDmaClaim() already populate — and prints a # ... WARNING ... line on
mismatch. Wired into both:

• printDmaoptEntry() — the dma <device> <index> get path and dma <device> list / bare
  dma list display path
• cliDmaopt() — the set path, so a dma UART_TX 1 <n> set gets an immediate proactive warning
  if the target stream is already held by something else

OWNER_FREE (stream never claimed by anyone) is treated as a no-op rather than a warning, since
it's indistinguishable from "this UART slot isn't opened as a serial port this boot" — the
overwhelmingly common state for any board with unused UART indices.

Closes #1388.

Test plan

• make test (unit tests) — 47 suites, 0 failures
• make HELIOSPRING TUNERCF405 SKYSTARSF405AIO PYRODRONEF7 FOXEERF722V4 FOXEERF405 APEXF7 TMOTORF7 STELLARH7DEV SITL — all succeed, 0 new warnings
• Reviewed via local CodeRabbit CLI (0 findings) and opencode second opinion (1 finding
  investigated, confirmed false positive — cited a SITL-only file never compiled where this
  feature runs)
• Hardware-verified on TMOTORF7 (build dd9d2b8, EmuFlight 0.4.3, MSP API 1.54), all three
  code paths through printDmaoptClaimStatus() exercised live via CLI:
  • Real conflict: dma UART_TX 3 0 (only option, resolves to DMA1 Stream 3, already held by
    SPI_SDI 2) → # UART_TX 3: WARNING stream held by SPI_SDI 2, not this UART -- expect IRQ-driven fallback on both set and get
  • OWNER_FREE (non-NONE value resolving to a genuinely-unclaimed stream): dma UART_TX 2 0
    (resolves to DMA1 Stream 6, FREE) → no warning
  • Explicit NONE: dma UART_TX 3 none → no warning
  • No save run at any point; flash-persisted defaults untouched

Summary by CodeRabbit

• Bug Fixes
  • Added diagnostics for UART DMA configuration mismatches.
  • The CLI now identifies when a selected DMA stream is assigned to an unexpected resource and warns that operation may fall back to interrupt-driven transfers.
  • Free DMA streams no longer generate unnecessary warnings.

[coderabbitai]

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 034e6d16-faeb-499d-81bc-a3de4637d267

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 7c7eaa8e-4153-4870-9c18-85803c2ef547

📥 Commits

Reviewing files that changed from the base of the PR and between a4ca6ad and dd9d2b8.

📒 Files selected for processing (1)

• src/main/interface/cli.c

Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review.

📜 Recent review details

⏰ Context from checks skipped due to timeout. (1)

• GitHub Check: Codacy Static Code Analysis

🔇 Additional comments (2)

src/main/interface/cli.c (2)

4091-4091: LGTM!

Also applies to: 4188-4194

---

4058-4067: 🎯 Functional Correctness

No unresolved DMA identifier is produced by the configured DMA mappings.

			> Likely an incorrect or invalid review comment.

---

📝 Walkthrough

Walkthrough

The CLI now checks live DMA ownership for UART DMA options. It ignores free streams and reports ownership mismatches that cause IRQ-driven fallback when listing or setting options.

Changes

UART DMA diagnostics

Layer / File(s)	Summary
Ownership checks and CLI integration src/main/interface/cli.c	The CLI validates each selected DMA stream against the expected UART owner and resource index. It reports conflicts during DMA option listing and after setting or retaining a non-NONE option.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to dd9d2

This localized CLI change adds diagnostics for DMA claim conflicts without introducing an actionable merge-blocking risk. It is merge-ready after normal checks and review.

Possibly related PRs

• emuflight/EmuFlight#1356: Addresses UART DMA stream ownership and IRQ fallback behavior.
• emuflight/EmuFlight#1384: Adds the UART DMA CLI functionality extended by these diagnostics.
• emuflight/EmuFlight#1383: Introduces the shared dmaopt configuration used by these ownership checks.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the CLI warning added for UART DMA claim loss.
Description check	✅ Passed	The description explains the problem, implementation, affected paths, scope, and verification results.
Linked Issues check	✅ Passed	The changes satisfy issue #1388 by warning when a configured UART DMA stream is owned by another peripheral.
Out of Scope Changes check	✅ Passed	The changes are limited to UART DMA ownership diagnostics in the CLI and match the linked issue objectives.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[nerdCopter]

@coderabbitai, AI generated comment

Requesting analysis (not a full review) on two specific points in printDmaoptClaimStatus()
(src/main/interface/cli.c):

1. The function early-returns without printing a warning when dmaGetOwner(identifier) == OWNER_FREE, on the reasoning that OWNER_FREE is indistinguishable from "this UART device
   slot was never opened as a serial port this boot" (the common case for unused UART indices)
   versus a genuine unexpected claim failure. Is there a real code path where a UART is
   actually opened via serialUART(), its DMA claim legitimately fails for a reason distinct
   from "another peripheral already holds this stream" (i.e. leaves the stream OWNER_FREE
   rather than owned by a rival), and where suppressing the warning here would hide an
   actionable signal rather than avoid a false positive?

2. dmaGetIdentifier() returns 0 (an invalid identifier) when the passed stream pointer isn't
   found in dmaDescriptors[]. Confirm whether dmaGetOwner()/dmaGetResourceIndex() in the
   actual compiled implementations for this feature's build targets —
   src/main/drivers/dma_stm32f4xx.c, dma_stm32f7xx.c, dma_stm32h7xx.c (NOT
   src/main/drivers/dma.c, which is SITL-only per make/mcu/SITL.mk and never compiled where
   this CLI code runs) — guard against identifier 0 via dmaIdentifierIsValid() before
   indexing dmaDescriptors[], i.e. that no out-of-bounds read is reachable through this new
   function on any real target.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '== Target file outlines =='
ast-grep outline src/main/interface/cli.c --match 'printDmaoptClaimStatus|printDmaoptEntry|cliDmaopt' --view expanded || true
ast-grep outline src/main/drivers/dma_stm32f4xx.c --items all --view expanded || true
ast-grep outline src/main/drivers/dma_stm32f7xx.c --items all --view expanded || true
ast-grep outline src/main/drivers/dma_stm32h7xx.c --items all --view expanded || true

echo '== CLI implementation and DMA status call sites =='
rg -n -C 12 'printDmaoptClaimStatus|dmaGetOwner\s*\(|dmaGetResourceIndex\s*\(|dmaGetIdentifier\s*\(' \
  src/main/interface/cli.c src/main 2>/dev/null || true

echo '== UART initialization and DMA claim paths =='
rg -n -C 15 'serialUART\s*\(|uartDmaClaim\s*\(|dmaAllocate\s*\(|dmaEnable\s*\(' src/main \
  -g '*.[ch]' 2>/dev/null || true

echo '== DMA function definitions and identifier validation =='
rg -n -C 10 'dmaGetIdentifier\s*\(|dmaGetOwner\s*\(|dmaGetResourceIndex\s*\(|dmaIdentifierIsValid\s*\(' \
  src/main/drivers/dma_stm32f4xx.c \
  src/main/drivers/dma_stm32f7xx.c \
  src/main/drivers/dma_stm32h7xx.c \
  src/main/drivers/dma_impl.h \
  src/main/drivers/dma.h 2>/dev/null || true

Length of output: 192925

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '== DMA request-map declarations, resolver, and macros =='
ast-grep outline src/main/drivers/dma_reqmap.c src/main/drivers/dma_reqmap_mcu.c src/main/drivers/dma_reqmap.h --items all --view expanded 2>/dev/null || true
rg -n -C 14 'dmaGetChannelSpecByPeripheral|dmaChannelSpec_t|DMA\(|dmaChannelSpec' \
  src/main/drivers/dma_reqmap.c \
  src/main/drivers/dma_reqmap_mcu.c \
  src/main/drivers/dma_reqmap.h \
  src/main/drivers/dma.h 2>/dev/null || true

echo '== Complete target DMA descriptor definitions =='
sed -n '30,76p' src/main/drivers/dma_stm32f4xx.c
sed -n '34,75p' src/main/drivers/dma_stm32f7xx.c
sed -n '37,76p' src/main/drivers/dma_stm32h7xx.c

echo '== UART request-map entries and relevant conditionals =='
rg -n -C 5 'DMA_PERIPH_UART_(RX|TX)' src/main/drivers/dma_reqmap_mcu.c 2>/dev/null || true

echo '== Exact DMA allocation return paths =='
sed -n '77,105p' src/main/drivers/dma_stm32f4xx.c
sed -n '87,115p' src/main/drivers/dma_stm32f7xx.c
sed -n '84,130p' src/main/drivers/dma_stm32h7xx.c

Length of output: 35222

---

@nerdCopter, I checked the F4, F7, and H7 paths. I also used the prior project guidance about DMA_NONE guards. It agrees with the current source.

1. OWNER_FREE suppression is correct for the current request maps.

   uartDmaClaim() calls dmaAllocate() unless the stream is already owned by the same UART direction and index.

   In all three compiled implementations, dmaAllocate() can return false for only two reasons:

   • The DMA identifier is invalid.
   • The descriptor owner is not OWNER_FREE.

   A valid identifier that is OWNER_FREE always succeeds. It changes the owner to OWNER_SERIAL_RX or OWNER_SERIAL_TX.

   Therefore, there is no normal serialUART() claim-failure path that leaves a valid resolved stream OWNER_FREE. A failed valid claim necessarily leaves the existing rival owner intact. printDmaoptClaimStatus() reports that condition.

   The other failure case is an invalid identifier. That would require dmaGetIdentifier() not to find dmaChannelSpec->ref. The current request maps cannot create this state:

   • F4/F7 UART entries in src/main/drivers/dma_reqmap_mcu.c use only DMA1_Stream0 through DMA2_Stream7.
   • H7 exposes the same 16 stream references through its dmaChannelSpec[].
   • Each target-specific dmaDescriptors[] includes all 16 of those stream references.

   Thus, suppressing OWNER_FREE does not hide an actionable DMA allocation failure for valid current configurations. It suppresses the expected case where no UART DMA claim occurred during boot.

   A future broken request-map entry could produce identifier 0. That is a map-integrity defect, not a valid contention case. The current CLI output would suppress it as OWNER_FREE.

2. The identifier 0 access is safe on F4, F7, and H7.

   Each compiled implementation defines:

   return identifier > DMA_NONE && identifier <= DMA_LAST_HANDLER;

   Each dmaGetOwner() checks dmaIdentifierIsValid(identifier) before indexing dmaDescriptors[].

   Each dmaGetResourceIndex() does the same.

   For identifier 0:

   • dmaGetOwner(0) returns OWNER_FREE.
   • dmaGetResourceIndex(0) returns 0.
   • Neither function evaluates DMA_IDENTIFIER_TO_INDEX(0).
   • No out-of-bounds descriptor read occurs.

   This applies to:

   • src/main/drivers/dma_stm32f4xx.c, Lines 143–155
   • src/main/drivers/dma_stm32f7xx.c, Lines 134–146
   • src/main/drivers/dma_stm32h7xx.c, Lines 139–153

   src/main/drivers/dma.c lacks these guards, but it is not relevant to the F4/F7/H7 builds for this CLI feature.

Conclusion: I found no issue in either point. The OWNER_FREE behavior matches the allocator semantics and avoids the intended false positives. The target-specific DMA accessors safely handle identifier 0.

---

🧠 Learnings used

Learnt from: nerdCopter
Repo: emuflight/EmuFlight PR: 0
File: :0-0
Timestamp: 2026-04-29T23:22:24.191Z
Learning: In EmuFlight dma_stm32f4xx.c and dma_stm32f7xx.c (PR `#1130`, Stage M.3.d onward), dmaAllocate() and dmaEnable() must guard against `identifier == DMA_NONE` (which maps to index −1) before indexing into the dmaDescriptors array. Without this guard, DMA_NONE passed from spiInitBusDMA for buses without an available DMA channel causes an out-of-bounds array access. The fix applies identically to both the F4 and F7 implementations.

_{You are interacting with an AI system.}