Repository navigation
Export the C6/C61 continuous-capture engine (RINGINFO?/RINGCAP) - #14
Open
juicebox6030 wants to merge 1 commit into
Open
juicebox6030 wants to merge 1 commit into
juicebox6030 wants to merge 1 commit into
Conversation
main/common/ring_capture.c is compiled for ESP32-C61 and ESP32-C6, and main/families/c5_c6_c61/receiver.c already drives it from ring_test(), but the raw-IQ export of that engine exists only in main/targets/esp32s3/receiver.c. It is unreachable on the chips whose ring is already running. This wires the export to the family's existing ring entry point, ring_test(), rather than adding a second call site in the shared handle_command() -- which C5 also compiles, without the engine, and which would then need its own target guard and forward declaration. handle_command() is left byte-identical. One file, 89 insertions. No new files, no main/targets/ edits, no build-system or sdkconfig change, and no test changes: tests/ passes unmodified. The export code is the same definition already exercised on the S3 for this engine, adapted for the family's two-bank geometry and made const-correct (ring_test takes const char *), and its rate validation uses the family's own ring_capture_rate_hz() rather than a copied literal -- the S3's 0/1/6 set is wrong on the C61, whose table is six rates. Replaces the earlier version of this branch, which additionally carried defensive edits to main/common/burst_serial.c. Those are dropped: neither was a memory-safety defect, and they are a separate change with a separate argument. Body carries the caveats stated in the pull request description, including the$ explicit non-claims.
juicebox6030
force-pushed
the
c6-c61-continuous-capture-export
branch
from
October 18, 2026 22:39
6f9ad52 to
c955f7b
Compare
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.
What this changes
main/common/ring_capture.cis compiled for ESP32-C61 and ESP32-C6 (main/CMakeLists.txt), andmain/families/c5_c6_c61/receiver.calready drives that engine from itsring_test()handler. Butthe raw-IQ export of the engine —
RINGINFO?andRINGCAP— exists only inmain/targets/esp32s3/receiver.c, so it is unreachable on the chips whose ring is alreadyrunning. That looks like a gap in the tree rather than a deliberate choice, so this PR wires the
export into the C5/C6/C61 family.
One file, one thing, 117 diff lines:
main/families/c5_c6_c61/receiver.cesp_heap_caps.hinclude; theRINGINFO?/RINGCAPexport;CAPSadvertisesRINGINFO RINGCAPNo new files, no
main/targets/edits, no build-system change, nosdkconfigchange, and no testchanges — your own
tests/suite passes unmodified against this patch (see How to verify).The shape, and why it is this small
Your ESP32-S3 target keeps one ring dispatcher: helpers above
ring_command(), a single dispatchin
handle_command, and the export commands answered by the dispatcher itself. The family receiveralready has exactly one ring entry point,
ring_test(), which owns every other continuous-capturecommand on this target. So the export is answered from there:
That choice is not cosmetic.
handle_command()is shared by C5 as well as C6/C61, whilering_capture.cis compiled only for C6/C61, so a second call site there would need its own targetguard and a forward declaration to stay linkable on C5. Reaching the export through
ring_test()keeps the guard in the one place that already has it, leaves
handle_command()byte-identical, andmeans a call through
ring_test()— which your tests already stub — needs no new stub.The export is declared with a
const char *lineparameter here, matching your ownring_command(const char *line). The copy this was composed from declares itchar *because theother profile in this project calls it from a
char *linehandler; the family calls it fromring_test(const char *line), so leaving the parameter non-const would make the caller discardthe qualifier and add a
-Wdiscarded-qualifierswarning to every maintainer build.The C61 rate check, and why it is not a copied literal
The copy's entry point hard-coded
rate != 0 && rate != 1 && rate != 6. That literal is yourESP32-S3 validation, verbatim, and it is correct there and on the C6 — both have exactly those three
rates. It is wrong for the C61, which this same family receiver compiles and whose table in
main/common/ring_capture.cis six rates (0..5, 80 down to 4 Msps). A copied literal would rejectevery C61 rate except
0/1/6and wave6through to the engine's own check.The family already owns the target-aware predicate a few lines below this one —
ring_test()validates with exactly!ring_capture_rate_hz(rate), andring_capture.hdeclares it.The contributed copy uses the same call, so the accepted set is correct on both targets by
construction:
The
RINGINFO?comment likewise said "Two 128 KiB banks". That is the C6 (RING_BANK_STRIDE0x20000); the C61 falls back to the0x10000default stride. The comment now states thereservation without a size that only one of the two targets has. The
RINGINFO?output isunchanged in both cases — it reports
RING_BANKSandRING_THRESHOLD, which are target-correctalready.
The one adaptation worth a reviewer's eye
The contributed code reads
This is a target adaptation, not a repair. The expression it is adapted from reads
r->units > 2 ? r->cap[2].count : 0; C's conditional operator does not evaluate that arm unlessunits > 2, and both the command parser and the engine already rejectcapture_units > RING_BANKS.On a two-bank part the third slot is never read. The literal zero makes the two-bank assumption
explicit instead of relying on a guard elsewhere, and the entry check refuses
units > RING_BANKSat the command boundary as well. Nothing here is memory-unsafe today.
How to verify
The generator is deterministic and refuses to run on anything but a verified copy of the pinned
tree. It rewrites the copy it is given, so pass a throwaway copy:
The patched source is what reproduces (
diff_lines: 117, and the file applies cleanly tof86da818):sha256sum build-topic-r1/copy/main/families/c5_c6_c61/receiver.c # a571e22d3aa4220996172ddb53f7f3359dc0e6ec3d5e30498f119d173cb86f87The patch was built against your own untouched
sdkconfig.defaults+sdkconfig.defaults.esp32c6+partitions.csv, withtools/generate_version.pyleft in place:That is the compiler's own
: warning:count over the whole build, which is 0 on all three.The five
CMake Warninglines the build does emit are IDF's owncomponents/esp_wifi/wpa_supplicantprivate-include validation, unchanged by this patch andpresent without it. There are no compiler warnings at all, on any of the three, including
-Wdiscarded-qualifiers.esp32c5is the target worth calling out, becausemain/CMakeLists.txtroutes it to this samefamilies/c5_c6_c61/receiver.cwhile compilingmain/common/ring_capture.conly for the C6 and C61.C5 is therefore the one target where the added file-level
esp_heap_caps.hinclude and thetarget-guarded export must coexist with the engine absent — the case that would break if the
guard or the include were placed wrongly. It builds clean, and
esp32c5is in your own CI matrix(
python3 tools/build_firmware.py --matrix).receiver.ccompiles to an object carrying theRINGINFO RINGCAPtoken, and the link producesesp_sdr.bin— 798,496 bytes foresp32c6, 757,584 foresp32c61, 803,536 foresp32c5. Theimage hash is not a reproducibility claim:
generate_version.pystamps a UTC timestamp, soesp_sdr.bindiffers on every build. Your own host suite passes on the patched tree with no testedits:
Upstream's C6 defaults already carry what this feature needs, and say why:
So no
sdkconfigchange belongs in this PR. Do not enable the interrupt watchdog and then runSPEC— the C6 does its FFT work insidering_capture.c's interrupt-masked span and the devicecrashes (observed as a Guru Meditation mid-transaction).
On hardware, your own client is the test:
python3 tools/check_spectrum.py --port /dev/ttyACM0 --milliseconds 3000 --stats— it readsSPECINFO?and exercises every advertised profile. On the equivalent image it exits 0 across all12 C6 profiles. For the export itself,
RINGINFO?reports the heap and bank geometry, andRINGCAP <units> <rate>streams the raw IQ with aRINGDATAheader, the samples, and aRINGCAPreport line.
What is proven, and what is not
Proven, with the receipt named. The patched source reproduces from the tracked generator (stable
hash across runs), the diff touches exactly one file and adds none, and applies cleanly to
f86da818(git apply --check --stat→1 file changed, 89 insertions(+)). It compiles foresp32c6,esp32c61andesp32c5with your own defaults, exit code 0 on all three, zerocompiler warnings on all three. Your
tests/suite passes unmodified on the patched tree — 59tests, no edits, run against a copy carrying the regenerated patch. The
RINGCAPhardware figures below were measured on theauthor's own build, not this trial; see item 1.
RINGCAP's device-emitted CRC32 matched the host'szlib.crc32exactly(
2813587756), so the checksum convention is device-confirmed.SPECatn = 256ran for 60 s at16 MS/s with 960,012,795 pairs over 60,000,810 µs × 16 MS/s = 960,012,960 pair-times
(0.99999983), 39,039 CRC-valid frames of the expected FFT shape, a monotonic sample index, zero
drops and status 0. State the second number too: 39,039 FFT windows analysed of 78,077 window
events, i.e. 39,038 abandoned — so 9,993,984 of 960,012,795 pairs (1.04 %) are transformed. The
acquisition is continuous; the spectral output is sampled windows over it, and the end line carries
both counts.
Not proven — stated here rather than left for a reviewer to discover.
trial keeps upstream's UART transport and console enabled; the measured image used a different
profile. The trial therefore proves composition and buildability, not that this exact image was
measured.
CONFIG_ESPTOOLPY_FLASHSIZE_2MB=ywhile its ownpartitions.csvdeclares a 4 MB factorypartition. Pre-existing, unaffected by this change, noted only so it is not attributed here.
accuracy, LO placement, passband and trigger alignment are unestablished, and no RF source was
characterised.
n ≥ 512is not a continuum on this chip.SPEC … 512returns successfully havingconsumed 2.4 % of the samples. A status 0 and a plausible frame count do not prove continuous
acquisition — only the pair ratio does.
SPECINFO?'s sixth profile field is themachine-checkable discriminator (
1forn == 256over USB,0otherwise).n = 256the 60 s run analysed 39,039 of 78,077 windows — 1.04 % of the acquired pairs. Your owndocs/spectrum.mdsays it: continuous capture "keeps the RF writer running, but the CPU analyzesonly selected FFT windows: it does not deliver every sample."
RINGINFO?/RINGCAPon the C6 in this exact form. The code path is theone already exercised on the S3 for the same engine, adapted only for the two-bank geometry, and
it compiles and links for both
esp32c6andesp32c61; it has not been exercised on a C6 deviceby this author.
ring_test(), whichsits above the line your host tests slice the file at (
#define BURST_MAX_RATE), sotest_c6_commands.py/test_c5_c61_commands.pycompile the shared handler and stubring_test()rather than exercising it. That is deliberate — it is also why this patch needs no test change —
but it does mean
RINGINFO?/RINGCAPparsing is covered by review and by the S3 precedent, notby a host test.
built the three that reach
families/c5_c6_c61/receiver.c(esp32c6,esp32c61,esp32c5). Theother eight never compile this file, so the patch is inert on them — but that is reasoning from
main/CMakeLists.txt, not a build, and your CI is the authority. This is exactly what the PR'sown check run is for. (At the time of writing the workflow on this PR reads
action_requiredwith0 jobs executing — a fork first-contribution approval gate, not a build result. Nothing has
been compiled on the runner, so treat these local builds as the only build evidence so far.)
Two judgement calls a maintainer may want to overrule
CAPSadvertisesRINGINFO, while the parser answersRINGINFO?. Your ESP32-S3 ladder does thesame thing — it advertises the bare
RINGtoken, notRINGINFO?— so this follows your ownconvention: the ladder names the capability family, not the literal query string. The differentiated
ladder is
SPEC/SPECN/SPECCAPS/SPECSTAT, which this patch does not touch. If you would ratherit read
RINGINFO?, that is a one-token change.The dated
/* Modified 2026-10-18: … */notice above your header. It records what changed andthat upstream attribution stands, which the contributing project's process requires. Your sources
carry no
SPDX-License-Identifierand this patch adds none. If you would prefer the contribution beplain upstream style, delete the notice — nothing depends on it.
License and provenance
attribution are preserved untouched in the file.
SPDX-License-Identifierline. The upstream source carries none (ituses a plain
/* … */header) and the generator does not insert one —grep -c SPDXon the patchreturns
0./* Modified 2026-10-18: … */notice above the existing upstreamheader, naming what changed and stating that original upstream source and license attribution
remain.
no generated or compiled artifacts are added.
tools/generate_version.pyis untouched.🤖 Generated with Claude Code