Skip to content

SoapySDDC: Add ADC frequency control and fix frequency offset bug#240

Open
SteamedFish wants to merge 5 commits intoik1xpv:masterfrom
SteamedFish:master
Open

SoapySDDC: Add ADC frequency control and fix frequency offset bug#240
SteamedFish wants to merge 5 commits intoik1xpv:masterfrom
SteamedFish:master

Conversation

@SteamedFish
Copy link
Contributor

SoapySDDC: Add ADC frequency control and fix frequency offset bug

This PR adds two improvements to the SoapySDDC module:

1. ADC Frequency Control

Adds support for configuring ADC sampling frequency through the SoapySDR Settings API, enabling users to adjust the ADC rate for both high-ADC devices (RX-888 mkII) and standard devices.

Features:

  • New adc_frequency setting in SoapySDR API
  • Auto-detect device capability: 128 MHz for high-ADC devices (RX888/RX888r2/RX888r3/RX999), 64 MHz otherwise
  • Frequency range: 50-140 MHz (high-ADC devices), 50-64 MHz (standard devices)
  • Supported sample rates:
    • High ADC mode (128 MHz): 64M, 32M, 16M, 8M, 4M, 2M Hz
    • Standard mode (64 MHz): 32M, 16M, 8M, 4M, 2M Hz
  • Dynamically updates available sample rates based on ADC frequency

2. Fix Frequency Offset Bug at 32MHz Sample Rate

Fixes a long-standing bug where getFrequency() returned a hardcoded 8MHz offset when sample rate was 32MHz. This caused spectrum display to incorrectly start at 8MHz instead of 0Hz, leaving the 0-8MHz range empty.

Impact:

Affects users using 32MHz sample rate with SoapySDR clients that query frequency with the name parameter
Bug existed since the initial SoapySDR implementation (commit 11fe138)
Fix:

Remove hardcoded offset logic
All sample rates now correctly return actual center frequency

Adds support for configuring ADC sampling frequency through SoapySDR
Settings API, enabling users to adjust the ADC rate for high-ADC devices
(RX-888 mkII) and standard devices.

Changes:
- Add adc_frequency setting in getSettingInfo() and listSettings()
- Implement readSetting() and writeSetting() for ADC frequency
- Add supportsHighADCFrequency() method to detect device capability
- Auto-detect default: 128 MHz for high-ADC devices, 64 MHz otherwise
- Frequency range: 50-140 MHz (high-ADC), 50-64 MHz (standard)
- Call RadioHandler.UpdateSampleRate() to apply ADC frequency changes

Tested with RX-888 mkII, confirmed ADC frequency control works correctly.
Remove hardcoded 8MHz offset in getFrequency() when sampleRate is 32MHz.
This offset caused spectrum display to start at 8MHz instead of 0Hz,
leaving 0-8MHz range empty.

The bug existed since the initial SoapySDR implementation.
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the SoapySDDC SoapySDR driver to (1) expose ADC sampling frequency control via the Settings API and (2) correct an incorrect frequency offset returned by getFrequency(name=...) at 32 MHz sample rate.

Changes:

  • Add an adc_frequency SoapySDR setting with capability-based defaults and ranges (64 MHz vs 128 MHz class devices).
  • Update sample-rate selection/listing to conditionally include 64 MHz output when the ADC is in the “high” regime.
  • Remove the hardcoded 8 MHz offset in getFrequency(..., name) for 32 MHz sample rate.
  • Add apt-get update before installing build dependencies in CI.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.

File Description
SoapySDDC/SoapySDDC.hpp Adds new Settings API hook (readSetting) and declares a helper for high-ADC capability detection.
SoapySDDC/Settings.cpp Implements adc_frequency setting, high-ADC detection, sample-rate list/selection changes, and removes the 32 MHz frequency offset hack.
.github/workflows/cmake.yml Makes CI installs more reliable by running apt-get update before apt-get install.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…d error safety

Addresses 5 of 6 AI code review comments on PR ik1xpv#240:

1. **Comment ik1xpv#1 (Performance)**: Cache supportsHighADCFrequency() result in
   getSettingInfo() to avoid redundant calls.

2. **Comment ik1xpv#2 (CRITICAL - Uninitialized variable)**: Initialize samplerateidx
   in constructor to match default sampleRate=32MHz. Previously undefined behavior
   if activateStream() called before setSampleRate().

3. **Comment ik1xpv#4 (HIGH - Architecture)**: Replace hardcoded sample rate switch
   statements with dynamic calculation based on current ADC frequency. Matches
   ExtIO reference implementation formula:
   rate = (adcfreq/64.0) * (2^idx) * 2.0 with Nyquist validation.

4. **Comment ik1xpv#5 (HIGH - API consistency)**: Make listSampleRates() dynamically
   generate rates from current adcnominalfreq instead of returning fixed list.

5. **Comment ik1xpv#6 (CRITICAL - Exception safety)**: Add try-catch exception handling
   for std::stoul() in writeSetting("adc_frequency"). Prevents crash on invalid
   input. Validates uint32_t range and hardware-specific frequency limits.

Comment ik1xpv#3 (const_cast in supportsHighADCFrequency) deferred - requires Core API
changes affecting all platforms.

Changes:
- Add helper methods: computeSampleRateFromIndex(), findSampleRateIndex()
- Add SoapySDR/Logger.hpp include for error logging
- Total: +97 insertions, -80 deletions

Tested: Builds cleanly with gcc 15.2.1, SoapySDR module links successfully.
When ADC frequency is changed via writeSetting("adc_frequency"), the
RadioHandler.UpdateSampleRate() is called correctly, but the sampleRate
member variable was not recomputed to reflect the new ADC clock.

This caused getSampleRate() to return stale values after ADC frequency
changes until the stream was restarted.

Fix: Recompute sampleRate from current samplerateidx after ADC update,
following ExtIO's SetOverclock pattern (ExtIO_sddc.cpp:854-874). If the
rate becomes invalid (Nyquist violation), reset to index 4 (mid-range).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants