Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make settings assignments atomic #1329

Open
srcejon opened this issue Jul 8, 2022 · 10 comments
Open

Make settings assignments atomic #1329

srcejon opened this issue Jul 8, 2022 · 10 comments
Assignees

Comments

@srcejon
Copy link
Collaborator

srcejon commented Jul 8, 2022

I think there are a couple of multi-threading problems with webapiSettingsPutPatch in devices and channels.

Using AirspyHF code as an example, but the same applies elsewhere:

    int AirspyHFInput::webapiSettingsPutPatch(
                    bool force,
                    const QStringList& deviceSettingsKeys,
                    SWGSDRangel::SWGDeviceSettings& response, // query + response
                    QString& errorMessage)
    {
        (void) errorMessage;
        AirspyHFSettings settings = m_settings;
        webapiUpdateDeviceSettings(settings, deviceSettingsKeys, response);
    
        MsgConfigureAirspyHF *msg = MsgConfigureAirspyHF::create(settings, force);
        m_inputMessageQueue.push(msg);
    
        if (m_guiMessageQueue) // forward to GUI if any
        {
            MsgConfigureAirspyHF *msgToGUI = MsgConfigureAirspyHF::create(settings, force);
            m_guiMessageQueue->push(msgToGUI);
        }
    
        webapiFormatDeviceSettings(response, settings);
        return 200;
    }

