Skip to content

chore(pins): re-pin zephyr + fprime-zephyr onto trimmed integration branches - #487

Merged
Mikefly123 merged 2 commits into
feat/usp-radiofrom
chore/usp-trim-repin
Aug 2, 2026
Merged

chore(pins): re-pin zephyr + fprime-zephyr onto trimmed integration branches#487
Mikefly123 merged 2 commits into
feat/usp-radiofrom
chore/usp-trim-repin

Conversation

@Mikefly123

Copy link
Copy Markdown
Contributor

Drops two changes from the USP radio changeset as part of the pre-merge
blast-radius trim. Both were pruned on the merits; neither costs any
measured throughput.

fprime-zephyr c81485f -> 3da24b8 (feat/proves-usp-radio-trimmed)
Removes the re-arm semaphore (OSSF/fprime-zephyr#19, closed). It had a
measured null result — 35.8 s before and after, 1.00x on the downlink
ladder — and the PR body itself retracted its premise: the k_sleep poll
it replaced never actually slept at saturation.

The GFSK RX->TX wedge fix (#23) was rebased off it onto feat/usp-radio.
Verified: the rebased tree differs from the pre-trim tree by exactly the
semaphore's 30 lines in UspRadio.cpp — quiesceRadio(), the TX_DONE
timeout recovery, the comStatus-SUCCESS-after-drop path and the
TxOutcomePolicy UTs are all intact. P5_GMSK_83K (#20, draft) was rebased
onto the new #23 head and is retained unchanged.

zephyr 3838a28 -> 144acbc (feat/proves-usp-radio-trimmed)
Removes the cdc_acm poll_out bounded wait (OSSF/zephyr#2, closed). The
added bound sits behind data->flow_ctrl, which is false on the v5e from
both directions: hw-flow-control is absent from the cdc_acm_uart0 node
(verified in the generated devicetree, not just source), and the sole
uart_configure() caller sets UART_CFG_FLOW_CTRL_NONE. The code was
unreachable in the flight image.

The TX-FIFO drain fix (#1) is explicitly RETAINED — it is reachable
wherever a USB host is enumerated, which includes the self-hosted
integration-uart/integration-radio runners and their Korad power-cycle
and re-enumeration steps. Fork delta drops from +21/-1 to +11/-0.

Old branches feat/proves-usp-radio are retained on both forks pending
review of this pin; pruned branches are retained for recoverability.

Co-Authored-By: Claude noreply@anthropic.com


Part of the pre-merge blast-radius trim of the USP radio changeset (tracking: #439).

Verification done before opening:

  • fprime-zephyr: rebased tree differs from pre-trim by exactly the semaphore's 30 lines; wedge fix + UTs byte-intact.
  • zephyr: fork delta vs v4.4.1-base confirmed +11/-0 (drain only) vs +21/-1 before.

Still needed: build + unit tests + full CI on this pin, and a bench smoke. Because neither pruned change is on the throughput ladder, the expected downlink number is unchanged at ~36.08 s / 4,953 B/s — this pin should not move perf at all, which is itself the check.

🤖 Generated with Claude Code

…ranches

Drops two changes from the USP radio changeset as part of the pre-merge
blast-radius trim. Both were pruned on the merits; neither costs any
measured throughput.

fprime-zephyr c81485f -> 3da24b8 (feat/proves-usp-radio-trimmed)
  Removes the re-arm semaphore (OSSF/fprime-zephyr#19, closed). It had a
  measured null result — 35.8 s before and after, 1.00x on the downlink
  ladder — and the PR body itself retracted its premise: the k_sleep poll
  it replaced never actually slept at saturation.

  The GFSK RX->TX wedge fix (#23) was rebased off it onto feat/usp-radio.
  Verified: the rebased tree differs from the pre-trim tree by exactly the
  semaphore's 30 lines in UspRadio.cpp — quiesceRadio(), the TX_DONE
  timeout recovery, the comStatus-SUCCESS-after-drop path and the
  TxOutcomePolicy UTs are all intact. P5_GMSK_83K (#20, draft) was rebased
  onto the new #23 head and is retained unchanged.

zephyr 3838a28 -> 144acbc (feat/proves-usp-radio-trimmed)
  Removes the cdc_acm poll_out bounded wait (OSSF/zephyr#2, closed). The
  added bound sits behind data->flow_ctrl, which is false on the v5e from
  both directions: hw-flow-control is absent from the cdc_acm_uart0 node
  (verified in the generated devicetree, not just source), and the sole
  uart_configure() caller sets UART_CFG_FLOW_CTRL_NONE. The code was
  unreachable in the flight image.

  The TX-FIFO drain fix (#1) is explicitly RETAINED — it is reachable
  wherever a USB host is enumerated, which includes the self-hosted
  integration-uart/integration-radio runners and their Korad power-cycle
  and re-enumeration steps. Fork delta drops from +21/-1 to +11/-0.

Old branches feat/proves-usp-radio are retained on both forks pending
review of this pin; pruned branches are retained for recoverability.

Co-Authored-By: Claude <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

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: ASSERTIVE

Plan: Pro Plus

Run ID: c07732cf-06e7-467b-bf5f-d5f867cb1d01

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

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

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

Prunes the last carried Zephyr change (OSSF/zephyr#1, CDC-ACM TX-FIFO drain
on disable) and with it the whole zephyr fork dependency. west.yml now
resolves zephyr through the default upstream remote at 1f6485ec
("release: Zephyr 4.4.1"), so the tree carries no Zephyr patches at all.

Why #1 is not necessary:

- The failure it addresses needs a host that permanently stops issuing IN
  tokens until a physical replug — a macOS cdc_acm quirk. TX_FIFO_BUSY is
  otherwise cleared on normal TX completion (usbd_cdc_acm.c:327), on ANY
  bulk-IN error completion (:297), and on the alloc/enqueue failure paths
  (:672, :686). Only "no completion ever fires" latches it.

- The integration runner is Linux: tools/ci/detect-board-tty.sh walks
  /sys/bus/usb/devices/*/idVendor and references /dev/serial/by-id. On
  Linux, closing the port kills the IN URBs and reopening resumes them, so
  the queued transfer completes, :327 clears BUSY and :331 reschedules the
  drain. It self-recovers without the patch.

- The Korad power-cycle sequence cuts power to the board, resetting the MCU
  and data->state with it, so the disable() hunk buys nothing there.

- The PR's stated premise is also overstated: it claims the error path
  "only clears the flag on disconnect/cancel (ECONNABORTED)", but the clear
  at :297 fires for any bulk-IN error; ECONNABORTED only selects log level.

Phase A/B never ran a negative control for this patch — every soak ran with
it present. This CI run IS that control: if integration-uart and
integration-radio pass on stock Zephyr, the patch was bench ergonomics, not
flight or CI necessity.

Also resolves the lib/fprime-zephyr conflict with #480 in favour of the
trimmed pin 3da24b8 (= #480's bc3b6b6 minus the pruned re-arm semaphore).

KNOWN NIT: the comment block above the zephyr entry still describes the old
fork and needs updating to match this pin.

Co-Authored-By: Claude <noreply@anthropic.com>
@Mikefly123
Mikefly123 merged commit 60eb485 into feat/usp-radio Aug 2, 2026
7 checks passed
@Mikefly123
Mikefly123 deleted the chore/usp-trim-repin branch August 2, 2026 23:31
@github-project-automation github-project-automation Bot moved this to Done in V1.X.X Aug 2, 2026
Mikefly123 added a commit that referenced this pull request Aug 3, 2026
Resolves the two conflicts from `git merge origin/main`:

1. ReferenceDeployment/Top/topology.fpp — refactor-vs-edit. Kept our
   `include "RadioTopology.fppi"` (CMake selects the per-board variant
   before the FPP autocoder runs) and ported ALL of main's PR #443
   startup-quiescence wiring into BOTH RadioTopology_{Lora,Usp}.fppi.
   Nothing from either side was dropped.

   Also adopts main's StartupManager port renames:
     sequenceStarted  -> startupsequenceStarted
     completeSequence -> startupCompleteSequence

   Usp variant binds the radio-side quiescence ports to uspRadio, whose
   first-start signal is named radioFirstStart (radio-agnostic); both
   ends are Fw.Signal so it binds to startupManager.loraFirstStart.

2. lib/fprime-zephyr — pinned to 9e3bedc (feat/proves-usp-radio-trimmed:
   fprime-zephyr main/PR #18 merged in, GFSK RX/TX wedge fix PR #23,
   P5_GMSK_83K profile PR #20).

Also:
- RadioPacketsRadio_Usp.fppi: drop uspRadio.ProfileTableVersion. That
  constant telemetry channel was deleted upstream in fprime-zephyr
  2830b73 ("trim UspRadio changeset for minimal diff"); leaving it here
  fails fpp-check against the new pin.
- west.yml: refresh the stale comment above the zephyr entry. The
  revision has pointed at stock upstream v4.4.1 since #487; the comment
  still described the OSSF fork and its carried patches.

Verified:
- fpp-check clean (rc=0) for BOTH deployment variants, with the CMake
  RadioTopology/RadioInstances/RadioPackets* symlinks pointed at each
  variant in turn. No startupManager/radio quiescence port appears in
  the fpp-check --unconnected output for either variant.
- make test-unit: 12/12 pass, including test_TxOutcomePolicy.

Co-Authored-By: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant