chore(pins): re-pin zephyr + fprime-zephyr onto trimmed integration branches - #487
Merged
Conversation
…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>
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
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
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>
12 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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:
v4.4.1-baseconfirmed+11/-0(drain only) vs+21/-1before.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