Possible problems include:

  • No mutex is used. This can be problematic for settings = m_settings, as this assignment isn't atomic - so if m_settings is being simultaneously written in applySettings(), settings may contain a mixture of old and new values.
  • If webapiSettingsPutPatch is called quickly twice in succession, handling of the Configure message pushed to the queue may not have been handled by the time of the second call - and thus settings will contain old settings. This can result in some settings being lost, if not all keys are specified in each call. (E.g. if there's one patch to log2Decim followed by another patch to devSampleRate, the setting for log2Decim can be lost).
  • Similarly, if webapiSettingsGet is called quickly after webapiSettingsPutPatch, old data may be returned for the get.
@f4exb
Copy link
Owner

f4exb commented Jul 8, 2022

For the first bullet this is very unlikely to happen and not worth a mutex.

For the middle bullet the solution is to return 204202 that just says the messages have been processed. This is essentially asynchronous and we can't wait for the messages to be processed before returning the reply.

Also for the last bullet the process being essentially asynchronous this is something to cope with.

@srcejon
Copy link
Collaborator Author

srcejon commented Jul 8, 2022

For the first bullet this is very unlikely to happen and not worth a mutex.

There is already a mutex which I think should be suitable, just a case of adding:

QMutexLocker mutexLocker(&m_mutex);

at the top of the function, I think.

For the middle bullet the solution is to return 204 that just says the messages have been processed. This is essentially asynchronous and we can't wait for the messages to be processed before returning the reply.

It's reasonable that processing is async, but how is a client to know when it is safe to send the next patch request?

Also for the last bullet the process being essentially asynchronous this is something to cope with.

How though? Keep calling get until the patched value is returned?

@f4exb
Copy link
Owner

f4exb commented Jul 8, 2022

This is a one way communication and how REST APIs work and one has to live with it.

@srcejon
Copy link
Collaborator Author

srcejon commented Jul 8, 2022

A possible solution after a little experimenting:

  • Add a m_semaphore member to the class
  • Call m_semaphore.acquire() at start of webapiSettingsPutPatch()
  • Add release parameter to the Configure message and set release=true for message in webapiSettingsPutPatch, but not other uses
  • After m_settings = settings in applySettings, if release parameter is true, call m_semaphore.release()

Then for the get:

  • Call m_semaphore.acquire() at start of webapiSettingsGet()
  • Call m_semaphore.release() at the end of webapiSettingsGet()

This seems to ensure both functions can't start until the previous settings from putPatch have been applied. Thoughts?

@f4exb
Copy link
Owner

f4exb commented Jul 8, 2022

Since this is a message queue the requests are processed one after the other so I don't see the need of such a complex setup.

@srcejon
Copy link
Collaborator Author

srcejon commented Jul 8, 2022

Here's an example of where it's currently not working for me:

  • Lets say m_settings starts with log2Decim=2 and devSampleRate=5M
  • A call to webapiSettingsPutPatch() is made with a single key, say log2Decim=1
  • webapiSettingsPutPatch takes a copy of m_settings as settings and set log2Decim=1
  • These settings (log2Decim=1 and devSampleRate=5M) are pushed to the queue
  • Another call to webapiSettingsPutPatch() is made with a single key devSampleRate=6M
  • The thread handling the queue hasn't run / completed yet, so when webapiSettingsPutPatch takes a copy of m_settings as settings, we get the old value for log2Decim
  • So the settings pushed to the queue are log2Decim=2 (the old value) and devSampleRate=6M
  • The message handler runs
  • First call to applySettings sets log2Decim =1 and devSampleRate=5M
  • Second call to applySettings set log2Decim=2 and devSampleRate=6M
  • So the patch to log2Decim is lost (overwritten with old value)

The semaphore ensures that applySettings has been called before m_settings is copied to settings, so we get:

  • m_settings starts with log2Decim=2 and devSampleRate=5M
  • A call to webapiSettingsPutPatch() is made with a single key, say log2Decim=1
  • webapiSettingsPutPatch takes a copy of m_settings as settings and updates log2Decim=1
  • These settings (log2Decim=1 and devSampleRate=5M) are pushed to the queue
  • Another call to webapiSettingsPutPatch() is made with devSampleRate=6M

This is the same as before, so far, but then:

  • The thread handling the queue hasn't run / completed yet, so semaphore.acquire() blocks
  • The message handler runs
  • First call to applySettings sets log2Decim =1 and devSampleRate=5M
  • semaphore is released
  • webapiSettingsPutPatch takes a copy of the new m_settings and updates devSampleRate to 6M
  • The settings pushed to the queue are now log2Decim=1 (the new value) and devSampleRate=6M
  • Second call to applySettings sets log2Decim=1 and devSampleRate=6M
  • So we get the expected settings

@f4exb
Copy link
Owner

f4exb commented Jul 8, 2022

So the issue is only with PATCH since with PUT all fields of the settings are updated anyway so the latest that comes in is right and that's the rule. With PATCH only some fields are supposed to be updated but the update message takes the whole settings as the reference making it a PUT without saying. And it takes the settings as they are at the time the request comes in which may be in the middle of the de-queuing of messages and thus possibly before the last queued request was processed. The GUI is also part of the game since it also has its own copy of the settings.

The semaphore solution is bad because it makes the message queue useless and blocks the requests processing. One should rather look at treating requests as transactions with only one valid copy (or properly synchronized copies) of the settings similarly to database transactions.

This is a big redesign and I am not sure it is actually worth it since you have to hit the API really hard or be in a massively parallel environment either multi process or multi user to be in such a situation. In most cases the previous message will have been processed before the next request comes in. Have you already encountered this situation practically?

Possibly the settings should be conveyed with a set of keys like the API keys and could be reused as well implementing the applySettings like:

	if ((settingsKeys.contain("log2Decim") || force)
	{
		forwardChange = true;

		if (m_airspyHFWorker)
		{
		    m_airspyHFWorker->setLog2Decimation(settings.m_log2Decim);
			qDebug() << "AirspyInput: set decimation to " << (1<<settings.m_log2Decim);
		}

                m_settings.m_log2Decim = settings.m_log2Decim;
	}

The settingsKeys would be reused in place of the reverseAPIKeys and the m_settings = settings at the end removed.
The MsgConfigureAirspyHF is modified like this:

	class MsgConfigureAirspyHF : public Message {
		MESSAGE_CLASS_DECLARATION

	public:
		const AirspyHFSettings& getSettings() const { return m_settings; }
                const QStringList& getSettingsKeys() const { return m_settingsKeys; }
		bool getForce() const { return m_force; }

		static MsgConfigureAirspyHF* create(const AirspyHFSettings& settings, const QStringList& settingsKeys, bool force) {
			return new MsgConfigureAirspyHF(settings, settingsKeys, force);
		}

	private:
		AirspyHFSettings m_settings;
                QStringList m_settingsKeys;
		bool m_force;

		MsgConfigureAirspyHF(const AirspyHFSettings& settings, const QStringList& settingsKeys, bool force) :
			Message(),
			m_settings(settings),
                        m_settingsKeys(settingsKeys),
			m_force(force)
		{ }
	};

The webapiSettingsPutPatch is then:

int AirspyHFInput::webapiSettingsPutPatch(
                bool force,
                const QStringList& deviceSettingsKeys,
                SWGSDRangel::SWGDeviceSettings& response, // query + response
                QString& errorMessage)
{
    (void) errorMessage;
    AirspyHFSettings settings;
    webapiUpdateDeviceSettings(settings, deviceSettingsKeys, response);

    MsgConfigureAirspyHF *msg = MsgConfigureAirspyHF::create(settings, deviceSettingsKeys, force);
    m_inputMessageQueue.push(msg);

    if (m_guiMessageQueue) // forward to GUI if any
    {
        MsgConfigureAirspyHF *msgToGUI = MsgConfigureAirspyHF::create(settings, deviceSettingsKeys, force);
        m_guiMessageQueue->push(msgToGUI);
    }

    webapiFormatDeviceSettings(response, settings);
    return 200;
}

Also the 200 will be replaced by 202 with standard response or 204 without response.

The GUI is also updated:

void AirspyHFGui::on_decim_currentIndexChanged(int index)
{
	if ((index < 0) || (index > 8))
		return;
	m_settings.m_log2Decim = index;
        m_settingsKeys.append("log2Decim");
	sendSettings();
}
void AirspyHFGui::updateHardware()
{
	qDebug() << "AirspyHFGui::updateHardware";
	AirspyHFInput::MsgConfigureAirspyHF* message = AirspyHFInput::MsgConfigureAirspyHF::create(m_settings, m_settingsKeys, m_forceSettings);
	m_sampleSource->getInputMessageQueue()->push(message);
	m_forceSettings = false;
        m_settingsKeys.clear();
	m_updateTimer.stop();
}
bool AirspyHFGui::handleMessage(const Message& message)
{
    if (AirspyHFInput::MsgConfigureAirspyHF::match(message))
    {
        const AirspyHFInput::MsgConfigureAirspyHF& cfg = (AirspyHFInput::MsgConfigureAirspyHF&) message;
        const AirspyHFSettings& settings = cfg.getSettings();
        const  QStringList& settingsKeys = cfg.getSettingsKeys();
 
        if (settingsKeys.contains("log2Decim") {
            m_settings.m_log2Decim = settings.m_log2Decim;
        }
        ...   
        blockApplySettings(true);
        displaySettings();
        blockApplySettings(false);
        return true;
    }
...
    else
    {
        return false;
    }
}

The

        if (settingsKeys.contains("log2Decim") {
            m_settings.m_log2Decim = settings.m_log2Decim;
        }
...

block can be implemented in AirspyHFSettings and reused in many places.

@srcejon
Copy link
Collaborator Author

srcejon commented Jul 9, 2022

Yes. It's a real problem. I found it while debugging why some patch calls weren't working.

Note that depending on the SDR used, the problem can appear even without hitting the API paticulary hard, as applySettings can take a relatively long time (Eg SDRplayv3's applySettings can be slow as it waits for settings to be applied be2dbab ).

@f4exb f4exb changed the title webapiSettingsPutPatch multi-threading problem Make settings assignments atomic Jul 23, 2022
@f4exb f4exb self-assigned this Jul 23, 2022
@f4exb
Copy link
Owner

f4exb commented Oct 22, 2022

Full diff for Airspy airspygui.cpp
diff --git a/plugins/samplesource/airspy/airspygui.cpp b/plugins/samplesource/airspy/airspygui.cpp
index d45c0b8f6..5e3e983b7 100644
--- a/plugins/samplesource/airspy/airspygui.cpp
+++ b/plugins/samplesource/airspy/airspygui.cpp
@@ -87,6 +87,7 @@ void AirspyGui::resetToDefaults()
 {
 	m_settings.resetToDefaults();
 	displaySettings();
+    m_forceSettings = true;
 	sendSettings();
 }
 
@@ -119,7 +120,13 @@ bool AirspyGui::handleMessage(const Message& message)
     if (AirspyInput::MsgConfigureAirspy::match(message))
     {
         const AirspyInput::MsgConfigureAirspy& cfg = (AirspyInput::MsgConfigureAirspy&) message;
-        m_settings = cfg.getSettings();
+
+        if (cfg.getForce()) {
+            m_settings = cfg.getSettings();
+        } else {
+            m_settings.applySettings(cfg.getSettingsKeys(), cfg.getSettings());
+        }
+
         blockApplySettings(true);
         displaySettings();
         blockApplySettings(false);
@@ -267,6 +274,7 @@ void AirspyGui::sendSettings()
 void AirspyGui::on_centerFrequency_changed(quint64 value)
 {
 	m_settings.m_centerFrequency = value * 1000;
+    m_settingsKeys.append("centerFrequency");
 	sendSettings();
 }
 
@@ -274,42 +282,49 @@ void AirspyGui::on_LOppm_valueChanged(int value)
 {
 	m_settings.m_LOppmTenths = value;
 	ui->LOppmText->setText(QString("%1").arg(QString::number(m_settings.m_LOppmTenths/10.0, 'f', 1)));
+    m_settingsKeys.append("LOppmTenths");
 	sendSettings();
 }
 
 void AirspyGui::on_dcOffset_toggled(bool checked)
 {
 	m_settings.m_dcBlock = checked;
+    m_settingsKeys.append("dcBlock");
 	sendSettings();
 }
 
 void AirspyGui::on_iqImbalance_toggled(bool checked)
 {
 	m_settings.m_iqCorrection = checked;
+    m_settingsKeys.append("iqCorrection");
 	sendSettings();
 }
 
 void AirspyGui::on_sampleRate_currentIndexChanged(int index)
 {
 	m_settings.m_devSampleRateIndex = index;
+    m_settingsKeys.append("devSampleRateIndex");
 	sendSettings();
 }
 
 void AirspyGui::on_biasT_stateChanged(int state)
 {
 	m_settings.m_biasT = (state == Qt::Checked);
+    m_settingsKeys.append("biasT");
 	sendSettings();
 }
 
 void AirspyGui::on_lnaAGC_stateChanged(int state)
 {
 	m_settings.m_lnaAGC = (state == Qt::Checked);
+    m_settingsKeys.append("lnaAGC");
 	sendSettings();
 }
 
 void AirspyGui::on_mixAGC_stateChanged(int state)
 {
 	m_settings.m_mixerAGC = (state == Qt::Checked);
+    m_settingsKeys.append("mixerAGC");
 	sendSettings();
 }
 
@@ -318,6 +333,7 @@ void AirspyGui::on_decim_currentIndexChanged(int index)
 	if ((index <0) || (index > 6))
 		return;
 	m_settings.m_log2Decim = index;
+    m_settingsKeys.append("log2Decim");
 	sendSettings();
 }
 
@@ -325,12 +341,15 @@ void AirspyGui::on_fcPos_currentIndexChanged(int index)
 {
 	if (index == 0) {
 		m_settings.m_fcPos = AirspySettings::FC_POS_INFRA;
+        m_settingsKeys.append("fcPos");
 		sendSettings();
 	} else if (index == 1) {
 		m_settings.m_fcPos = AirspySettings::FC_POS_SUPRA;
+        m_settingsKeys.append("fcPos");
 		sendSettings();
 	} else if (index == 2) {
 		m_settings.m_fcPos = AirspySettings::FC_POS_CENTER;
+        m_settingsKeys.append("fcPos");
 		sendSettings();
 	}
 }
@@ -342,6 +361,7 @@ void AirspyGui::on_lna_valueChanged(int value)
 
 	ui->lnaGainText->setText(tr("%1dB").arg(value));
 	m_settings.m_lnaGain = value;
+    m_settingsKeys.append("lnaGain");
 	sendSettings();
 }
 
@@ -352,6 +372,7 @@ void AirspyGui::on_mix_valueChanged(int value)
 
 	ui->mixText->setText(tr("%1dB").arg(value));
 	m_settings.m_mixerGain = value;
+    m_settingsKeys.append("mixerGain");
 	sendSettings();
 }
 
@@ -362,6 +383,7 @@ void AirspyGui::on_vga_valueChanged(int value)
 
 	ui->vgaText->setText(tr("%1dB").arg(value));
 	m_settings.m_vgaGain = value;
+    m_settingsKeys.append("vgaGain");
 	sendSettings();
 }
 
@@ -382,14 +404,19 @@ void AirspyGui::on_transverter_clicked()
     qDebug("AirspyGui::on_transverter_clicked: %lld Hz %s", m_settings.m_transverterDeltaFrequency, m_settings.m_transverterMode ? "on" : "off");
     updateFrequencyLimits();
     m_settings.m_centerFrequency = ui->centerFrequency->getValueNew()*1000;
+    m_settingsKeys.append("transverterMode");
+    m_settingsKeys.append("m_transverterDeltaFrequency");
+    m_settingsKeys.append("m_iqOrder");
+    m_settingsKeys.append("centerFrequency");
     sendSettings();
 }
 
 void AirspyGui::updateHardware()
 {
 	qDebug() << "AirspyGui::updateHardware";
-	AirspyInput::MsgConfigureAirspy* message = AirspyInput::MsgConfigureAirspy::create(m_settings, m_forceSettings);
+	AirspyInput::MsgConfigureAirspy* message = AirspyInput::MsgConfigureAirspy::create(m_settings, m_settingsKeys, m_forceSettings);
 	m_sampleSource->getInputMessageQueue()->push(message);
+    m_settingsKeys.clear();
 	m_forceSettings = false;
 	m_updateTimer.stop();
 }
@@ -466,6 +493,10 @@ void AirspyGui::openDeviceSettingsDialog(const QPoint& p)
         m_settings.m_reverseAPIPort = dialog.getReverseAPIPort();
         m_settings.m_reverseAPIDeviceIndex = dialog.getReverseAPIDeviceIndex();
 
+        m_settingsKeys.append("useReverseAPI");
+        m_settingsKeys.append("reverseAPIAddress");
+        m_settingsKeys.append("reverseAPIPort");
+        m_settingsKeys.append("reverseAPIDeviceIndex");
         sendSettings();
     }

