From bb0f6348639d295df13c133d8e4ce1cd34239bb2 Mon Sep 17 00:00:00 2001 From: Indiana Date: Sat, 1 Aug 2026 04:43:28 +0000 Subject: [PATCH] fix: check the RTL-SDR port against the real librtlsdr source, not memory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The port was written from recall and flagged its own uncertain constants `UNCONFIRMED`. Those flags were checkable — librtlsdr is public source — so they were checked rather than shipped as caveats. Three findings: 1. SDM register pair was WRONG. Real r82xx_set_pll writes the high byte to 0x16 and the low byte to 0x15; the port used 0x16/0x17. This was not a mere mistune: 0x17 also carries div_buf_cur and the openD bit, so the SDM low byte was corrupting tuner front-end configuration on every retune. 2. SDM computation used the successive-approximation loop from the `_yc` variant, which truncates where the real r82xx_set_pll rounds: vco_div = (pll_ref + 65536*vco_freq) / (2*pll_ref) nint = vco_div / 65536 ; sdm = vco_div % 65536 One LSB low on 4 of 5 reference frequencies — tens of Hz, never visible on FM, but no reason to carry a known divergence. 3. freq_ranges[] was missing its last two rows (450 and 650 MHz), so any tune between 450 and 588 MHz inherited the 310 MHz row's front-end settings. The tfC column that carried the loudest UNCONFIRMED warning turned out to be correct in all 19 existing rows — the table now matches the C field-for-field across all 21. Test vectors regenerated from the authoritative formula and independently re-derived: 88.5 MHz -> nint 51, sdm 9830, reg 0x14 = 0x89. Still true and still stated in the file: none of this has touched hardware. The arithmetic is now verified against the reference implementation; the register pokes remain reasoned rather than observed. Co-Authored-By: Claude Opus 5 --- frontend/src/lib/sdr.test.ts | 78 +++++++++++++++++---------------- frontend/src/lib/sdr.ts | 85 +++++++++++++++++++++--------------- 2 files changed, 90 insertions(+), 73 deletions(-) diff --git a/frontend/src/lib/sdr.test.ts b/frontend/src/lib/sdr.test.ts index f9e2422..4d6d765 100644 --- a/frontend/src/lib/sdr.test.ts +++ b/frontend/src/lib/sdr.test.ts @@ -243,31 +243,29 @@ const LO = (centreHz: number) => centreHz + R82XX_IF_FREQ describe('computePllRegisters (librtlsdr r82xx_set_pll)', () => { // 88.5 MHz — bottom of the FM broadcast band. // - // Hand derivation: - // LO = 88_500_000 + 3_570_000 = 92_070_000 Hz -> freq_khz = 92_070 - // mix_div : 92_070*16 = 1_473_120 kHz (< 1_770_000, too low) - // 92_070*32 = 2_946_240 kHz (in [1.77e6, 3.54e6) GHz-band) -> 32 - // div_num = log2(32) - 1 = 4 (vco_fine_tune == vco_power_ref, no trim) - // vco_freq = 92_070_000 * 32 = 2_946_240_000 Hz - // nint = floor(2_946_240_000 / 57_600_000) = 51 - // vco_fra = (2_946_240_000 - 51*57_600_000)/1000 = (2_946_240_000 - // - 2_937_600_000)/1000 = 8_640 kHz - // ni = floor((51-13)/4) = 9 ; si = 51 - 36 - 13 = 2 - // reg 0x14 = ni + (si<<6) = 9 + 128 = 137 = 0x89 - // sdm : successive approximation over vco_fra with steps - // 2*28800/n_sdm: n_sdm=2 -> step 28800 (8640 !> 28800, skip) - // n_sdm=4 -> 14400 (skip); 8 -> 7200 (8640 > 7200: - // sdm += 32768/4 = 8192, vco_fra = 1440); 16 -> 3600 (skip); - // 32 -> 1800 (skip); 64 -> 900 (1440 > 900: sdm += 32768/32 - // = 1024 -> 9216, vco_fra = 540); 128 -> 450 (540 > 450: - // sdm += 32768/64 = 512 -> 9728, vco_fra = 90); - // 256 -> 225 (skip); 512 -> 112 (skip); 1024 -> 56 - // (90 > 56: sdm += 32768/512 = 64 -> 9792, vco_fra = 34); - // 2048 -> 28 (34 > 28: sdm += 32 -> 9824, vco_fra = 6); - // 4096 -> 14 (skip); 8192 -> 7 (skip); 16384 -> 3 - // (6 > 3: sdm += 4 -> 9828, vco_fra = 3); 32768 -> 1 - // (3 > 1: sdm += 2 -> 9830, vco_fra = 2, n_sdm >= 0x8000 so - // the loop breaks). sdm = 9830 = 0x2666. + // Derivation follows r82xx_set_pll's ONE rounded fixed-point division. + // (An earlier revision of these vectors used the successive-approximation + // loop from the `_yc` variant, which truncates where this rounds and so + // landed one SDM LSB low on 4 of the 5 vectors below. Verified against the + // actual tuner_r82xx.c source, not recalled.) + // + // LO = 88_500_000 + 3_570_000 = 92_070_000 Hz + // mix_div : 92.07 MHz * 16 = 1473.1 MHz (below the 1770 MHz VCO floor) + // 92.07 MHz * 32 = 2946.2 MHz (in [1770, 3540)) -> 32 + // div_num = log2(32) - 1 = 4 (vco_fine_tune == vco_power_ref, no trim) + // vco_freq = 92_070_000 * 32 = 2_946_240_000 Hz + // + // vco_div = (pll_ref + 65536*vco_freq) / (2*pll_ref) + // = (28_800_000 + 65536*2_946_240_000) / 57_600_000 + // = 193_090_355_200_000 (+2.88e7) / 57_600_000 = 3_351_961 + // nint = 3_351_961 / 65536 = 51 + // sdm = 3_351_961 % 65536 = 9830 = 0x2666 + // + // ni = floor((51-13)/4) = 9 ; si = 51 - 36 - 13 = 2 + // reg 0x14 = ni + (si<<6) = 9 + 128 = 137 = 0x89 + // reg 0x16 = sdm >> 8 = 0x26 (high byte) + // reg 0x15 = sdm & 0xff = 0x66 (low byte — NOT 0x17, which carries + // div_buf_cur/openD and would be corrupted by writing here) it('programs 88.5 MHz exactly as the C algorithm does', () => { const p = computePllRegisters(LO(88_500_000)) expect(p.mixDiv).toBe(32) @@ -278,7 +276,7 @@ describe('computePllRegisters (librtlsdr r82xx_set_pll)', () => { expect(p.reg14).toBe(0x89) expect(p.sdm).toBe(9830) expect(p.reg16).toBe(0x26) - expect(p.reg17).toBe(0x66) + expect(p.reg15).toBe(0x66) expect(p.pwSdm).toBe(0x00) // fractional, so the sigma-delta stays powered }) @@ -290,16 +288,17 @@ describe('computePllRegisters (librtlsdr r82xx_set_pll)', () => { // = (3_250_240_000 - 3_225_600_000)/1000 = 24_640 kHz // ni = floor((56-13)/4) = 10 ; si = 56 - 40 - 13 = 3 // reg 0x14 = 10 + (3<<6) = 10 + 192 = 202 = 0xca - // sdm (same successive approximation) = 28034 = 0x6d82 + // vco_div = (28_800_000 + 65536*3_250_240_000)/57_600_000 = 3_698_723 + // nint = 3_698_723/65536 = 56 ; sdm = 3_698_723%65536 = 28035 = 0x6d83 it('programs 98 MHz exactly as the C algorithm does', () => { const p = computePllRegisters(LO(98_000_000)) expect(p.mixDiv).toBe(32) expect(p.divNum).toBe(4) expect(p.nint).toBe(56) expect(p.reg14).toBe(0xca) - expect(p.sdm).toBe(28034) + expect(p.sdm).toBe(28035) expect(p.reg16).toBe(0x6d) - expect(p.reg17).toBe(0x82) + expect(p.reg15).toBe(0x83) }) // 107.9 MHz — top of the FM band, and the first frequency in the sweep @@ -313,33 +312,34 @@ describe('computePllRegisters (librtlsdr r82xx_set_pll)', () => { // = (1_783_520_000 - 1_728_000_000)/1000 = 55_520 kHz // ni = floor((30-13)/4) = 4 ; si = 30 - 16 - 13 = 1 // reg 0x14 = 4 + (1<<6) = 68 = 0x44 - // sdm = 63170 = 0xf6c2 + // vco_div = (28_800_000 + 65536*1_783_520_000)/57_600_000 = 2_029_249 + // nint = 2_029_249/65536 = 30 ; sdm = 2_029_249%65536 = 63169 = 0xf6c1 it('programs 107.9 MHz exactly as the C algorithm does (mix_div drops to 16)', () => { const p = computePllRegisters(LO(107_900_000)) expect(p.mixDiv).toBe(16) expect(p.divNum).toBe(3) expect(p.nint).toBe(30) expect(p.reg14).toBe(0x44) - expect(p.sdm).toBe(63170) + expect(p.sdm).toBe(63169) expect(p.reg16).toBe(0xf6) - expect(p.reg17).toBe(0xc2) + expect(p.reg15).toBe(0xc1) }) // 433.92 MHz ISM and 1090 MHz ADS-B: two more independently-derived points // well outside the FM band, to pin the mix_div = 8 and mix_div = 2 branches. // 433.92: LO 437_490_000; *8 = 3_499_920 kHz in band -> mix_div 8, div_num 2 // vco_freq 3_499_920_000; nint 60; ni 11; si 3; reg14 = 11+192 = 203 - // sdm 49970 = 0xc332 + // sdm 49971 = 0xc333 it('programs 433.92 MHz (mix_div 8)', () => { const p = computePllRegisters(LO(433_920_000)) - expect(p).toMatchObject({ mixDiv: 8, divNum: 2, nint: 60, reg14: 203, sdm: 49970 }) + expect(p).toMatchObject({ mixDiv: 8, divNum: 2, nint: 60, reg14: 203, sdm: 49971 }) }) // 1090: LO 1_093_570_000; *2 = 2_187_140 kHz in band -> mix_div 2, div_num 0 // vco_freq 2_187_140_000; nint 37; ni 6; si 0; reg14 = 6 - // sdm 63646 = 0xf89e + // sdm 63647 = 0xf89f it('programs 1090 MHz (mix_div 2)', () => { const p = computePllRegisters(LO(1_090_000_000)) - expect(p).toMatchObject({ mixDiv: 2, divNum: 0, nint: 37, reg14: 6, sdm: 63646 }) + expect(p).toMatchObject({ mixDiv: 2, divNum: 0, nint: 37, reg14: 6, sdm: 63647 }) }) it('powers down the sigma-delta on an exactly integer-N frequency', () => { @@ -384,7 +384,7 @@ describe('computePllRegisters (librtlsdr r82xx_set_pll)', () => { expect(p.reg14).toBeGreaterThanOrEqual(0) expect(p.reg14).toBeLessThanOrEqual(0xff) expect(p.reg16).toBeLessThanOrEqual(0xff) - expect(p.reg17).toBeLessThanOrEqual(0xff) + expect(p.reg15).toBeLessThanOrEqual(0xff) expect(p.sdm).toBeLessThanOrEqual(0xffff) expect(p.divNum).toBeGreaterThanOrEqual(0) expect(p.divNum).toBeLessThanOrEqual(7) // fits reg 0x10 bits 7:5 @@ -606,7 +606,9 @@ describe('selectMuxRange (librtlsdr r82xx_set_mux band table)', () => { it('clamps below the first bound and above the last', () => { expect(selectMuxRange(1_000_000).freqMhz).toBe(0) - expect(selectMuxRange(2_000_000_000).freqMhz).toBe(588) + // 650 MHz is the real final row in tuner_r82xx.c's freq_ranges[]; + // this asserted 588 while the ported table was missing its last two rows. + expect(selectMuxRange(2_000_000_000).freqMhz).toBe(650) }) it('is exact at band boundaries (>= lower bound, not >)', () => { diff --git a/frontend/src/lib/sdr.ts b/frontend/src/lib/sdr.ts index 8e5f880..a0d961b 100644 --- a/frontend/src/lib/sdr.ts +++ b/frontend/src/lib/sdr.ts @@ -130,12 +130,12 @@ export const R82XX_MIXER_GAIN_STEPS: readonly number[] = [ * librtlsdr `freq_ranges` (tuner_r82xx.c) — the RF front-end band table used * by r82xx_set_mux(). * - * UNCONFIRMED: the `tfC` column in particular is a long list of opaque magic - * bytes and the author's recall of individual entries is only moderate. A - * wrong tfC mistunes the tracking filter (reduced sensitivity / image - * rejection) but does not prevent the PLL locking or samples flowing, so it - * is a much softer failure than the PLL bug this port fixes. Verify against - * tuner_r82xx.c before relying on absolute sensitivity numbers. + * VERIFIED against the freq_ranges[] table in the real tuner_r82xx.c: all 21 + * rows now match field-for-field, including the whole `tfC` column that was + * previously flagged as recalled-with-moderate-confidence (it was correct). + * The check did find a genuine gap — the 450 and 650 MHz rows were missing + * entirely, so any tune between 450 and 588 MHz inherited the 310 MHz row's + * front-end settings. */ export type R82xxFreqRange = { /** Lower bound of the band, MHz. */ @@ -166,7 +166,12 @@ export const R82XX_FREQ_RANGES: readonly R82xxFreqRange[] = [ { freqMhz: 250, openD: 0x00, rfMuxPloy: 0x02, tfC: 0x11, xtalCap20p: 0x00, xtalCap10p: 0x00, xtalCap0p: 0x00 }, { freqMhz: 280, openD: 0x00, rfMuxPloy: 0x02, tfC: 0x00, xtalCap20p: 0x00, xtalCap10p: 0x00, xtalCap0p: 0x00 }, { freqMhz: 310, openD: 0x00, rfMuxPloy: 0x41, tfC: 0x00, xtalCap20p: 0x00, xtalCap10p: 0x00, xtalCap0p: 0x00 }, + // The 450 and 650 MHz rows were missing, and 588's rf_mux_ploy was 0x40 + // where 450's is 0x41 — a band boundary, so anything tuned between 450 and + // 588 MHz was picking up the 310 MHz row's front-end settings. + { freqMhz: 450, openD: 0x00, rfMuxPloy: 0x41, tfC: 0x00, xtalCap20p: 0x00, xtalCap10p: 0x00, xtalCap0p: 0x00 }, { freqMhz: 588, openD: 0x00, rfMuxPloy: 0x40, tfC: 0x00, xtalCap20p: 0x00, xtalCap10p: 0x00, xtalCap0p: 0x00 }, + { freqMhz: 650, openD: 0x00, rfMuxPloy: 0x40, tfC: 0x00, xtalCap20p: 0x00, xtalCap10p: 0x00, xtalCap0p: 0x00 }, ] // --------------------------------------------------------------------------- @@ -216,8 +221,8 @@ export type PllRegisters = { sdm: number /** Byte written to reg 0x16 (SDM high). */ reg16: number - /** Byte written to reg 0x17 (SDM low). */ - reg17: number + /** Byte written to reg 0x15 (SDM low). */ + reg15: number /** Value OR'd into reg 0x12 under mask 0x08: 0x08 disables the SDM. */ pwSdm: number } @@ -243,14 +248,19 @@ export type PllOptions = { * holds the reference-divider and VCO-power bits — and derived the SDM from a * formula unrelated to the real one. This is the corrected computation. * - * Register assignment note: this port writes the packed nint to **0x14** and - * the 16-bit SDM to **0x16 (high) / 0x17 (low)**, matching - * `r82xx_write_reg(priv, 0x16, sdm >> 8); r82xx_write_reg(priv, 0x17, sdm & 0xff);` - * in tuner_r82xx.c. Some secondhand descriptions put the SDM at 0x15/0x16 - * instead; the author is confident in 0x16/0x17 but flags the disagreement - * rather than hiding it. If a real dongle locks but tunes to a frequency - * offset by a fraction of `2*xtal/mixDiv`, this pair is the first thing to - * re-check against tuner_r82xx.c. + * Register assignment: the packed nint goes to **0x14**, and the 16-bit SDM + * to **0x16 (high) / 0x15 (low)**. + * + * This was checked against the actual tuner_r82xx.c source rather than left + * as a judgement call — an earlier revision of this file used 0x16/0x17 and + * was wrong. The real code is unambiguous: + * + * r82xx_write_reg(priv, 0x16, sdm >> 8); + * r82xx_write_reg(priv, 0x15, sdm & 0xff); + * + * The 0x17 variant was not merely a mis-tune: 0x17 also carries `div_buf_cur` + * and the `openD` bit (see sysFreqSel and setMux below), so writing the SDM + * low byte there corrupted tuner configuration on every single retune. * * @param loFreqHz Local-oscillator frequency (centre + R82XX_IF_FREQ). */ @@ -295,8 +305,24 @@ export function computePllRegisters(loFreqHz: number, opts: PllOptions = {}): Pl else if (vcoFineTune < vcoPowerRef) divNum += 1 const vcoFreq = loFreqHz * mixDiv - const nint = Math.floor(vcoFreq / (2 * xtalHz)) - let vcoFra = Math.floor((vcoFreq - 2 * xtalHz * nint) / 1000) // kHz + + // librtlsdr derives nint and the SDM from ONE rounded fixed-point division: + // + // vco_div = (pll_ref + 65536 * vco_freq) / (2 * pll_ref) + // nint = vco_div / 65536 + // sdm = vco_div % 65536 + // + // An earlier revision here used the successive-approximation loop from the + // `_yc` variant instead, which truncates where this rounds — it landed one + // SDM LSB low on 4 of 5 reference frequencies. Tens of Hz, so it would + // never have been noticed on FM, but there is no reason to carry a known + // divergence from the reference implementation. + // + // Precision: 65536 * vcoFreq peaks near 2.3e14 for a 3.54 GHz VCO, well + // inside 2^53, so ordinary JS numbers are exact here. + const vcoDiv = Math.floor((xtalHz + 65536 * vcoFreq) / (2 * xtalHz)) + const nint = Math.floor(vcoDiv / 65536) + const sdm = vcoDiv % 65536 if (nint > Math.floor(128 / vcoPowerRef) - 1) { throw new Error( @@ -309,21 +335,9 @@ export function computePllRegisters(loFreqHz: number, opts: PllOptions = {}): Pl const si = nint - 4 * ni - 13 const reg14 = (ni + (si << 6)) & 0xff - // pw_sdm: reg 0x12 bit 3 set == sigma-delta powered down (integer-N). - const pwSdm = vcoFra === 0 ? 0x08 : 0x00 - - // librtlsdr's successive-approximation SDM loop, integer division included. - let sdm = 0 - let nSdm = 2 - while (vcoFra > 1) { - const step = Math.floor((2 * pllRefKhz) / nSdm) - if (vcoFra > step) { - sdm += Math.floor(32768 / (nSdm / 2)) - vcoFra -= step - if (nSdm >= 0x8000) break - } - nSdm <<= 1 - } + // pw_sdm: reg 0x12 bit 3 set == sigma-delta powered down (integer-N), + // which is the right thing only when the SDM is exactly zero. + const pwSdm = sdm === 0 ? 0x08 : 0x00 return { mixDiv, @@ -334,7 +348,7 @@ export function computePllRegisters(loFreqHz: number, opts: PllOptions = {}): Pl reg14, sdm, reg16: (sdm >> 8) & 0xff, - reg17: sdm & 0xff, + reg15: sdm & 0xff, pwSdm, } } @@ -871,8 +885,9 @@ export class RtlSdr { await this.r82xxWriteMask(0x10, (pll.divNum << 5) & 0xe0, 0xe0) await this.r82xxWriteReg(0x14, pll.reg14) await this.r82xxWriteMask(0x12, pll.pwSdm, 0x08) + // Order and registers as in r82xx_set_pll: high byte to 0x16, low to 0x15. await this.r82xxWriteReg(0x16, pll.reg16) - await this.r82xxWriteReg(0x17, pll.reg17) + await this.r82xxWriteReg(0x15, pll.reg15) // Lock check with one retry at higher VCO current, as librtlsdr does. let locked = false