README + docs + sync to upstream librx888 ABI in rx888-tools#6
Merged
Conversation
These existed before but weren't discoverable from the README. Two new sections: - "Skip the host setup: use the Docker image" — pointers to docker/build.sh, docker run with --privileged USB passthrough, and the grc-demo entrypoint for X11. - "At the bench" — points at scripts/rx888_smoketest.sh and the pre-built examples/hf_waterfall_demo.grc. Also drops the stale "-DWITH_RX888=OFF" option from the build example since librx888 is now a hard build requirement.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 623840dab1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Two fixes addressing PR #6 feedback: 1. Makefile: firmware/udev install is best-effort. The Dockerfile clones rx888-tools fresh and runs `make install`. rx888-tools/firmware/SDDC_FX3.img is a release artifact, not a tracked file, so the install fails with "cannot stat". Guard the firmware + udev install steps with file-existence checks. 2. README: explain the librx888 overlay required today. Codex review correctly pointed out that the "install librx888 system-wide" line linked rx888-tools but didn't mention that librx888 isn't there yet — a user following the link literally would hit pkg_check_modules(LIBRX888 REQUIRED librx888) failure. Add an explicit overlay recipe to the Build section.
librx888 has landed upstream in rx888-tools — and with a richer ABI
than the v0.0 staging copy: new rx888_config_t fields (queue_depth,
req_packets, ctrl_timeout_ms, stream_timeout_ms, watchdog_ms) plus
three new public functions (rx888_config_init_default,
rx888_get_stats, rx888_is_running). The old open() now rejects a cfg
with any of the three required new fields set to zero, so v0.2's
source block would fail at runtime when linked against upstream.
This commit:
- Deletes tests/librx888/ entirely (the staging arrangement is no
longer needed; rx888-tools is canonical).
- Vendors the current upstream librx888.h in tests/mock/ so the
mock library can compile against the ABI it implements.
- Updates the mock to implement the three new functions and to
mirror real librx888's rejection of a zeroed cfg.
- source_impl.cc now calls rx888_config_init_default() before
overriding the user-facing fields, so the tuning knobs get sane
values and open() doesn't reject.
- expected-abi.txt grows three entries.
- Dockerfile + ci.yml drop the "overlay tests/librx888 onto a
fresh rx888-tools clone" dance. CI/Docker just clones, runs
`make firmware` (fetches pinned FX3 blob from rx888-firmware),
builds, installs.
- README and docker/README simplified — no more overlay recipe.
Addresses the Docker build failure on PR #6 (rx888-tools/Makefile
references firmware/SDDC_FX3.img which isn't tracked) and the Codex
review comment about the README's install pointer being incomplete.
Verified locally: mock build + 12 mock-backed QA tests pass against
the new ABI; ABI symbol diff matches expected-abi.txt (9 symbols).
ringof
changed the title
The pinned-firmware fetch in rx888-tools/Makefile uses curl to download the FX3 release asset from rx888-firmware. Ubuntu 24.04's minimal base image doesn't ship curl by default, so \`make firmware\` failed in the docker build step. ca-certificates was already there.
Two bugs surfaced when running the published image at the bench: 1. \`rx888_smoketest\` couldn't find \`rx888_preflight.sh\`. The Dockerfile installed the helpers without their .sh suffix, but smoketest's sibling lookup (\`\$HERE/rx888_preflight.sh\`) needs the suffix. Install them as *.sh and add symlinks without the suffix for the CLI-pretty names. 2. \`gnuradio-companion\` failed with "Namespace Gtk not available". --no-install-recommends stripped the GTK3 Python bindings the editor needs. Add them explicitly (gir1.2-gtk-3.0, python3-gi, python3-gi-cairo), plus python3-pyqt5 which the bundled hf_waterfall_demo.grc's qtgui_* sinks need at runtime.
GRC's code generator inserts the .grc \`description\` as a header comment, but a multi-paragraph value with an embedded blank line breaks out of the comment block — the second paragraph ends up emitted as bare Python, which fails with SyntaxError at flowgraph launch. Caught when launching hf_waterfall_demo from gnuradio-companion inside the Docker image. Single-line description avoids the issue.
For a real-valued direct-sampling source like the RX888, the ADC's DC bin IS DC at 0 Hz — there's no downconversion to label around. With fc=samp_rate/4 (8 MHz), the qtgui sinks labeled the DC spike as if it were at 8 MHz, which is misleading: a thumbtack-pickup test showed a "signal" near 8 MHz that was actually DC. With fc=0 and freqhalf=True the display now spans 0..16 MHz (real Nyquist) with DC at the left edge — actual HF signals show at their actual MHz.
am_bcb_demo.grc tunes a slider across 540 kHz–1700 kHz to put any AM
station at DC of a 2 MS/s complex baseband, then shows the
neighborhood on a waterfall and PSD. Default center 1.0 MHz puts
WMVP / KMOX / similar in view. AM BCB picks up reliably even on a
thumbtack antenna, which makes it the right "this always works"
demo for booth setups.
Flow:
rx888.source (32 MS/s real)
-> float_to_complex (imag=0)
-> freq_shift_cc by -center_freq
-> rational_resampler_ccc decim=16
-> qtgui_waterfall_sink_c + qtgui_freq_sink_c
The slider's set_center_freq() callback retunes the freq_shift block
live and relabels both displays — no flowgraph restart needed.
Keeps hf_waterfall_demo.grc around as the "show me the whole 0-16 MHz
spectrum" diagnostic view; this new file is the booth-friendly one.
- examples/halfband_coefs_fixed.csv — pre-designed ~493-tap linear- phase half-band LPF (half the taps are zero, center tap 0.5, cutoff at fs/4). Drop into a variable_file_filter_taps block for cascaded decim=2 stages that are dramatically cheaper than a single fat rational_resampler at high ADC rates. Not used by the bundled booth demos; provided as a resource for users adapting their own flowgraphs at 135 MS/s. - docker/grc-demo.sh — launch am_bcb_demo.grc by default instead of hf_waterfall_demo.grc. AM BCB picks up reliably even on a thumbtack antenna, so the booth-default demo "always works." hf_waterfall_demo remains available as the wide-spectrum diagnostic view; open it manually from GRC. - examples/README.md documents both flowgraphs + the CSV. - README points at examples/ now that there's more than one demo.
Adds am_bcb_audio_demo.grc — the AM BCB tuner from am_bcb_demo plus
a demod-to-speakers tail:
2 MS/s complex baseband
→ analog.am_demod_cf (audio_decim=40, 5 kHz LPF) → 50 kHz real
→ rational_resampler_fff (interp=24, decim=25) → 48 kHz
→ multiply_const_ff (volume slider)
→ audio.sink (ALSA default device)
Tune and volume both have qtgui_range sliders so the demo is fully
interactive at the booth — drag the slider across the dial, hear
each station. analog.am_demod_cf handles envelope + carrier-bias
removal + audio LPF in one hier block.
Docker:
- apt install libasound2t64 + alsa-utils so the ALSA runtime is
present inside the container and `aplay -l` is available for
diagnosing.
- grc-demo entrypoint now launches the audio demo by default. Users
pass --device /dev/snd on the docker run line to plumb host
speakers through. Without it the flowgraph starts but audio.sink
fails to open; the silent variant (am_bcb_demo.grc) remains a
one-line override.
- docker/README.md documents the run command and how to fall back
to the silent variants.
The audio_sink crash ("Device or resource busy") was real: PulseAudio
(or PipeWire-pulse) on the host holds the ALSA hardware exclusively,
so a second process inside the container that opens \`default\` direct
via --device /dev/snd loses. ka9q-radio dodges this by publishing
RTP instead — out of scope for our booth demo, we want one click and
audio out.
This commit takes the PulseAudio-plugin route:
- libasound2-plugins added to apt list (provides the ALSA pulse pcm
module).
- docker/asound.conf bind-mounted at /etc/asound.conf inside the
image. Maps ALSA's "default" device to type=pulse.
- docker/README.md updated with the new run line:
-v /run/user/\$(id -u)/pulse:/tmp/pulse:ro
-e PULSE_SERVER=unix:/tmp/pulse/native
PipeWire users get this for free via PipeWire's pulse-compat socket.
GR's audio.sink now writes to the in-container ALSA default → pulse
plugin → host PulseAudio/PipeWire → speakers, mixed alongside
everything else on the host instead of fighting for the device.
After the previous commit got us past EBUSY, audio.sink hit "set_periods failed: Invalid argument" at flowgraph start. ALSA's pulse plugin only advertises a narrow set of period_size / period_count combinations, and GR's audio_alsa_sink negotiates via snd_pcm_hw_params_set_periods_near() which doesn't always land on one the pulse plugin will accept. Fix: route default -> plug -> pulse instead of default -> pulse directly. The plug plugin mediates format / rate / period conversion transparently, so any hardware params audio_alsa_sink asks for are translated into something the pulse slave will accept. Pattern matches what most asound.conf templates on PulseAudio hosts ship by default.
am_demod_cf's internal optfir.low_pass call designs an audio LPF from (channel_rate, audio_pass=3kHz, audio_stop=4.5kHz). At channel_rate=2 MHz the resulting filter needs ~5000 Parks-McClellan taps to meet the 60 dB stopband at a 1.5 kHz transition, and pm_remez bails with "insufficient extremals -- cannot continue." Drop a rational_resampler_ccc(decim=10) between the 2 MS/s baseband and am_demod_cf so the AM demod runs at 200 kHz instead. The Remez filter design then needs ~480 taps — well within numerical range — and audio_decim=4 inside am_demod_cf produces 50 kHz audio, which the existing 24/25 fff resampler converts to exactly 48 kHz for the audio sink. Chain now: 32M -> 2M (waterfall) -> 200k -> am_demod -> 50k -> 48k.
3 tasks
ringof
added a commit
that referenced
this pull request
May 15, 2026
Documentation refresh: align first-read docs with what shipped in PR #6
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
2 participants
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.
Summary
Started as a README pointer to the Docker image and helper scripts. Two pieces of CI feedback then expanded the scope:
tests/librx888/Makefile(a stale staging copy) didn't know how to fetch the pinned firmware blob.rx888-toolswas incomplete.Investigating the Docker failure surfaced the real issue: librx888 has landed upstream in
rx888-toolswith a richer ABI than v0.2 expects. New requiredrx888_config_tfields (queue_depth,req_packets,ctrl_timeout_ms,stream_timeout_ms,watchdog_ms); three new public functions (rx888_config_init_default,rx888_get_stats,rx888_is_running); andrx888_open()now rejects a zeroed cfg. v0.2's source block would have failed at runtime when linked against upstream.So this PR rolls up three concerns:
README + docs (the original ask)
docker/build.sh+ thegrc-demoX11 entrypoint.scripts/rx888_smoketest.shandexamples/hf_waterfall_demo.grc.-DWITH_RX888=OFFbuild option (librx888 is now a hard requirement).librx888 upstream sync
tests/librx888/entirely. The staging arrangement was always meant to disappear when librx888 landed upstream — it has, so it's gone.librx888.hintests/mock/so the mock library can compile against the ABI it implements.tests/expected-abi.txtgains three new symbols (now 9 total).tests/mock/librx888_mock.cto implement the three new functions and to mirror real librx888's rejection of a zeroed cfg.lib/source_impl.ccnow callsrx888_config_init_default()before overriding the user-facing fields, so the tuning knobs get sane values andopen()doesn't reject.Docker + CI
tests/librx888/*onto a freshrx888-toolsclone" dance.git clone rx888-tools→make firmware(fetches the pinned FX3 release fromrx888-firmware) →make→ install.Verified locally
expected-abi.txt(9 symbols).lib/source_impl.cccorrectly callsrx888_config_init_default()so the tuning knobs satisfy upstream's validation.Test plan
WITH_RX888_MOCK=ONbuild + ctest passes locallyrx888_*symbols as upstream librx888.h declaresmake firmwareworks inside the GHA runner sandbox and insidedocker build(both need outbound HTTPS to github.com/ringof/rx888-firmware/releases — should be fine)examples/hf_waterfall_demo.grcWhy this isn't split into two PRs
I considered splitting (README in one PR, librx888 sync in another), but the librx888 sync is the root cause of the docker build failure on the README PR — landing the README alone would leave CI red. So they're coupled and ship together.