airspygui.h

diff --git a/plugins/samplesource/airspy/airspygui.h b/plugins/samplesource/airspy/airspygui.h
index 2f7de1f48..249829252 100644
--- a/plugins/samplesource/airspy/airspygui.h
+++ b/plugins/samplesource/airspy/airspygui.h
@@ -57,6 +57,7 @@ private:
 	bool m_doApplySettings;
 	bool m_forceSettings;
 	AirspySettings m_settings;
+    QList<QString> m_settingsKeys;
 	QTimer m_updateTimer;
 	QTimer m_statusTimer;
 	std::vector<uint32_t> m_rates;

airspyinput.cpp

diff --git a/plugins/samplesource/airspy/airspyinput.cpp b/plugins/samplesource/airspy/airspyinput.cpp
index 3c234459e..cdccd978c 100644
--- a/plugins/samplesource/airspy/airspyinput.cpp
+++ b/plugins/samplesource/airspy/airspyinput.cpp
@@ -170,7 +170,7 @@ bool AirspyInput::openDevice()
 
 void AirspyInput::init()
 {
-    applySettings(m_settings, true);
+    applySettings(m_settings, QList<QString>(), true);
 }
 
 bool AirspyInput::start()
@@ -202,7 +202,7 @@ bool AirspyInput::start()
     m_airspyWorkerThread->start();
 
     qDebug("AirspyInput::startInput: started");
-    applySettings(m_settings, true);
+    applySettings(m_settings, QList<QString>(), true);
     m_running = true;
 
 	return true;
@@ -257,12 +257,12 @@ bool AirspyInput::deserialize(const QByteArray& data)
         success = false;
     }
 
-    MsgConfigureAirspy* message = MsgConfigureAirspy::create(m_settings, true);
+    MsgConfigureAirspy* message = MsgConfigureAirspy::create(m_settings, QList<QString>(), true);
     m_inputMessageQueue.push(message);
 
     if (m_guiMessageQueue)
     {
-        MsgConfigureAirspy* messageToGUI = MsgConfigureAirspy::create(m_settings, true);
+        MsgConfigureAirspy* messageToGUI = MsgConfigureAirspy::create(m_settings, QList<QString>(), true);
         m_guiMessageQueue->push(messageToGUI);
     }
 
@@ -290,12 +290,12 @@ void AirspyInput::setCenterFrequency(qint64 centerFrequency)
     AirspySettings settings = m_settings;
     settings.m_centerFrequency = centerFrequency;
 
-    MsgConfigureAirspy* message = MsgConfigureAirspy::create(settings, false);
+    MsgConfigureAirspy* message = MsgConfigureAirspy::create(settings, QList<QString>{"centerFrequency"}, false);
     m_inputMessageQueue.push(message);
 
     if (m_guiMessageQueue)
     {
-        MsgConfigureAirspy* messageToGUI = MsgConfigureAirspy::create(settings, false);
+        MsgConfigureAirspy* messageToGUI = MsgConfigureAirspy::create(settings, QList<QString>{"centerFrequency"}, false);
         m_guiMessageQueue->push(messageToGUI);
     }
 }
@@ -307,10 +307,9 @@ bool AirspyInput::handleMessage(const Message& message)
 		MsgConfigureAirspy& conf = (MsgConfigureAirspy&) message;
 		qDebug() << "AirspyInput::handleMessage: MsgConfigureAirspy";
 
-		bool success = applySettings(conf.getSettings(), conf.getForce());
+		bool success = applySettings(conf.getSettings(), conf.getSettingsKeys(), conf.getForce());
 
-		if (!success)
-		{
+		if (!success) {
 			qDebug("AirspyInput::handleMessage: Airspy config error");
 		}
 
@@ -323,8 +322,7 @@ bool AirspyInput::handleMessage(const Message& message)
 
         if (cmd.getStartStop())
         {
-            if (m_deviceAPI->initDeviceEngine())
-            {
+            if (m_deviceAPI->initDeviceEngine()) {
                 m_deviceAPI->startDeviceEngine();
             }
         }
@@ -352,42 +350,32 @@ void AirspyInput::setDeviceCenterFrequency(quint64 freq_hz)
 
 	airspy_error rc = (airspy_error) airspy_set_freq(m_dev, static_cast<uint32_t>(freq_hz));
 
-	if (rc != AIRSPY_SUCCESS)
-	{
+	if (rc != AIRSPY_SUCCESS) {
 		qWarning("AirspyInput::setDeviceCenterFrequency: could not frequency to %llu Hz", freq_hz);
-	}
-	else
-	{
+	} else {
 		qDebug("AirspyInput::setDeviceCenterFrequency: frequency set to %llu Hz", freq_hz);
 	}
 }
 
-bool AirspyInput::applySettings(const AirspySettings& settings, bool force)
+bool AirspyInput::applySettings(const AirspySettings& settings, const QList<QString>& settingsKeys, bool force)
 {
 	QMutexLocker mutexLocker(&m_mutex);
 
 	bool forwardChange = false;
 	airspy_error rc = AIRSPY_ERROR_OTHER;
-    QList<QString> reverseAPIKeys;
-
-	qDebug() << "AirspyInput::applySettings";
 
-    if ((m_settings.m_dcBlock != settings.m_dcBlock) || force) {
-        reverseAPIKeys.append("dcBlock");
-    }
-    if ((m_settings.m_iqCorrection != settings.m_iqCorrection) || force) {
-        reverseAPIKeys.append("iqCorrection");
-    }
+	qDebug() << "AirspyInput::applySettings:"
+        << "force:" << force
+        << settings.getDebugString(settingsKeys, force);
 
-    if ((m_settings.m_dcBlock != settings.m_dcBlock) ||
-        (m_settings.m_iqCorrection != settings.m_iqCorrection) || force)
+    if ((settingsKeys.contains("dcBlock")) ||
+        (settingsKeys.contains("iqCorrection")) || force)
     {
 		m_deviceAPI->configureCorrections(settings.m_dcBlock, settings.m_iqCorrection);
 	}
 
-	if ((m_settings.m_devSampleRateIndex != settings.m_devSampleRateIndex) || force)
+	if ((settingsKeys.contains("devSampleRateIndex")) || force)
 	{
-        reverseAPIKeys.append("devSampleRateIndex");
 		forwardChange = true;
 
 		if (m_dev != 0)
@@ -406,9 +394,8 @@ bool AirspyInput::applySettings(const AirspySettings& settings, bool force)
 		}
 	}
 
-	if ((m_settings.m_log2Decim != settings.m_log2Decim) || force)
+	if ((settingsKeys.contains("log2Decim")) || force)
 	{
-        reverseAPIKeys.append("log2Decim");
 		forwardChange = true;
 
 		if (m_airspyWorker)
@@ -418,37 +405,19 @@ bool AirspyInput::applySettings(const AirspySettings& settings, bool force)
 		}
 	}
 
-	if ((m_settings.m_iqOrder != settings.m_iqOrder) || force)
+	if ((settingsKeys.contains("iqOrder")) || force)
 	{
-        reverseAPIKeys.append("iqOrder");
-
 		if (m_airspyWorker) {
 			m_airspyWorker->setIQOrder(settings.m_iqOrder);
 		}
 	}
 
-    if ((m_settings.m_centerFrequency != settings.m_centerFrequency) || force) {
-        reverseAPIKeys.append("centerFrequency");
-    }
-    if ((m_settings.m_LOppmTenths != settings.m_LOppmTenths) || force) {
-        reverseAPIKeys.append("LOppmTenths");
-    }
-    if ((m_settings.m_fcPos != settings.m_fcPos) || force) {
-        reverseAPIKeys.append("fcPos");
-    }
-    if ((m_settings.m_transverterMode != settings.m_transverterMode) || force) {
-        reverseAPIKeys.append("transverterMode");
-    }
-    if ((m_settings.m_transverterDeltaFrequency != settings.m_transverterDeltaFrequency) || force) {
-        reverseAPIKeys.append("transverterDeltaFrequency");
-    }
-
-	if ((m_settings.m_centerFrequency != settings.m_centerFrequency)
-        || (m_settings.m_LOppmTenths != settings.m_LOppmTenths)
-        || (m_settings.m_fcPos != settings.m_fcPos)
-        || (m_settings.m_log2Decim != settings.m_log2Decim)
-        || (m_settings.m_transverterMode != settings.m_transverterMode)
-        || (m_settings.m_transverterDeltaFrequency != settings.m_transverterDeltaFrequency) || force)
+	if ((settingsKeys.contains("centerFrequency"))
+        || (settingsKeys.contains("transverterDeltaFrequency"))
+        || (settingsKeys.contains("log2Decim"))
+        || (settingsKeys.contains("fcPos"))
+        || (settingsKeys.contains("devSampleRateIndex"))
+        || (settingsKeys.contains("transverterMode")) || force)
 	{
         qint64 deviceCenterFrequency = DeviceSampleSource::calculateDeviceCenterFrequency(
                 settings.m_centerFrequency,
@@ -466,7 +435,7 @@ bool AirspyInput::applySettings(const AirspySettings& settings, bool force)
 		forwardChange = true;
 	}
 
-	if ((m_settings.m_fcPos != settings.m_fcPos) || force)
+	if ((settingsKeys.contains("fcPos")) || force)
 	{
 		if (m_airspyWorker)
 		{
@@ -475,10 +444,8 @@ bool AirspyInput::applySettings(const AirspySettings& settings, bool force)
 		}
 	}
 
-	if ((m_settings.m_lnaGain != settings.m_lnaGain) || force)
+	if (settingsKeys.contains("lnaGain") || force)
 	{
-        reverseAPIKeys.append("lnaGain");
-
 		if (m_dev != 0)
 		{
 			rc = (airspy_error) airspy_set_lna_gain(m_dev, settings.m_lnaGain);
@@ -491,10 +458,8 @@ bool AirspyInput::applySettings(const AirspySettings& settings, bool force)
 		}
 	}
 
-	if ((m_settings.m_lnaAGC != settings.m_lnaAGC) || force)
+	if (settingsKeys.contains("lnaAGC") || force)
 	{
-        reverseAPIKeys.append("lnaAGC");
-
 		if (m_dev != 0) {
 			rc = (airspy_error) airspy_set_lna_agc(m_dev, (settings.m_lnaAGC ? 1 : 0));
 		}
@@ -506,10 +471,8 @@ bool AirspyInput::applySettings(const AirspySettings& settings, bool force)
 		}
 	}
 
-	if ((m_settings.m_mixerGain != settings.m_mixerGain) || force)
+	if (settingsKeys.contains("mixerGain") || force)
 	{
-        reverseAPIKeys.append("mixerGain");
-
 		if (m_dev != 0)
 		{
 			rc = (airspy_error) airspy_set_mixer_gain(m_dev, settings.m_mixerGain);
@@ -522,10 +485,8 @@ bool AirspyInput::applySettings(const AirspySettings& settings, bool force)
 		}
 	}
 
-	if ((m_settings.m_mixerAGC != settings.m_mixerAGC) || force)
+	if (settingsKeys.contains("mixerAGC") || force)
 	{
-        reverseAPIKeys.append("mixerAGC");
-
 		if (m_dev != 0) {
 			rc = (airspy_error) airspy_set_mixer_agc(m_dev, (settings.m_mixerAGC ? 1 : 0));
 		}
@@ -537,10 +498,8 @@ bool AirspyInput::applySettings(const AirspySettings& settings, bool force)
 		}
 	}
 
