Assign descriptive names to SDRangel threads so thread activity can be
identified when investigating multi-threaded behavior, including crashes,
leaks, and synchronization issues in debuggers and others tools.
Use compact names to fit platform thread-name limits (14 chars) while
retaining the component and thread role where possible.
Signed-off-by: Robin Getz <rgetz503@gmail.com>
MessageQueue::pop() transfers ownership of the popped message to the
caller, but many handleInputMessages() implementations only deleted
the message when handleMessage() returned true. When a message was not
handled, ownership had already been transferred from the queue and the
message was leaked.
This was identified while investigating an ASAN leak in the audio
subsystem.
Update the applicable handleInputMessages() implementations to always
delete messages after handleMessage() returns. Unhandled messages are
also logged with the component and message identifier so unexpected
message routing is visible during debugging.
This change:
- Ensures every message popped from a queue has its ownership resolved.
- Prevents leaks when handleMessage() returns false.
- Makes unhandled messages visible instead of silently discarding them.
- Preserves the existing message handling logic and does not require
components to handle message types that are legitimately irrelevant
to them.
The changes are intentionally mechanical and limited to the common
message queue ownership and diagnostic pattern.
Signed-off-by: Robin Getz <rgetz503@gmail.com>
Remove unused loop counters from the SoapySDR input and output GUI loops.
The counters were incremented but never used, triggering
-Wunused-but-set-variable compiler warnings.
The loops are already controlled by their iterators, so removing the counters
does not change behavior.
Signed-off-by: Robin Getz <rgetz503@gmail.com>
Initialize m_packets, m_overflows, and m_timeouts to zero so the USRP input
thread starts with defined stream status counters.
The counters were previously left uninitialized, allowing stream status
reporting to use indeterminate values.
This resolves the uninitialized member findings reported by Coverity
(UNINIT_CTOR).
Signed-off-by: Robin Getz <rgetz503@gmail.com>
Initialize m_rfChanged to zero so the SDRPlay V3 thread starts with a defined
RF change state before RF changes are processed.
The member was previously left uninitialized, allowing RF change state checks
to use an indeterminate value.
This resolves the uninitialized member finding reported by Coverity
(UNINIT_CTOR).
Signed-off-by: Robin Getz <rgetz503@gmail.com>
Drop the current input batch when the remote stream reports an unsupported
sample size instead of continuing with an unpopulated conversion buffer.
The existing warning identifies the invalid stream configuration, while the
early return prevents uninitialized data from being written to the
uncompressed data FIFO.
This resolves the uninitialized value finding reported by Coverity (UNINIT).
Signed-off-by: Robin Getz <rgetz503@gmail.com>
Initialize the RemoteTCP input handler members that were left undefined so
protocol, decompression, and connection state starts with defined values.
Value-initialize the zlib stream and initialize the remaining uninitialized
members according to their intended initial state.
This resolves the uninitialized member findings reported by Coverity
(UNINIT_CTOR).
Signed-off-by: Robin Getz <rgetz503@gmail.com>
Initialize m_sampleRate to zero so LocalInput starts with a defined sample rate
before one is configured.
The member was previously left uninitialized, allowing sample rate access to
use an indeterminate value.
This resolves the uninitialized member finding reported by Coverity
(UNINIT_CTOR).
Signed-off-by: Robin Getz <rgetz503@gmail.com>
Zero-initialize the FCDProPlusThread sample buffer to eliminate the cppcheck
UNINIT_CTOR warning. This ensures the sample buffer starts in a defined state
and avoids undefined behavior from reading uninitialized samples.
Signed-off-by: Robin Getz <rgetz503@gmail.com>
Zero-initialize the FCDProThread sample buffer to eliminate the cppcheck
UNINIT_CTOR warning. This ensures the sample buffer starts in a defined state
and avoids undefined behavior from reading uninitialized samples.
Signed-off-by: Robin Getz <rgetz503@gmail.com>
Initialize the previously uninitialized frequency position and audio sample
buffer members to defined values to eliminate cppcheck UNINIT_CTOR warnings.
This ensures the worker starts with a known state and avoids undefined behavior
from reading uninitialized values.
Signed-off-by: Robin Getz <rgetz503@gmail.com>
Initialize the AGC and bias tee state to false to eliminate cppcheck
UNINIT_CTOR warnings and ensure DeviceGains starts in a defined state.
Signed-off-by: Robin Getz <rgetz503@gmail.com>
The sawtooth and square patterns had a fixed 1000 sample pulse width. Both
now take a period, and the square pattern a duty cycle, from the GUI, a
preset or the web API, with the period shown in seconds beside the sample
count.
A period of zero divides by zero in the sawtooth and wraps the unsigned
sample counter in the square pattern, giving a cycle of four billion
samples, so it is refused at every entrance: a minimum of one on the spin
box, a clamp when an older or hand edited preset is read, a clamp on the
web API, and a clamp in the worker for anything that reaches it another
way. The duty cycle is held to 0 to 100 the same way.
Also fixes the worker's timer. It is now parented to the worker so that it
moves to the worker thread with it, and it runs between startWork and
stopWork rather than from construction. TestSourceInput::stop() runs in
another thread, and Qt will not stop a timer from one, so the stop is
asked of the worker and waited for. The m_running flag it replaces is
gone, along with the test of it in every tick.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A number of settings keys existed in the spec and in the SWG classes but
were missing from the plugin's webapi handlers, so setting them over the
API silently did nothing: dnrScheme on the SSB demodulator, aprsData on
the M17 modulator, pulseShaping on the packet, PSK31 and RTTY modulators,
audioDeviceName on Simple PTT, period, autosave, autoload, autosavePeriod
and filename on SID, and several on Star Tracker, XTRX and the Android SDR
driver input.
Three more were spelled so that they could never match. The channel
analyzer tested for "rationalDownSample " with a trailing space, radio
astronomy tested for "sweep12Start" and "sweep12Stop" rather than
"sweep2Start" and "sweep2Stop", and AFC read "hasTargetFrequency" into
m_transverterTarget rather than "transverterTarget". The channel analyzer
also now accepts the shorter "frequency", "downSample" and
"downSamplerRate" spellings alongside the existing ones.
MapSettings::applySettings ignored rgbColor and mapType, so a patch
carrying them was accepted and then dropped.
Registers MCPServer.yaml in the web API resources so it is served with the
rest of the documentation, and drops a redundant nested guard in the Sky
Map report left by the QString leak fix.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
QMessageBox::information() and friends build the box on the stack, parent
it to the calling widget and run a nested event loop in exec(). If the
parent is closed while the box is open, which the Web API can do at any
time, the parent deletes the box and its children and the heap is
corrupted.
MessageDialog offers the same calls, but the box is heap allocated, given
Qt::WA_DeleteOnClose and shown rather than exec'd, so nothing runs a
nested event loop and the box owns its own lifetime. A box with the same
text as one already up is raised instead of a second being created.
The trade off is that information(), warning() and critical() return
immediately and cannot report which button was pressed. question() takes
a callback for the cases that need the answer.
Converts the call sites in the device, channel and feature GUIs.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The generated SWG setters overwrite the string pointer without deleting
what is already there, and every webapiSettingsGet and webapiReportGet
calls init() first, which has allocated one. Each call therefore leaked a
QString per string field. Measured at 117 bytes per settings GET for the
Satellite Tracker, over 20,000 requests.
Assign in place when the pointer is already set, which is what these same
functions have always done for title and reverseAPIAddress. 394 sites
across webapiFormatChannelSettings, webapiFormatDeviceSettings,
webapiFormatFeatureSettings and the three report equivalents.
The webapiReverseSend* functions are deliberately left alone: they build a
fresh SWG object whose constructor leaves the pointers null, so passing a
new QString is correct there.
After the change the same measurement is flat, at 7 bytes per GET.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
m_running is used to synchronize the PlutoSDR manager and runner
threads. The manager-side startWork() and stopWork() functions control
whether the runner in run() should execute.
startWork() reads m_running before starting the QThread and then waits
for run() to set it true. stopWork() sets m_running false to request
that the runner exit, while run() continuously reads the flag and sets
it false when it terminates.
These accesses occur across the manager and runner threads without
atomic synchronization. Make m_running an atomic_bool in the PlutoSDR
input, output, MIMO input, and MIMO output threads to eliminate the data
race identified by ThreadSanitizer.
This can prevent inconsistent thread state during user-driven device
start, stop, and reload operations, where the GUI can cause the manager
thread to start or stop a PlutoSDR runner while that runner is
processing.
Signed-off-by: Robin Getz <rgetz503@gmail.com>
Update the PlutoSDR input, output, and MIMO GUIs to properly handle the
SDRangel device engine entering the StError state following an
asynchronous device failure.
Clear the affected start/stop controls without emitting their signals,
preventing the GUI from attempting to restart or otherwise operate on a
device that has become unavailable. Indicate the error state visually
and display the error message reported by the device engine.
Stop updating device-specific status information while the device engine
is in the error state, since the underlying PlutoSDR device and streaming
resources may already have been closed.
This keeps the GUI state synchronized with the SDRangel device engine
after asynchronous failures such as a disconnected or unavailable
PlutoSDR.
Signed-off-by: Robin Getz <rgetz503@gmail.com>
Handle asynchronous errors reported by the PlutoSDR streaming threads and
transition the affected SDRangel device to a stopped and closed state.
When a PlutoSDR acquisition or generation thread reports an error, stop
and destroy the affected streaming threads and release the associated
buffers and device resources. For MIMO devices, shut down both acquisition
and generation paths and close the associated RX and TX resources so the
device is left in a consistent state.
Propagate the failure to the SDRangel device engine using the
DSPAcquisitionError and DSPGenerationError messages, including the
underlying error code and a message indicating that the PlutoSDR needs
to be restarted.
Also guard PlutoSDRInput::start() against an unavailable device parameter
object. This can occur after an asynchronous device failure has already
closed the device and prevents a subsequent start operation from
dereferencing an invalid device state.
This ensures that asynchronous failures such as a disconnected PlutoSDR
or an unrecoverable acquisition or generation error do not leave the
SDRangel device engine running against an unavailable device.
Signed-off-by: Robin Getz <rgetz503@gmail.com>
Detect negative return values from rxBufferRefill() and txBufferPush() in
the PlutoSDR input, output, and MIMO worker threads. These calls can fail
asynchronously after the device has successfully started, for example
when the PlutoSDR is disconnected or the underlying I/O connection
encounters an error.
Log the underlying error and emit the error code from the affected worker
thread instead of treating the negative return value as an ordinary
short or incomplete buffer.
This allows asynchronous PlutoSDR acquisition and generation failures to
be propagated out of the worker threads and handled by the device and DSP
error-handling infrastructure, rather than allowing the streaming thread
to continue operating with an invalid device state.
Signed-off-by: Robin Getz <rgetz503@gmail.com>
AddressSanitizer (ASAN) reports a memory leak from the buffer conversion
memory allocated by PlutoSDRInputThread. The memory was not released when
the thread was destroyed, causing the allocation to persist after the
device was closed.
Delete the conversion buffer in the thread destructor so the allocation is
properly released and the leak reported by ASAN is eliminated.
Signed-off-by: Robin Getz <rgetz503@gmail.com>
Coverity reported that an invalid sample size could leave the
conversion buffer uninitialized before it was passed to calcPower()
and the sample FIFO.
Return early when the sample conversion format is unsupported to
prevent processing invalid data.
Signed-off-by: Robin Getz <rgetz503@gmail.com>
Validate the tuner type index before accessing the tuner name table.
The previous check allowed an index equal to names.size(), which could
result in an out-of-range QStringList access.
Use a local integer value for the enum conversion and ensure only valid
indices are used when looking up tuner names.
pointed out by cppcheck: Access out of bounds
Signed-off-by: Robin Getz <rgetz503@gmail.com>
The gain change handler allowed an index equal to m_gains.size(),
which is outside the valid vector range. Reject invalid indices before
accessing the gain table to avoid undefined behavior.
found with cppcheck: Access out of bounds
Signed-off-by: Robin Getz <rgetz503@gmail.com>
Clear the m_open flag when closing PlutoSDR input and output
devices. Previously closeDevice() released m_deviceParams but left
m_open set, leaving the object in an inconsistent state where code
could treat a closed device as still valid.
This was exposed when reloading a running PlutoSDR device, where GUI
updates could access the partially torn-down device state.
Hopefully fixes#2833 (I don't have a windows machine to test).
Signed-off-by: Robin Getz <rgetz503@gmail.com>
Query the PlutoSDR hardware for the current RX gain range and use it
to configure the GUI gain control dynamically.
The AD936x gain limits vary with LO frequency. Previously the GUI used
a fixed gain range, allowing users to select values that the hardware
would reject after changing bands. The gain slider now refreshes its
minimum, maximum, and step size whenever the device center frequency is
updated.
Also change the gain setting serialization to use a signed integer so
negative gain values are preserved for operating modes that support
them.
Changes include:
- Add DevicePlutoSDRBox::getGainRange() to read
in_voltage0_hardwaregain_available.
- Expose gain range through PlutoSDRInput.
- Refresh GUI gain limits when the device frequency changes.
- Avoid unnecessary widget updates when limits are unchanged.
- Store/restore gain as a signed value.
- Add default RX gain limit constants for fallback when the device
cannot provide its range.
Signed-off-by: Robin Getz <rgetz503@gmail.com>
Add defensive null checks before dereferencing DeviceAPI::getBuddySharedPtr()
in the PlutoSDR input and output plugins.
DeviceAPI initializes the buddy shared pointer to nullptr, so a buddy may
exist before its shared state has been attached. This could result in null
pointer dereferences during device initialization, buddy thread
suspend/resume, or settings application.
Changes include:
- Validate buddy shared pointer in openDevice() and fail gracefully if absent.
- Guard suspendBuddies() and resumeBuddies() against null shared pointers.
- Skip buddies without shared state during applySettings() while logging a
warning.
- Prevent dereferencing null buddy shared pointers when restarting buddy
threads.
These changes improve robustness during PlutoSDR buddy initialization and
avoid crashes caused by partially initialized buddy relationships.
Signed-off-by: Robin Getz <rgetz503@gmail.com>
Robin Getz
Edouard Griffiths
Jon Beniston
Alex Antonov UT2UM
Tammo Jan Dijkema
Alejandro Aleman