From abb80a5160f88c82bc95fa0d7ddb8f3f13af1240 Mon Sep 17 00:00:00 2001 From: Name Date: Sun, 30 Jul 2023 09:03:06 +0200 Subject: [PATCH] Sweep.py: add getters and setters for private fields (#659) * style, Sweep.py: remove a double negation * style, NanoVNASaver.py: simplify sweepSource computation * Sweep.py: add getters and setters for private fields Beware that this commit removes a lock from SweepSettings.update_tex_power, and adds one to DeviceSettings.updatecustomPoint. Both changse may be incorrect, depending on the role of the lock (issue #657). Follows: 6eb24f23 d09b55e1 dbea311a Since d09b55e1, the Properties.name class attribute is overriden by each assignment to the properties.name instance attribute. This is most probably unwanted. This commit * removes @dataclass, which is confusing as some attributes are managed because of the lock. Because of this, it has to restore __repr__ and __eq__. * provides getters and setters for private attributes, and protects each update by a thread lock * adds a regression test for the bug fixed by d09b55e1 (immutable properties). --- src/NanoVNASaver/Controls/SweepControl.py | 12 +-- src/NanoVNASaver/NanoVNASaver.py | 10 +- src/NanoVNASaver/Settings/Sweep.py | 105 +++++++++++++++++---- src/NanoVNASaver/SweepWorker.py | 3 +- src/NanoVNASaver/Windows/DeviceSettings.py | 5 +- src/NanoVNASaver/Windows/SweepSettings.py | 15 +-- tests/test_sweep.py | 5 + 7 files changed, 113 insertions(+), 42 deletions(-) diff --git a/src/NanoVNASaver/Controls/SweepControl.py b/src/NanoVNASaver/Controls/SweepControl.py index 2ce569a..ba35735 100644 --- a/src/NanoVNASaver/Controls/SweepControl.py +++ b/src/NanoVNASaver/Controls/SweepControl.py @@ -216,9 +216,9 @@ class SweepControl(Control): self.update_sweep() def update_sweep(self): - sweep = self.app.sweep - with sweep.lock: - sweep.start = self.get_start() - sweep.end = self.get_end() - sweep.segments = self.get_segments() - sweep.points = self.app.vna.datapoints + self.app.sweep.update( + start = self.get_start(), + end = self.get_end(), + segments = self.get_segments(), + points = self.app.vna.datapoints, + ) diff --git a/src/NanoVNASaver/NanoVNASaver.py b/src/NanoVNASaver/NanoVNASaver.py index a3d3a6e..d4daa77 100644 --- a/src/NanoVNASaver/NanoVNASaver.py +++ b/src/NanoVNASaver/NanoVNASaver.py @@ -511,10 +511,12 @@ class NanoVNASaver(QWidget): if source is not None: self.sweepSource = source else: - self.sweepSource = ( - f"{self.sweep.properties.name}" - f" {strftime('%Y-%m-%d %H:%M:%S', localtime())}" - ).lstrip() + time = strftime('%Y-%m-%d %H:%M:%S', localtime()) + name = self.sweep.properties.name + if name: + self.sweepSource = name + ' ' + time + else: + self.sweepSource = time def markerUpdated(self, marker: Marker): with self.dataLock: diff --git a/src/NanoVNASaver/Settings/Sweep.py b/src/NanoVNASaver/Settings/Sweep.py index 6e45510..2ad44cf 100644 --- a/src/NanoVNASaver/Settings/Sweep.py +++ b/src/NanoVNASaver/Settings/Sweep.py @@ -17,11 +17,10 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . import logging -from dataclasses import dataclass, replace from enum import Enum from math import log from threading import Lock -from typing import Iterator +from typing import Iterator, NamedTuple logger = logging.getLogger(__name__) @@ -32,28 +31,69 @@ class SweepMode(Enum): AVERAGE = 2 -class Properties: +class Properties(NamedTuple): name: str = "" mode: "SweepMode" = SweepMode.SINGLE averages: tuple[int, int] = (3, 0) logarithmic: bool = False -@dataclass class Sweep: - start: int = 3600000 - end: int = 30000000 - points: int = 101 - segments: int = 1 - properties: "Properties" = Properties() - - def __post_init__(self): - self.lock = Lock() + def __init__(self, + start: int = 3600000, + end: int = 30000000, + points: int = 101, + segments: int = 1, + properties: "Properties" = Properties(), + ): + self._start = start + self._end = end + self._points = points + self._segments = segments + self._properties = properties + self._lock = Lock() self.check() logger.debug("%s", self) + def __repr__(self) -> str: + return 'Sweep(' + ', '.join(map(str, ( + self.start, self.end, self.points, self.segments, self.properties + ))) + ')' + + def __eq__(self, other) -> bool: + return (self.start == other.start + and self.end == other.end + and self.points == other.points + and self.segments == other.segments + and self.properties == other.properties) + def copy(self) -> "Sweep": - return replace(self) + with self._lock: + return Sweep(self.start, self.end, self.points, self.segments, + self._properties) + + # Getters for attributes, either private or computed. + + @property + def start(self) -> int: + return self._start + + @property + def end(self) -> int: + return self._end + + @property + def points(self) -> int: + return self._points + + @property + def segments(self) -> int: + return self._segments + + # Properties are immutable, this does not circumvent the accessors. + @property + def properties(self) -> "Properties": + return self._properties @property def span(self) -> int: @@ -63,6 +103,37 @@ class Sweep: def stepsize(self) -> int: return round(self.span / (self.points * self.segments - 1)) + # Setters + + def set_points(self, points: int) -> None: + with self._lock: + self._points = points + self.check() + + def update(self, start: int, end: int, segments: int, points: int) -> None: + with self._lock: + self._start = start + self._end = end + self._segments = segments + self._points = points + self.check() + + def set_name(self, name: str) -> None: + with self._lock: + self._properties = self.properties._replace(name=name) + + def set_mode(self, mode: "SweepMode") -> None: + with self._lock: + self._properties = self.properties._replace(mode=mode) + + def set_averages(self, amount: int, truncates: int) -> None: + with self._lock: + self._properties = self.properties._replace(averages=(amount, truncates)) + + def set_logarithmic(self, logarithmic: bool) -> None: + with self._lock: + self._properties = self.properties._replace(logarithmic=logarithmic) + def check(self): if ( self.segments <= 0 @@ -77,12 +148,12 @@ class Sweep: return 1 - log(self.segments + 1 - index) / log(self.segments + 1) def get_index_range(self, index: int) -> tuple[int, int]: - if not self.properties.logarithmic: - start = self.start + index * self.points * self.stepsize - end = start + (self.points - 1) * self.stepsize - else: + if self.properties.logarithmic: start = round(self.start + self.span * self._exp_factor(index)) end = round(self.start + self.span * self._exp_factor(index + 1)) + else: + start = self.start + index * self.points * self.stepsize + end = start + (self.points - 1) * self.stepsize logger.debug("get_index_range(%s) -> (%s, %s)", index, start, end) return start, end diff --git a/src/NanoVNASaver/SweepWorker.py b/src/NanoVNASaver/SweepWorker.py index 3cae9bc..fe8576d 100644 --- a/src/NanoVNASaver/SweepWorker.py +++ b/src/NanoVNASaver/SweepWorker.py @@ -93,8 +93,7 @@ class SweepWorker(QtCore.QRunnable): self.running = True self.percentage = 0 - with self.app.sweep.lock: - sweep = self.app.sweep.copy() + sweep = self.app.sweep.copy() if sweep != self.sweep: # parameters changed self.sweep = sweep diff --git a/src/NanoVNASaver/Windows/DeviceSettings.py b/src/NanoVNASaver/Windows/DeviceSettings.py index 1080d83..d746d4a 100644 --- a/src/NanoVNASaver/Windows/DeviceSettings.py +++ b/src/NanoVNASaver/Windows/DeviceSettings.py @@ -189,8 +189,7 @@ class DeviceSettingsWindow(QtWidgets.QWidget): return logger.debug("DP: %s", self.datapoints.itemText(i)) self.app.vna.datapoints = int(self.datapoints.itemText(i)) - with self.app.sweep.lock: - self.app.sweep.points = self.app.vna.datapoints + self.app.sweep.set_points(self.app.vna.datapoints) self.app.sweep_control.update_step_size() def updateBandwidth(self, i): @@ -217,6 +216,6 @@ class DeviceSettingsWindow(QtWidgets.QWidget): if points != self.app.vna.datapoints: logger.debug("DP: %s", points) self.app.vna.datapoints = points - self.app.sweep.points = self.app.vna.datapoints + self.app.sweep.set_points(self.app.vna.datapoints) self.app.sweep_control.update_step_size() self.custom_points_Eidt.setText(str(points)) diff --git a/src/NanoVNASaver/Windows/SweepSettings.py b/src/NanoVNASaver/Windows/SweepSettings.py index 1c013c3..1322062 100644 --- a/src/NanoVNASaver/Windows/SweepSettings.py +++ b/src/NanoVNASaver/Windows/SweepSettings.py @@ -290,18 +290,15 @@ class SweepSettingsWindow(QtWidgets.QWidget): logger.debug("update_averaging(%s, %s)", amount, truncates) averages.setText(str(amount)) truncs.setText(str(truncates)) - with self.app.sweep.lock: - self.app.sweep.properties.averages = (amount, truncates) + self.app.sweep.set_averages(amount, truncates) def update_logarithmic(self, logarithmic: bool): logger.debug("update_logarithmic(%s)", logarithmic) - with self.app.sweep.lock: - self.app.sweep.properties.logarithmic = logarithmic + self.app.sweep.set_logarithmic(logarithmic) def update_mode(self, mode: "SweepMode"): logger.debug("update_mode(%s)", mode) - with self.app.sweep.lock: - self.app.sweep.properties.mode = mode + self.app.sweep.set_mode(mode) def update_padding(self, padding: int): logger.debug("update_padding(%s)", padding) @@ -310,11 +307,9 @@ class SweepSettingsWindow(QtWidgets.QWidget): def update_title(self, title: str = ""): logger.debug("update_title(%s)", title) - with self.app.sweep.lock: - self.app.sweep.properties.name = title + self.app.sweep.set_name(title) self.app.update_sweep_title() def update_tx_power(self, freq_range, power_desc): logger.debug("update_tx_power(%r)", power_desc) - with self.app.sweep.lock: - self.app.vna.setTXPower(freq_range, power_desc) + self.app.vna.setTXPower(freq_range, power_desc) diff --git a/tests/test_sweep.py b/tests/test_sweep.py index ea0c3a6..3494270 100644 --- a/tests/test_sweep.py +++ b/tests/test_sweep.py @@ -53,3 +53,8 @@ class TestCases(unittest.TestCase): sweep2 = sweep.copy() self.assertEqual(sweep, sweep2) + + sweep.set_points(14) + self.assertEqual(sweep.points, 14) + sweep.set_name('bla') + self.assertEqual(sweep.properties.name, 'bla')