-	if ((m_settings.m_vgaGain != settings.m_vgaGain) || force)
+	if (settingsKeys.contains("vgaGain") || force)
 	{
-        reverseAPIKeys.append("vgaGain");
-
 		if (m_dev != 0)
 		{
 			rc = (airspy_error) airspy_set_vga_gain(m_dev, settings.m_vgaGain);
@@ -553,10 +512,8 @@ bool AirspyInput::applySettings(const AirspySettings& settings, bool force)
 		}
 	}
 
-	if ((m_settings.m_biasT != settings.m_biasT) || force)
+	if (settingsKeys.contains("biasT") || force)
 	{
-        reverseAPIKeys.append("biasT");
-
 		if (m_dev != 0)
 		{
 			rc = (airspy_error) airspy_set_rf_bias(m_dev, (settings.m_biasT ? 1 : 0));
@@ -575,10 +532,14 @@ bool AirspyInput::applySettings(const AirspySettings& settings, bool force)
                }
        }
 
-    if (settings.m_useReverseAPI)
+    if (settingsKeys.contains("useReverseAPI"))
     {
-        bool fullUpdate = ((m_settings.m_useReverseAPI != settings.m_useReverseAPI) && settings.m_useReverseAPI) ||
-                (m_settings.m_reverseAPIAddress != settings.m_reverseAPIAddress) ||
-                (m_settings.m_reverseAPIPort != settings.m_reverseAPIPort) ||
-                (m_settings.m_reverseAPIDeviceIndex != settings.m_reverseAPIDeviceIndex);
-        webapiReverseSendSettings(reverseAPIKeys, settings, fullUpdate || force);
+        bool fullUpdate = (settingsKeys.contains("useReverseAPI") && settings.m_useReverseAPI) ||
+            settingsKeys.contains("reverseAPIAddress") ||
+            settingsKeys.contains("reverseAPIPort") ||
+            settingsKeys.contains("reverseAPIDeviceIndex");
+        webapiReverseSendSettings(settingsKeys, settings, fullUpdate || force);
     }
 
-    m_settings = settings;
+    if (force) {
+        m_settings = settings;
+    } else {
+        m_settings.applySettings(settingsKeys, settings);
+    }
 
 	if (forwardChange)
 	{
@@ -665,12 +626,12 @@ int AirspyInput::webapiSettingsPutPatch(
     AirspySettings settings = m_settings;
     webapiUpdateDeviceSettings(settings, deviceSettingsKeys, response);
 
-    MsgConfigureAirspy *msg = MsgConfigureAirspy::create(settings, force);
+    MsgConfigureAirspy *msg = MsgConfigureAirspy::create(settings, deviceSettingsKeys, force);
     m_inputMessageQueue.push(msg);
 
     if (m_guiMessageQueue) // forward to GUI if any
     {
-        MsgConfigureAirspy *msgToGUI = MsgConfigureAirspy::create(settings, force);
+        MsgConfigureAirspy *msgToGUI = MsgConfigureAirspy::create(settings, deviceSettingsKeys, force);
         m_guiMessageQueue->push(msgToGUI);
     }
 
@@ -800,7 +761,7 @@ void AirspyInput::webapiFormatDeviceReport(SWGSDRangel::SWGDeviceReport& respons
     }
 }
 
-void AirspyInput::webapiReverseSendSettings(QList<QString>& deviceSettingsKeys, const AirspySettings& settings, bool force)
+void AirspyInput::webapiReverseSendSettings(const QList<QString>& deviceSettingsKeys, const AirspySettings& settings, bool force)
 {
     SWGSDRangel::SWGDeviceSettings *swgDeviceSettings = new SWGSDRangel::SWGDeviceSettings();
     swgDeviceSettings->setDirection(0); // single Rx

airspyinput.h

diff --git a/plugins/samplesource/airspy/airspyinput.h b/plugins/samplesource/airspy/airspyinput.h
index e415a969c..c8e1c67fd 100644
--- a/plugins/samplesource/airspy/airspyinput.h
+++ b/plugins/samplesource/airspy/airspyinput.h
@@ -40,20 +40,23 @@ public:
 
 	public:
 		const AirspySettings& getSettings() const { return m_settings; }
+        const QList<QString>& getSettingsKeys() const { return m_settingsKeys; }
 		bool getForce() const { return m_force; }
 
-		static MsgConfigureAirspy* create(const AirspySettings& settings, bool force)
+		static MsgConfigureAirspy* create(const AirspySettings& settings, const QList<QString>& settingsKeys, bool force)
 		{
-			return new MsgConfigureAirspy(settings, force);
+			return new MsgConfigureAirspy(settings, settingsKeys, force);
 		}
 
 	private:
 		AirspySettings m_settings;
+        QList<QString> m_settingsKeys;
 		bool m_force;
 
-		MsgConfigureAirspy(const AirspySettings& settings, bool force) :
+		MsgConfigureAirspy(const AirspySettings& settings, const QList<QString>& settingsKeys, bool force) :
 			Message(),
 			m_settings(settings),
+            m_settingsKeys(settingsKeys),
 			m_force(force)
 		{ }
 	};
@@ -148,11 +151,11 @@ private:
 
 	bool openDevice();
 	void closeDevice();
-	bool applySettings(const AirspySettings& settings, bool force);
+	bool applySettings(const AirspySettings& settings, const QList<QString>& settingsKeys, bool force);
 	struct airspy_device *open_airspy_from_sequence(int sequence);
 	void setDeviceCenterFrequency(quint64 freq);
     void webapiFormatDeviceReport(SWGSDRangel::SWGDeviceReport& response);
-    void webapiReverseSendSettings(QList<QString>& deviceSettingsKeys, const AirspySettings& settings, bool force);
+    void webapiReverseSendSettings(const QList<QString>& deviceSettingsKeys, const AirspySettings& settings, bool force);
     void webapiReverseSendStartStop(bool start);
 
 private slots:

airspysettings.cpp

diff --git a/plugins/samplesource/airspy/airspysettings.cpp b/plugins/samplesource/airspy/airspysettings.cpp
index 5be260a5a..a3fc1fb8f 100644
--- a/plugins/samplesource/airspy/airspysettings.cpp
+++ b/plugins/samplesource/airspy/airspysettings.cpp
@@ -127,3 +127,135 @@ bool AirspySettings::deserialize(const QByteArray& data)
 		return false;
 	}
 }
