Skip to content

Stability fixes: streaming, comms, chip drivers, boards, CLI (batch 8) - #376

Merged
rjonaitis merged 33 commits into
myriadrf:developfrom
dmitrymmm:batch-stability-fixes-8
Aug 3, 2026
Merged

rjonaitis merged 33 commits into
myriadrf:developfrom
dmitrymmm:batch-stability-fixes-8

Conversation

@dmitrymmm

Copy link
Copy Markdown
Contributor

A full-codebase audit pass. 32 fixes, one defect per commit, each verified by
reading the code path; no behavioural change intended beyond correcting the defect.
Builds clean and the unit test suite (59 tests) passes. The Windows-only USB fixes,
the board/PCIe fixes and the RF-configuration fixes still want a bench check, but the
defects are all visible from the code.

comms

  • FT601 (Windows): AbortXfer no longer releases the OVERLAPPED that the following
    WaitForXfer/FinishDataXfer still use, and BeginDataXfer stops leaking the event
    handle it created twice per transfer.
  • FX3 (Windows): BulkTransfer returns the actual transferred count, so short transfers
    are visible to callers instead of looking complete.
  • USBDMAEmulation: transfers that complete short or errored are reported instead of
    being counted as full buffers and parsed as stale samples.
  • PCIe: RunControlCommand bounds the caller length to the 64-byte ioctl packet.

streaming

  • Rx UNIX_EPOCH timestamps: the epoch/front-end-delay correction is applied once per
    user packet instead of once per batched hardware packet.
  • Tx: null meta and asymmetric channel configs (more Rx than Tx channels) no longer
    dereference or memcpy from a null pointer.
  • StreamComposite: sample-tick Tx timestamps keep their tick rate instead of being
    divided by the sample rate twice.
  • The packet pool is clamped so a low/absent sample-rate hint cannot underflow the FIFO
    size to SIZE_MAX.

protocols

  • Custom parameter scaling guards log10 of non-positive values and masks the exponent
    nibble.
  • WriteSerialNumber and SPI_generic no longer build outgoing packets on top of the
    bytes the device wrote back into the shared buffer.

