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

@@ -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,
+        )

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:

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 <https://www.gnu.org/licenses/>.
 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
 

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

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))

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)

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')