+
+void AirspySettings::applySettings(const QStringList& settingsKeys, const AirspySettings& settings)
+{
+    if (settingsKeys.contains("centerFrequency")) {
+        m_centerFrequency = settings.m_centerFrequency;
+    }
+    if (settingsKeys.contains("LOppmTenths")) {
+        m_LOppmTenths = settings.m_LOppmTenths;
+    }
+    if (settingsKeys.contains("devSampleRateIndex")) {
+        m_devSampleRateIndex = settings.m_devSampleRateIndex;
+    }
+    if (settingsKeys.contains("log2Decim")) {
+        m_log2Decim = settings.m_log2Decim;
+    }
+    if (settingsKeys.contains("fcPos")) {
+        m_fcPos = settings.m_fcPos;
+    }
+    if (settingsKeys.contains("lnaGain")) {
+        m_lnaGain = settings.m_lnaGain;
+    }
+    if (settingsKeys.contains("mixerGain")) {
+        m_mixerGain = settings.m_mixerGain;
+    }
+    if (settingsKeys.contains("vgaGain")) {
+        m_vgaGain = settings.m_vgaGain;
+    }
+    if (settingsKeys.contains("biasT")) {
+        m_biasT = settings.m_biasT;
+    }
+    if (settingsKeys.contains("dcBlock")) {
+        m_dcBlock = settings.m_dcBlock;
+    }
+    if (settingsKeys.contains("iqCorrection")) {
+        m_iqCorrection = settings.m_iqCorrection;
+    }
+    if (settingsKeys.contains("lnaAGC")) {
+        m_lnaAGC = settings.m_lnaAGC;
+    }
+    if (settingsKeys.contains("mixerAGC")) {
+        m_mixerAGC = settings.m_mixerAGC;
+    }
+    if (settingsKeys.contains("transverterMode")) {
+        m_transverterMode = settings.m_transverterMode;
+    }
+    if (settingsKeys.contains("transverterDeltaFrequency")) {
+        m_transverterDeltaFrequency = settings.m_transverterDeltaFrequency;
+    }
+    if (settingsKeys.contains("useReverseAPI")) {
+        m_useReverseAPI = settings.m_useReverseAPI;
+    }
+    if (settingsKeys.contains("reverseAPIAddress")) {
+        m_reverseAPIAddress = settings.m_reverseAPIAddress;
+    }
+    if (settingsKeys.contains("reverseAPIPort")) {
+        m_reverseAPIPort = settings.m_reverseAPIPort;
+    }
+    if (settingsKeys.contains("reverseAPIDeviceIndex")) {
+        m_reverseAPIDeviceIndex = settings.m_reverseAPIDeviceIndex;
+    }
+    if (settingsKeys.contains("iqOrder")) {
+        m_iqOrder = settings.m_iqOrder;
+    }
+}
+
+QString AirspySettings::getDebugString(const QStringList& settingsKeys, bool force) const
+{
+    std::ostringstream ostr;
+
+    if (settingsKeys.contains("centerFrequency") || force) {
+        ostr << " m_centerFrequency: " << m_centerFrequency;
+    }
+    if (settingsKeys.contains("LOppmTenths") || force) {
+        ostr << " m_LOppmTenths: " << m_LOppmTenths;
+    }
+    if (settingsKeys.contains("devSampleRateIndex") || force) {
+        ostr << " m_devSampleRateIndex: " << m_devSampleRateIndex;
+    }
+    if (settingsKeys.contains("log2Decim") || force) {
+        ostr << " m_log2Decim: " << m_log2Decim;
+    }
+    if (settingsKeys.contains("fcPos") || force) {
+        ostr << " m_fcPos: " << m_fcPos;
+    }
+    if (settingsKeys.contains("lnaGain") || force) {
+        ostr << " m_lnaGain: " << m_lnaGain;
+    }
+    if (settingsKeys.contains("mixerGain") || force) {
+        ostr << " m_mixerGain: " << m_mixerGain;
+    }
+    if (settingsKeys.contains("vgaGain") || force) {
+        ostr << " m_vgaGain: " << m_vgaGain;
+    }
+    if (settingsKeys.contains("biasT") || force) {
+        ostr << " m_biasT: " << m_biasT;
+    }
+    if (settingsKeys.contains("dcBlock") || force) {
+        ostr << " m_dcBlock: " << m_dcBlock;
+    }
+    if (settingsKeys.contains("iqCorrection") || force) {
+        ostr << " m_iqCorrection: " << m_iqCorrection;
+    }
+    if (settingsKeys.contains("lnaAGC") || force) {
+        ostr << " m_lnaAGC: " << m_lnaAGC;
+    }
+    if (settingsKeys.contains("mixerAGC") || force) {
+        ostr << " m_mixerAGC: " << m_mixerAGC;
+    }
+    if (settingsKeys.contains("transverterMode") || force) {
+        ostr << " m_transverterMode: " << m_transverterMode;
+    }
+    if (settingsKeys.contains("transverterDeltaFrequency") || force) {
+        ostr << " m_transverterDeltaFrequency: " << m_transverterDeltaFrequency;
+    }
+    if (settingsKeys.contains("useReverseAPI") || force) {
+        ostr << " m_useReverseAPI: " << m_useReverseAPI;
+    }
+    if (settingsKeys.contains("reverseAPIAddress") || force) {
+        ostr << " m_reverseAPIAddress: " << m_reverseAPIAddress.toStdString();
+    }
+    if (settingsKeys.contains("reverseAPIPort") || force) {
+        ostr << " m_reverseAPIPort: " << m_reverseAPIPort;
+    }
+    if (settingsKeys.contains("reverseAPIDeviceIndex") || force) {
+        ostr << " m_reverseAPIDeviceIndex: " << m_reverseAPIDeviceIndex;
+    }
+    if (settingsKeys.contains("iqOrder") || force) {
+        ostr << " m_iqOrder: " << m_iqOrder;
+    }
+
+    return QString(ostr.str().c_str());
+}

airspysettings.h

diff --git a/plugins/samplesource/airspy/airspysettings.h b/plugins/samplesource/airspy/airspysettings.h
index 5b2850fcd..7ad403c33 100644
--- a/plugins/samplesource/airspy/airspysettings.h
+++ b/plugins/samplesource/airspy/airspysettings.h
@@ -52,6 +52,8 @@ struct AirspySettings {
 	void resetToDefaults();
 	QByteArray serialize() const;
 	bool deserialize(const QByteArray& data);
+    void applySettings(const QStringList& settingsKeys, const AirspySettings& settings);
+    QString getDebugString(const QStringList& settingsKeys, bool force=false) const;
 };
 
 #endif /* _AIRSPY_AIRSPYSETTINGS_H_ */

@f4exb
Copy link
Owner

f4exb commented Nov 19, 2022

Full diff for AFC feature

afc.cpp

diff --git a/plugins/feature/afc/afc.cpp b/plugins/feature/afc/afc.cpp
index ba4c6097f..57bc499cf 100644
--- a/plugins/feature/afc/afc.cpp
+++ b/plugins/feature/afc/afc.cpp
@@ -118,7 +118,7 @@ void AFC::start()
     m_worker->setMessageQueueToGUI(getMessageQueueToGUI());
     m_thread->start();
 
-    AFCWorker::MsgConfigureAFCWorker *msg = AFCWorker::MsgConfigureAFCWorker::create(m_settings, true);
+    AFCWorker::MsgConfigureAFCWorker *msg = AFCWorker::MsgConfigureAFCWorker::create(m_settings, QList<QString>(), true);
     m_worker->getInputMessageQueue()->push(msg);
 
     m_state = StRunning;