chip drivers

  • LMS7002M: channel B config load reads the ch.B NCO section, IsSynced selects MAC B
    before the readback, and the MCU-assisted write verifies against the chip not the
    cache.
  • embedded lms7002m: VDIV_VCO restored to 0xB9 (not 0xFF), RSSI settling delay
    saturates instead of truncating to 16 bits, the CGEN divider midpoint is clamped
    rather than masked, the calibration save/restore buffers are per-context, and the
    batch flush masks the upper MISO half.
  • CDCM6208: fractional divider no longer wraps its 8-bit integer part, the read-back
    loop stops before the phantom map entry at address 0, and the input-mux recompute
    conditions are corrected.
  • Si5351C: PLL denominator registers are assigned (not OR'd over defaults) and PLLB
    uses its own input frequency.
  • ADF4002: R/N counters are validated against their 14/13-bit fields before packing.

boards

  • LimeSDR-X3: the LMS3 RF switch bit is computed from the per-chip channel instead of a
    negative shift.
  • MMX8: the stream data path is routed through the module's own streamers with null
    guards, and StreamSetup fails when a subdevice stream cannot be created.
  • The Tx test signal is configured even when the Rx test signal is enabled.
  • LimeSDR-XTRX: the GNSS UART test terminates on a read timeout instead of spinning.

FPGA / DSP

  • SetInterfaceFreq fails on a failed LMS7002M readback instead of using uninitialized
    values, and skips bypassed/zero-frequency clocks so the divider math cannot divide by
    zero.
  • FFT window coefficients are swapped under a mutex and read under it on the worker
    thread; the average count is atomic.
  • CFR derives the FIR odd flag from the actual tap count.

legacyapi / CLI

  • LMS_RecvStream reports the Rx timestamp, LMS_DestroyStream tears the shared stream
    down only when no channels remain and matches the align-phase-masked index,
    LMS_Read/WriteParam preserve the current MAC selection, and the NCO frequency
    functions guard a null table pointer.
  • LimePlugin validates max_channels_to_use and releases the calibration stream before
    the devices it references are freed.
  • limeTRX resends the tail of a partial Tx and checks the output file; limeConfig
    resolves a relative --ini against the CWD and frees the device on error; limePPSDO
    selects the register map from the connected device and returns a failure exit code;
    frontendDelayTest checks the StreamCreate result.

claude added 30 commits August 3, 2026 08:42
On the Windows path AbortXfer released the OVERLAPPED and closed its
event, but USBDMAEmulation::AbortAllTransfers follows AbortXfer with
WaitForXfer and FinishDataXfer on the same context, waiting on a closed
handle and releasing the overlapped a second time. BeginDataXfer also
replaced the event that FT_InitializeOverlapped had just created,
leaking one event handle per submitted transfer.

Co-Authored-By: Claude <noreply@anthropic.com>
BulkTransfer returned the requested length even when CyAPI's XferData
completed the transfer short, so callers that compare the returned
count against the expected size (control command round trips) treated
truncated responses as complete and parsed stale buffer bytes. The
Linux path already returns the actual transferred count.

Co-Authored-By: Claude <noreply@anthropic.com>
UpdateProducerStates discarded the byte count from FinishDataXfer and
counted every completion as a full buffer, so an errored, stalled or
short bulk transfer was consumed downstream as a complete batch of
packets, parsing stale buffer contents without any trace of the loss.
Surface the mismatch through the logger so stream corruption after a
bus error is attributable.

Co-Authored-By: Claude <noreply@anthropic.com>
RunControlCommand copied the caller-supplied length into the fixed
64-byte request/response arrays of the on-stack ioctl packet without a
bounds check. The method is part of the exported API, so an oversized
request from outside the tree smashed the stack frame.

Co-Authored-By: Claude <noreply@anthropic.com>
The epoch base and FPGA front-end delay correction sat inside the
per-sub-packet loop while the user packet's timestamp is initialized
once per DMA buffer, so with the default packetsToBatch greater than
one the epoch seconds were added N times and the delay subtracted N
times, putting UNIX_EPOCH Rx timestamps billions of seconds off.

Co-Authored-By: Claude <noreply@anthropic.com>
StreamTxTemplate crashed in two reachable ways: passing meta as null
through the public Transmit overloads dereferenced it right after two
lines that treat it as optional, and a config with more Rx channels
than Tx channels staged packets whose channel B plane was memcpy'd
from a null source pointer, since staging packets are sized for the
larger direction.

Co-Authored-By: Claude <noreply@anthropic.com>
StreamComposite pre-divided the SAMPLE_TICKS timestamp by the sample
rate before handing it to TRXLooper, whose SetTickRate then divided
the stored value by the tick rate again, so a timed burst requested at
N ticks was programmed at roughly N divided by the sample rate squared
and transmitted immediately. Use the explicit tick-rate constructor,
matching TRXLooper's own conversion of the same case.

Co-Authored-By: Claude <noreply@anthropic.com>
With no requested bufferSize and a low or unset sample rate hint the
0.25-second heuristic produced a pool of 0 to 2 packets, and the FIFO
constructed from capacity()-2 underflowed size_t - throwing
length_error out of Setup for capacity 0, or creating an empty or
permanently-full ring for capacities 1 and 2. Clamp the pool to the
two staging packets plus a usable FIFO on both directions.

Co-Authored-By: Claude <noreply@anthropic.com>
CustomParameterWrite computed log10 of the raw value for anything
below 65.536 with units set: a zero or negative value converted
minus infinity or NaN to int (undefined behavior) and went on air as
garbage, and a legitimate small value produced a negative exponent
whose sign bits overwrote the units nibble when OR'ed into the packed
byte. Guard the logarithm, clamp the exponent to its 4-bit field and
mask it during packing.

Co-Authored-By: Claude <noreply@anthropic.com>
WriteSerialNumber's one-time-programming path reused the packet that
the key-stage reply had overwritten, re-setting only a few fields, so
the header and most payload bytes of the permanent OTP write were
whatever the device echoed. SPI_generic similarly carried response
bytes into the SPI-config preamble of every packet after the first in
a multi-packet transfer. Rebuild the OTP packet from scratch and clear
the preamble on every iteration.

Co-Authored-By: Claude <noreply@anthropic.com>
Three channel and cache addressing errors: LoadConfigLegacyFile read
the channel B Tx NCO values from the file's channel A section, leaving
the actual ch.B section unread; IsSynced issued the channel B raw
register reads while MAC still pointed at channel A, comparing A's
silicon against B's cache; and the MCU-assisted 0x0640/0x0641 write
verified itself through the register cache, where 0x040B is not marked
volatile, instead of reading the chip like the matching read path.

Co-Authored-By: Claude <noreply@anthropic.com>
…ffers

Four embedded-driver fixes: the SX defaults wrote 0xB9FF into the
8-bit VDIV_VCO field, programming the VCO LDO divider to 0xFF instead
of the 0xB9 default; the RSSI settling delay was truncated to 16 bits
at high AGC averaging so the wait wrapped modulo 65536; the CGEN
divider midpoint was masked with 0xFF instead of clamped, spuriously
rejecting deliverable frequencies between roughly 3.8 and 4.8 MHz;
the calibration save/restore scratch buffers were file-scope statics
shared by every chip context, so concurrent calibrations on multi-chip
boards corrupted each other's saved state - they now live per context.
Also mask the upper miso half in the batch flush, whose ~mask promoted
to int and let undefined hook bits reach the address field.

Co-Authored-By: Claude <noreply@anthropic.com>
Three configuration errors: SolveFracDiv could select the prescaler-2
candidate for dividers above 514 whose 8-bit integer part then wrapped
when packed, programming a completely different output frequency;
UploadConfiguration's read-back loop ran one step past the register
map, creating a phantom entry that read and wrote absolute SPI address
0; and SetPrimaryFreq/SetSecondaryFreq recomputed the output
frequencies only when the *other* input was selected, leaving every
derived frequency stale when the active input changed.

Co-Authored-By: Claude <noreply@anthropic.com>
The Si5351C feedback fraction denominator was merged into the register
image with OR over non-zero defaults and over any previous
configuration, and PLLB's feedback divider was computed from PLLA's
input frequency even though the two inputs can differ. The ADF4002 R/N
solver stored counters without validating them against the 14-bit R
and 13-bit N register fields, so an oversized N flipped the charge
pump gain and test bits when packed.

Co-Authored-By: Claude <noreply@anthropic.com>
LMS3SetPath computed the switch bit as 1 shifted by (chan - 4), a
leftover from global channel numbering, but its callers pass the
per-chip channel 0 or 1 - a negative shift with undefined behavior,
and the LMS3 calibration/LNAH switch was never actually configured.

Co-Authored-By: Claude <noreply@anthropic.com>
StreamSetup wrapped the subdevice stream without checking that
StreamCreate succeeded (an unpopulated slot or a rejected config
returns null) and reported success because only the always-non-null
wrapper was tested. StreamRx/StreamTx/StreamStatus and
GetHardwareTimestamp then forwarded to the subdevices, whose own
streamer slots are never set up in the MMX8 flow, dereferencing a null
unique_ptr; StreamStart/Stop/Destroy had the same unguarded access on
the module streamers. Route the data path through the MMX8's own
streamers with null guards, and add the missing null checks to the
base class forwards.

Co-Authored-By: Claude <noreply@anthropic.com>
LMS7002TestSignalConfigure returned from the Rx branch before reaching
the Tx half, so with the Rx test signal enabled the Tx test signal was
neither applied nor cleared - INSEL_TXTSP kept whatever state the chip
was in. Run both halves and combine their status.

Co-Authored-By: Claude <noreply@anthropic.com>
The read loop treated a zero-byte read as progress-free success, but
with VMIN=0/VTIME=10 zero means the 1-second timeout expired, so a
silent GNSS UART hung the test forever. The chunk size was also
sizeof(16) - four bytes - instead of the intended 16, and the open()
of the uart node was never checked.

Co-Authored-By: Claude <noreply@anthropic.com>
SetFIRCoefficients wrote ODD_FIR from the parity of the fixed constant
Filt_N (32), so it was always zero regardless of the real coefficient
count passed in. Configure also wrote the FIR count parity into the
CFR order register, which UpdateHannCoeff already programs from the
CFR order. Use the coefficient count for ODD_FIR and drop the wrong
ODD_CFR write.

Co-Authored-By: Claude <noreply@anthropic.com>
The interface-frequency setup read register 0x002A (and 0x0020 in the
phase-search path) into uninitialized locals while ignoring the read
status. On a failed SPI read the protocol layer leaves MISO untouched,
so bypassTx/bypassRx were decided from indeterminate values and the
garbage 0x0020 backup was later written back to the chip. Initialize
the locals and abort on a failed read before touching any register.

Co-Authored-By: Claude <noreply@anthropic.com>
A clock entry with bypass set and outFrequency 0 (the default) legally
reaches the divider-configuration loops, where ceil(Fvco/0) produced
infinity and the conversion to int is undefined behavior, then wrote a
garbage divider into the output register. Skip bypassed and
zero-frequency clocks in both output loops, leaving their bypass bits
set.

Co-Authored-By: Claude <noreply@anthropic.com>
SetWindowFunction rewrote mWindowCoeffs in place on the caller thread
while the worker thread read the same vector inside FFT_Calculate,
with no synchronization, and avgCount was a plain size_t read and
written across threads. Generate the coefficients into a temporary and
swap under a dedicated mutex, take that mutex around the worker's
FFT_Calculate, and make avgCount atomic.

Co-Authored-By: Claude <noreply@anthropic.com>
…ream

Three LimePlugin robustness fixes: max_channels_to_use came straight
from the user config and indexed the fixed 16-entry SDRConfig::channel
array unchecked, so an oversized value wrote out of bounds;
LimePlugin_Init discarded the TransferSettingsToDevicesConfig status,
reporting success after an antenna-name failure left a partially
applied configuration; and the calibration stream was never stopped or
reset, so LimePlugin_Destroy freed the devices while its TRXLooper
still held raw pointers to them.

Co-Authored-By: Claude <noreply@anthropic.com>
ReceiveStream filled a local StreamMeta with the hardware timestamp on
each staging refill but never stored it into the staging buffer, so
LMS_RecvStream's metadata always reported timestamp 0. Copy the
received timestamp into the staging element.

Co-Authored-By: Claude <noreply@anthropic.com>
All channels of a device share one RFStream, but LMS_DestroyStream
reset it on the first channel's destruction, leaving the other live
handles dereferencing a null stream on Start/Stop/Recv/Send. Only
reset once both staging areas report no channels set up. The channel
was also searched in the saved config with the raw index while setup
had stored it with the align-phase flag masked off, so align-phase
channels were never removed.

Co-Authored-By: Claude <noreply@anthropic.com>
Both functions passed channel 0 to Set/GetParameter, which forces
SetActiveChannel to channel A and rewrites the MAC register, so a
caller working on channel B through MAC-based addressing had its
selection silently switched to A. Read the current MAC and pass the
matching channel through.

Co-Authored-By: Claude <noreply@anthropic.com>
LMS_SetNCOFrequency dereferenced freq[i] and LMS_GetNCOFrequency wrote
freq[i] with no null check, while legacy LimeSuite allowed a null
table to mean set or read only the phase offset - and the sibling NCO
phase functions already guard their array pointer. Honor a null table
on both paths.

Co-Authored-By: Claude <noreply@anthropic.com>
The StreamCreate result went straight into StreamComposite::Add, which
stored the raw pointer without checking for null, so a board that
returns null (unavailable stream port or a rejected config) crashed in
StreamComposite::Setup dereferencing it.

Co-Authored-By: Claude <noreply@anthropic.com>
The transmit loop decremented the remaining count by whatever Transmit
accepted but always advanced to the next file chunk, so a short send
skipped the unsent tail and desynchronized the file position. Resend
the tail of each chunk until it is fully queued. The Rx output file
was also opened without checking success, so a capture to an
unwritable path was silently discarded while the tool still exited 0.

Co-Authored-By: Claude <noreply@anthropic.com>
frontendDelayTest passed the StreamCreate result to MeasureChannelDelays
without a null check, crashing when the board returns null. limePPSDO
tracked the operation status only to skip later steps and then always
returned EXIT_SUCCESS, so a failed register operation reported success
to the shell.

Co-Authored-By: Claude <noreply@anthropic.com>
This was referenced Aug 3, 2026
@rjonaitis
rjonaitis merged commit ff4d119 into myriadrf:develop Aug 3, 2026
19 checks passed
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