@@ -146,7 +146,7 @@ bool AFC::handleMessage(const Message& cmd)
 	{
         MsgConfigureAFC& cfg = (MsgConfigureAFC&) cmd;
         qDebug() << "AFC::handleMessage: MsgConfigureAFC";
-        applySettings(cfg.getSettings(), cfg.getForce());
+        applySettings(cfg.getSettings(), cfg.getSettingsKeys(), cfg.getForce());
 
 		return true;
 	}
@@ -225,70 +225,32 @@ bool AFC::deserialize(const QByteArray& data)
 {
     if (m_settings.deserialize(data))
     {
-        MsgConfigureAFC *msg = MsgConfigureAFC::create(m_settings, true);
+        MsgConfigureAFC *msg = MsgConfigureAFC::create(m_settings, QList<QString>(), true);
         m_inputMessageQueue.push(msg);
         return true;
     }
     else
     {
         m_settings.resetToDefaults();
-        MsgConfigureAFC *msg = MsgConfigureAFC::create(m_settings, true);
+        MsgConfigureAFC *msg = MsgConfigureAFC::create(m_settings, QList<QString>(), true);
         m_inputMessageQueue.push(msg);
         return false;
     }
 }
 
-void AFC::applySettings(const AFCSettings& settings, bool force)
+void AFC::applySettings(const AFCSettings& settings, const QList<QString>& settingsKeys, bool force)
 {
-    qDebug() << "AFC::applySettings:"
-            << " m_title: " << settings.m_title
-            << " m_rgbColor: " << settings.m_rgbColor
-            << " m_trackerDeviceSetIndex: " << settings.m_trackerDeviceSetIndex
-            << " m_trackedDeviceSetIndex: " << settings.m_trackedDeviceSetIndex
-            << " m_hasTargetFrequency: " << settings.m_hasTargetFrequency
-            << " m_transverterTarget: " << settings.m_transverterTarget
-            << " m_targetFrequency: " << settings.m_targetFrequency
-            << " m_freqTolerance: " << settings.m_freqTolerance
-            << " m_trackerAdjustPeriod:" << settings.m_trackerAdjustPeriod
-            << " force: " << force;
+    qDebug() << "AFC::applySettings:" << settings.getDebugString(settingsKeys, force) << " force: " << force;
 
     QList<QString> reverseAPIKeys; // ===> TO BE REMOVED!
 
-    if ((m_settings.m_title != settings.m_title) || force) {
-        reverseAPIKeys.append("title");
-    }
-    if ((m_settings.m_rgbColor != settings.m_rgbColor) || force) {
-        reverseAPIKeys.append("rgbColor");
-    }
-    if ((m_settings.m_trackerDeviceSetIndex != settings.m_trackerDeviceSetIndex) || force) {
-        reverseAPIKeys.append("trackerDeviceSetIndex");
-    }
-    if ((m_settings.m_trackedDeviceSetIndex != settings.m_trackedDeviceSetIndex) || force) {
-        reverseAPIKeys.append("trackedDeviceSetIndex");
-    }
-    if ((m_settings.m_hasTargetFrequency != settings.m_hasTargetFrequency) || force) {
-        reverseAPIKeys.append("hasTargetFrequency");
-    }
-    if ((m_settings.m_transverterTarget != settings.m_transverterTarget) || force) {
-        reverseAPIKeys.append("transverterTarget");
-    }
-    if ((m_settings.m_targetFrequency != settings.m_targetFrequency) || force) {
-        reverseAPIKeys.append("targetFrequency");
-    }
-    if ((m_settings.m_freqTolerance != settings.m_freqTolerance) || force) {
-        reverseAPIKeys.append("freqTolerance");
-    }
-    if ((m_settings.m_trackerAdjustPeriod != settings.m_trackerAdjustPeriod) || force) {
-        reverseAPIKeys.append("trackerAdjustPeriod");
-    }
-
-    if ((m_settings.m_trackerDeviceSetIndex != settings.m_trackerDeviceSetIndex) || force)
+    if (settingsKeys.contains("trackerDeviceSetIndex") || force)
     {
         removeTrackerFeatureReference();
         trackerDeviceChange(settings.m_trackerDeviceSetIndex);
     }
 
-    if ((m_settings.m_trackedDeviceSetIndex != settings.m_trackedDeviceSetIndex) || force)
+    if (settingsKeys.contains("trackedDeviceSetIndex") || force)
     {
         removeTrackedFeatureReferences();
         trackedDeviceChange(settings.m_trackedDeviceSetIndex);
@@ -297,22 +259,27 @@ void AFC::applySettings(const AFCSettings& settings, bool force)
     if (m_running)
     {
         AFCWorker::MsgConfigureAFCWorker *msg = AFCWorker::MsgConfigureAFCWorker::create(
-            settings, force
+            settings, settingsKeys, force
         );
         m_worker->getInputMessageQueue()->push(msg);
     }
 
-    if (settings.m_useReverseAPI)
+    if (settingsKeys.contains("useReverseAPI"))
     {
-        bool fullUpdate = ((m_settings.m_useReverseAPI != settings.m_useReverseAPI) && settings.m_useReverseAPI) ||
-                (m_settings.m_reverseAPIAddress != settings.m_reverseAPIAddress) ||
-                (m_settings.m_reverseAPIPort != settings.m_reverseAPIPort) ||
-                (m_settings.m_reverseAPIFeatureSetIndex != settings.m_reverseAPIFeatureSetIndex) ||
-                (m_settings.m_reverseAPIFeatureIndex != settings.m_reverseAPIFeatureIndex);
+
+        bool fullUpdate = (settingsKeys.contains("useReverseAPI") && settings.m_useReverseAPI) ||
+                settingsKeys.contains("reverseAPIAddress") ||
+                settingsKeys.contains("reverseAPIPort") ||
+                settingsKeys.contains("reverseAPIFeatureSetIndex") ||
+                settingsKeys.contains("m_reverseAPIFeatureIndex");
         webapiReverseSendSettings(reverseAPIKeys, settings, fullUpdate || force);
// CORRECT ==>          webapiReverseSendSettings(settingsKeys, settings, fullUpdate || force);
     }
 
-    m_settings = settings;
+    if (force) {
+        m_settings = settings;
+    } else {
+        m_settings.applySettings(settingsKeys, settings);
+   }
 }
 
 void AFC::updateDeviceSetLists()
@@ -384,13 +351,13 @@ int AFC::webapiSettingsPutPatch(
     AFCSettings settings = m_settings;
     webapiUpdateFeatureSettings(settings, featureSettingsKeys, response);
 
-    MsgConfigureAFC *msg = MsgConfigureAFC::create(settings, force);
+    MsgConfigureAFC *msg = MsgConfigureAFC::create(settings, featureSettingsKeys, force);
     m_inputMessageQueue.push(msg);
 
     qDebug("AFC::webapiSettingsPutPatch: forward to GUI: %p", m_guiMessageQueue);
     if (m_guiMessageQueue) // forward to GUI if any
     {
-        MsgConfigureAFC *msgToGUI = MsgConfigureAFC::create(settings, force);
+        MsgConfigureAFC *msgToGUI = MsgConfigureAFC::create(settings, featureSettingsKeys, force);
         m_guiMessageQueue->push(msgToGUI);
     }
 
@@ -579,7 +546,7 @@ void AFC::webapiFormatFeatureReport(SWGSDRangel::SWGFeatureReport& response)
     }
 }
 
-void AFC::webapiReverseSendSettings(QList<QString>& channelSettingsKeys, const AFCSettings& settings, bool force)
+void AFC::webapiReverseSendSettings(const QList<QString>& channelSettingsKeys, const AFCSettings& settings, bool force)
 {
     SWGSDRangel::SWGFeatureSettings *swgFeatureSettings = new SWGSDRangel::SWGFeatureSettings();
     // swgFeatureSettings->setOriginatorFeatureIndex(getIndexInDeviceSet());

afc.h

diff --git a/plugins/feature/afc/afc.h b/plugins/feature/afc/afc.h
index ff6a14452..eeb471b71 100644
--- a/plugins/feature/afc/afc.h
+++ b/plugins/feature/afc/afc.h
@@ -46,19 +46,22 @@ public:
 
     public:
         const AFCSettings& getSettings() const { return m_settings; }
+        const QList<QString>& getSettingsKeys() const { return m_settingsKeys; }
         bool getForce() const { return m_force; }
 
-        static MsgConfigureAFC* create(const AFCSettings& settings, bool force) {
-            return new MsgConfigureAFC(settings, force);
+        static MsgConfigureAFC* create(const AFCSettings& settings, const QList<QString>& settingsKeys, bool force) {
+            return new MsgConfigureAFC(settings, settingsKeys, force);
         }
 
     private:
         AFCSettings m_settings;
+        QList<QString> m_settingsKeys;
         bool m_force;
 
-        MsgConfigureAFC(const AFCSettings& settings, bool force) :
+        MsgConfigureAFC(const AFCSettings& settings, const QList<QString>& settingsKeys, bool force) :
             Message(),
             m_settings(settings),
+            m_settingsKeys(settingsKeys),
             m_force(force)
         { }
     };
@@ -218,9 +221,9 @@ private:
 
     void start();
     void stop();
-    void applySettings(const AFCSettings& settings, bool force = false);
+    void applySettings(const AFCSettings& settings, const QList<QString>& settingsKeys, bool force = false);
     void webapiFormatFeatureReport(SWGSDRangel::SWGFeatureReport& response);
-    void webapiReverseSendSettings(QList<QString>& featureSettingsKeys, const AFCSettings& settings, bool force);
+    void webapiReverseSendSettings(const QList<QString>& featureSettingsKeys, const AFCSettings& settings, bool force);
     void trackerDeviceChange(int deviceIndex);
     void trackedDeviceChange(int deviceIndex);
     void removeTrackerFeatureReference();

afcgui.cpp

diff --git a/plugins/feature/afc/afcgui.cpp b/plugins/feature/afc/afcgui.cpp
index f61a1ed6c..ec8866602 100644
--- a/plugins/feature/afc/afcgui.cpp
+++ b/plugins/feature/afc/afcgui.cpp
@@ -72,7 +72,13 @@ bool AFCGUI::handleMessage(const Message& message)
     {
         qDebug("AFCGUI::handleMessage: AFC::MsgConfigureAFC");
         const AFC::MsgConfigureAFC& cfg = (AFC::MsgConfigureAFC&) message;
-        m_settings = cfg.getSettings();
+
+        if (cfg.getForce()) {
+            m_settings = cfg.getSettings();
+        } else {
+            m_settings.applySettings(cfg.getSettingsKeys(), cfg.getSettings());
+        }
+
         blockApplySettings(true);
         displaySettings();
         blockApplySettings(false);
@@ -258,6 +264,8 @@ void AFCGUI::updateDeviceSetLists(const AFC::MsgDeviceSetListsReport& report)
         qDebug("AFCGUI::updateDeviceSetLists: device index changed: %d:%d", trackerDeviceIndex, trackedDeviceIndex);
         m_settings.m_trackerDeviceSetIndex = trackerDeviceIndex;
         m_settings.m_trackedDeviceSetIndex = trackedDeviceIndex;
+        m_settingsKeys.append("trackerDeviceSetIndex");
+        m_settingsKeys.append("trackedDeviceSetIndex");
         applySettings();
     }
 
@@ -291,6 +299,12 @@ void AFCGUI::onMenuDialogCalled(const QPoint &p)
         setTitle(m_settings.m_title);
         setTitleColor(m_settings.m_rgbColor);
 
// MISSING ==>
//         m_settingsKeys.append("title");
//         m_settingsKeys.append("rgbColor");
+        m_settingsKeys.append("useReverseAPI");
+        m_settingsKeys.append("reverseAPIAddress");
+        m_settingsKeys.append("reverseAPIPort");
+        m_settingsKeys.append("reverseAPIFeatureSetIndex");
+        m_settingsKeys.append("reverseAPIFeatureIndex");
+
         applySettings();
     }
 
@@ -309,18 +323,21 @@ void AFCGUI::on_startStop_toggled(bool checked)
 void AFCGUI::on_hasTargetFrequency_toggled(bool checked)
 {
     m_settings.m_hasTargetFrequency = checked;
+    m_settingsKeys.append("hasTargetFrequency");
     applySettings();
 }
 
 void AFCGUI::on_targetFrequency_changed(quint64 value)
 {
     m_settings.m_targetFrequency = value;
+    m_settingsKeys.append("targetFrequency");
     applySettings();
 }
 
 void AFCGUI::on_transverterTarget_toggled(bool checked)
 {
     m_settings.m_transverterTarget = checked;
+    m_settingsKeys.append("transverterTarget");
     applySettings();
 }
 
@@ -328,6 +345,7 @@ void AFCGUI::on_transverterTarget_toggled(bool checked)
 void AFCGUI::on_toleranceFrequency_changed(quint64 value)
 {
     m_settings.m_freqTolerance = value;
+    m_settingsKeys.append("freqTolerance");
     applySettings();
 }
 
@@ -347,6 +365,7 @@ void AFCGUI::on_trackerDevice_currentIndexChanged(int index)
     if (index >= 0)
     {
         m_settings.m_trackerDeviceSetIndex = index;
+        m_settingsKeys.append("trackerDeviceSetIndex");
         applySettings();
     }
 }
@@ -356,6 +375,7 @@ void AFCGUI::on_trackedDevice_currentIndexChanged(int index)
     if (index >= 0)
     {
         m_settings.m_trackedDeviceSetIndex = index;
+        m_settingsKeys.append("trackedDeviceSetIndex");
         applySettings();
     }
 }
@@ -370,6 +390,7 @@ void AFCGUI::on_targetPeriod_valueChanged(int value)
 {
     m_settings.m_trackerAdjustPeriod = value;
     ui->targetPeriodText->setText(tr("%1").arg(m_settings.m_trackerAdjustPeriod));
+    m_settingsKeys.append("trackerAdjustPeriod");
     applySettings();
 }
 
@@ -411,7 +432,7 @@ void AFCGUI::applySettings(bool force)
 {
 	if (m_doApplySettings)
 	{
-	    AFC::MsgConfigureAFC* message = AFC::MsgConfigureAFC::create( m_settings, force);
+	    AFC::MsgConfigureAFC* message = AFC::MsgConfigureAFC::create( m_settings, m_settingsKeys, force);
 	    m_afc->getInputMessageQueue()->push(message);
 	}

// ===> missing m_settingsKeys.clear();
 }

afcgui.h

diff --git a/plugins/feature/afc/afcgui.h b/plugins/feature/afc/afcgui.h
index 22f32c229..6aa2e78fa 100644
--- a/plugins/feature/afc/afcgui.h
+++ b/plugins/feature/afc/afcgui.h
@@ -53,6 +53,7 @@ private:
 	PluginAPI* m_pluginAPI;
 	FeatureUISet* m_featureUISet;
 	AFCSettings m_settings;
+    QList<QString> m_settingsKeys;
 	RollupState m_rollupState;
 	bool m_doApplySettings;

afcsettings.cpp

diff --git a/plugins/feature/afc/afcsettings.cpp b/plugins/feature/afc/afcsettings.cpp
index d8dc8eed5..9d4327753 100644
--- a/plugins/feature/afc/afcsettings.cpp
+++ b/plugins/feature/afc/afcsettings.cpp
@@ -133,3 +133,105 @@ bool AFCSettings::deserialize(const QByteArray& data)
         return false;
     }
 }
+
+void AFCSettings::applySettings(const QStringList& settingsKeys, const AFCSettings& settings)
+{
+    if (settingsKeys.contains("title")) {
+        m_title = settings.m_title;
+    }
+    if (settingsKeys.contains("rgbColor")) {
+        m_rgbColor = settings.m_rgbColor;
+    }
+    if (settingsKeys.contains("trackerDeviceSetIndex")) {
+        m_trackerDeviceSetIndex = settings.m_trackerDeviceSetIndex;
+    }
+    if (settingsKeys.contains("trackedDeviceSetIndex")) {
+        m_trackedDeviceSetIndex = settings.m_trackedDeviceSetIndex;
+    }
+    if (settingsKeys.contains("hasTargetFrequency")) {
+        m_hasTargetFrequency = settings.m_hasTargetFrequency;
+    }
+    if (settingsKeys.contains("transverterTarget")) {
+        m_transverterTarget = settings.m_transverterTarget;
+    }
+    if (settingsKeys.contains("targetFrequency")) {
+        m_targetFrequency = settings.m_targetFrequency;
+    }
+    if (settingsKeys.contains("freqTolerance")) {
+        m_freqTolerance = settings.m_freqTolerance;
+    }
+    if (settingsKeys.contains("trackerAdjustPeriod")) {
+        m_trackerAdjustPeriod = settings.m_trackerAdjustPeriod;
+    }
+    if (settingsKeys.contains("useReverseAPI")) {
+        m_useReverseAPI = settings.m_useReverseAPI;
+    }
+    if (settingsKeys.contains("reverseAPIAddress")) {
+        m_reverseAPIAddress = settings.m_reverseAPIAddress;
+    }
+    if (settingsKeys.contains("reverseAPIPort")) {
+        m_reverseAPIPort = settings.m_reverseAPIPort;
+    }
+    if (settingsKeys.contains("reverseAPIFeatureSetIndex")) {
+        m_reverseAPIFeatureSetIndex = settings.m_reverseAPIFeatureSetIndex;
+    }
+    if (settingsKeys.contains("reverseAPIFeatureIndex")) {
+        m_reverseAPIFeatureIndex = settings.m_reverseAPIFeatureIndex;
+    }
+    if (settingsKeys.contains("workspaceIndex")) {
+        m_workspaceIndex = settings.m_workspaceIndex;
+    }
+}
+
+QString AFCSettings::getDebugString(const QStringList& settingsKeys, bool force) const
+{
+    std::ostringstream ostr;
+
+    if (settingsKeys.contains("title") || force) {
+        ostr << " m_title: " << m_title.toStdString();
+    }
+    if (settingsKeys.contains("rgbColor") || force) {
+        ostr << " m_rgbColor: " << m_rgbColor;
+    }
+    if (settingsKeys.contains("trackerDeviceSetIndex") || force) {
+        ostr << " m_trackerDeviceSetIndex: " << m_trackerDeviceSetIndex;
+    }
+    if (settingsKeys.contains("trackedDeviceSetIndex") || force) {
+        ostr << " m_trackedDeviceSetIndex: " << m_trackedDeviceSetIndex;
+    }
+    if (settingsKeys.contains("hasTargetFrequency") || force) {
+        ostr << " m_hasTargetFrequency: " << m_hasTargetFrequency;
+    }
+    if (settingsKeys.contains("transverterTarget") || force) {
+        ostr << " m_transverterTarget: " << m_transverterTarget;
+    }
+    if (settingsKeys.contains("targetFrequency") || force) {
+        ostr << " m_targetFrequency: " << m_targetFrequency;
+    }
+    if (settingsKeys.contains("freqTolerance") || force) {
+        ostr << " m_freqTolerance: " << m_freqTolerance;
+    }
+    if (settingsKeys.contains("trackerAdjustPeriod") || force) {
+        ostr << " m_trackerAdjustPeriod: " << m_trackerAdjustPeriod;
+    }
+    if (settingsKeys.contains("useReverseAPI") || force) {
+        ostr << " m_useReverseAPI: " << m_useReverseAPI;
+    }
+    if (settingsKeys.contains("reverseAPIAddress") || force) {
+        ostr << " m_reverseAPIAddress: " << m_reverseAPIAddress.toStdString();
+    }
+    if (settingsKeys.contains("reverseAPIPort") || force) {
+        ostr << " m_reverseAPIPort: " << m_reverseAPIPort;
+    }
+    if (settingsKeys.contains("reverseAPIFeatureSetIndex") || force) {
+        ostr << " m_reverseAPIFeatureSetIndex: " << m_reverseAPIFeatureSetIndex;
+    }
+    if (settingsKeys.contains("reverseAPIFeatureIndex") || force) {
+        ostr << " m_reverseAPIFeatureIndex: " << m_reverseAPIFeatureIndex;
+    }
+    if (settingsKeys.contains("workspaceIndex") || force) {
+        ostr << " m_workspaceIndex: " << m_workspaceIndex;
+    }
+
+    return QString(ostr.str().c_str());
+}

afcsettings.h

diff --git a/plugins/feature/afc/afcsettings.h b/plugins/feature/afc/afcsettings.h
index fb3d750a4..cb3d6ae3e 100644
--- a/plugins/feature/afc/afcsettings.h
+++ b/plugins/feature/afc/afcsettings.h
@@ -48,6 +48,8 @@ struct AFCSettings
     QByteArray serialize() const;
     bool deserialize(const QByteArray& data);
     void setRollupState(Serializable *rollupState) { m_rollupState = rollupState; }
+    void applySettings(const QStringList& settingsKeys, const AFCSettings& settings);
+    QString getDebugString(const QStringList& settingsKeys, bool force=false) const;
 };
 
 #endif // INCLUDE_FEATURE_AFCSETTINGS_H_

afcworker.cpp

diff --git a/plugins/feature/afc/afcworker.cpp b/plugins/feature/afc/afcworker.cpp
index 7274b4f09..2790cf289 100644
--- a/plugins/feature/afc/afcworker.cpp
+++ b/plugins/feature/afc/afcworker.cpp
@@ -99,7 +99,7 @@ bool AFCWorker::handleMessage(const Message& cmd)
         QMutexLocker mutexLocker(&m_mutex);
         MsgConfigureAFCWorker& cfg = (MsgConfigureAFCWorker&) cmd;
 
-        applySettings(cfg.getSettings(), cfg.getForce());
+        applySettings(cfg.getSettings(), cfg.getSettingsKeys(), cfg.getForce());
 
         return true;
     }
@@ -135,32 +135,23 @@ bool AFCWorker::handleMessage(const Message& cmd)
     }
 }
 
-void AFCWorker::applySettings(const AFCSettings& settings, bool force)
+void AFCWorker::applySettings(const AFCSettings& settings, const QList<QString>& settingsKeys, bool force)
 {
-    qDebug() << "AFCWorker::applySettings:"
-            << " m_title: " << settings.m_title
-            << " m_rgbColor: " << settings.m_rgbColor
-            << " m_trackerDeviceSetIndex: " << settings.m_trackerDeviceSetIndex
-            << " m_trackedDeviceSetIndex: " << settings.m_trackedDeviceSetIndex
-            << " m_hasTargetFrequency: " << settings.m_hasTargetFrequency
-            << " m_transverterTarget: " << settings.m_transverterTarget
-            << " m_targetFrequency: " << settings.m_targetFrequency
-            << " m_freqTolerance: " << settings.m_freqTolerance
-            << " force: " << force;
-
-    if ((settings.m_trackerDeviceSetIndex != m_settings.m_trackerDeviceSetIndex) || force) {
+    qDebug() << "AFCWorker::applySettings:" << settings.getDebugString(settingsKeys, force)  << " force: " << force;
+
+    if (settingsKeys.contains("trackerDeviceSetIndex") || force) {
         initTrackerDeviceSet(settings.m_trackerDeviceSetIndex);
     }
 
-    if ((settings.m_trackedDeviceSetIndex != m_settings.m_trackedDeviceSetIndex) || force) {
+    if (settingsKeys.contains("trackedDeviceSetIndex") || force) {
         initTrackedDeviceSet(settings.m_trackedDeviceSetIndex);
     }
 
-    if ((settings.m_trackerAdjustPeriod != m_settings.m_trackerAdjustPeriod) || force) {
+    if (settingsKeys.contains("trackerAdjustPeriod") || force) {
         m_updateTimer.setInterval(settings.m_trackerAdjustPeriod * 1000);
     }
 
-    if ((settings.m_hasTargetFrequency != m_settings.m_hasTargetFrequency) || force)
+    if (settingsKeys.contains("hasTargetFrequency") || force)
     {
         if (settings.m_hasTargetFrequency) {
             m_updateTimer.start(m_settings.m_trackerAdjustPeriod * 1000);

afcworker.h

diff --git a/plugins/feature/afc/afcworker.h b/plugins/feature/afc/afcworker.h
index 19b5b61ad..3ec5a9d47 100644
--- a/plugins/feature/afc/afcworker.h
+++ b/plugins/feature/afc/afcworker.h
@@ -40,20 +40,23 @@ public:
 
     public:
         const AFCSettings& getSettings() const { return m_settings; }
+        const QList<QString>& getSettingsKeys() const { return m_settingsKeys; }
         bool getForce() const { return m_force; }
 
-        static MsgConfigureAFCWorker* create(const AFCSettings& settings, bool force)
+        static MsgConfigureAFCWorker* create(const AFCSettings& settings, const QList<QString>& settingsKeys, bool force)
         {
-            return new MsgConfigureAFCWorker(settings, force);
+            return new MsgConfigureAFCWorker(settings, settingsKeys, force);
         }
 
     private:
         AFCSettings m_settings;
+        QList<QString> m_settingsKeys;
         bool m_force;
 
-        MsgConfigureAFCWorker(const AFCSettings& settings, bool force) :
+        MsgConfigureAFCWorker(const AFCSettings& settings, const QList<QString>& settingsKeys, bool force) :
             Message(),
             m_settings(settings),
+            m_settingsKeys(settingsKeys),
             m_force(force)
         { }
     };
@@ -154,7 +157,7 @@ private:
     QRecursiveMutex m_mutex;
 
     bool handleMessage(const Message& cmd);
-    void applySettings(const AFCSettings& settings, bool force = false);
+    void applySettings(const AFCSettings& settings, const QList<QString>& settingsKeys, bool force = false);
     void initTrackerDeviceSet(int deviceSetIndex);
     void initTrackedDeviceSet(int deviceSetIndex);
     void processChannelSettings(

@f4exb f4exb modified the milestone: v7.8.4 Dec 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants