Ensure exclusive access to radios per editorset

A ChirpLiveEditorSet represents a conversation with a single radio
endpoint, but may involve multiple independent event streams. We
really don't need multiple threads and queues for talking to a single
radio, however radios with multiple sub-devices will end up with
different Radio classes by nature. So, instead of collapsing editors
into a single thread, we can just make them share a lock, established
at the EditorSet level, which is what this patch does.

This also means the other drivers (that I know of) that do their
own locking (kenwood_live, kenwood_d7, because settings) and ic9x
(because multiple sub-devices) can simplify and rely on this being
handled in the UI.

Related to #10683

chirp/drivers/ic9x.py

@@ -52,31 +52,27 @@
     "!\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~"
 
 
-class Lock:
+class IC9xState:
     """Maintains the state of an ic9x
 
     This makes sure that only one RadioThread accesses the radio at one
     time. It also keeps track of the last successful unlock of the radio
     so that we know when to re-send the magic wakeup sequence.
     """
     def __init__(self):
-        self.lock = threading.Lock()
         self.id = str(uuid.uuid4())
         self._last = 0
 
     def __enter__(self):
-        LOG.debug('%s locking', self.id)
-        self.lock.acquire()
-        LOG.debug('%s locked', self.id)
+        pass
 
     def __exit__(self, exc_type, exc_value, exc_tb):
-        self.lock.release()
         if exc_type is None:
             self._last = time.time()
         LOG.debug('%s unlocked success=%s', self.id, exc_type is None)
 
     def __repr__(self):
-        return '<IC9x Lock %s>' % self.id
+        return '<IC9x State %s>' % self.id
 
     @property
     def stale(self):
@@ -170,7 +166,7 @@ def __init__(self, *args, **kwargs):
         if 'lock' in kwargs:
             self._lock = kwargs.pop('lock')
         else:
-            self._lock = Lock()
+            self._lock = IC9xState()
         super().__init__(*args, **kwargs)
 
         self.__memcache = {}

chirp/drivers/kenwood_d7.py

@@ -977,28 +977,24 @@ def _set_setting(self, name, sdata, element):
     def _command(self, cmd, *args):
         """Send @cmd to radio via @ser"""
 
-        # This lock is needed to allow clicking the settings tab while
-        # the memories are still loading.  Most important with the TH-D7A
-        # and TH-D7A(G) with the 9600bps maximum.
-        with self._LOCK:
-            if args:
-                cmd += self._ARG_DELIMITER + self._ARG_DELIMITER.join(args)
-            cmd += self._CMD_DELIMITER
-            self._drain_input()
-
-            LOG.debug("PC->RADIO: %s" % cmd.strip())
-            self.pipe.write(cmd.encode('cp1252'))
-            cd = self._CMD_DELIMITER.encode('cp1252')
-            keep_reading = True
-            while keep_reading:
-                result = self.pipe.read_until(cd).decode('cp1252')
-                if result.endswith(self._CMD_DELIMITER):
-                    keep_reading = self._keep_reading(result)
-                    LOG.debug("RADIO->PC: %r" % result.strip())
-                    result = result[:-1]
-                else:
-                    keep_reading = False
-                    LOG.error("Timeout waiting for data")
+        if args:
+            cmd += self._ARG_DELIMITER + self._ARG_DELIMITER.join(args)
+        cmd += self._CMD_DELIMITER
+        self._drain_input()
+
+        LOG.debug("PC->RADIO: %s" % cmd.strip())
+        self.pipe.write(cmd.encode('cp1252'))
+        cd = self._CMD_DELIMITER.encode('cp1252')
+        keep_reading = True
+        while keep_reading:
+            result = self.pipe.read_until(cd).decode('cp1252')
+            if result.endswith(self._CMD_DELIMITER):
+                keep_reading = self._keep_reading(result)
+                LOG.debug("RADIO->PC: %r" % result.strip())
+                result = result[:-1]
+            else:
+                keep_reading = False
+                LOG.error("Timeout waiting for data")
 
         return result.strip()
 

chirp/drivers/kenwood_live.py

@@ -54,7 +54,6 @@
     "ID023": "TS-590S/SG_LiveMode"          # as SG
 }
 
-LOCK = threading.Lock()
 COMMAND_RESP_BUFSIZE = 8
 LAST_BAUD = 4800
 LAST_DELIMITER = ("\r", " ")
@@ -67,7 +66,7 @@
 
 def _command(ser, cmd, *args):
     """Send @cmd to radio via @ser"""
-    global LOCK, LAST_DELIMITER, COMMAND_RESP_BUFSIZE
+    global LAST_DELIMITER, COMMAND_RESP_BUFSIZE
 
     start = time.time()
 
@@ -97,8 +96,7 @@ def _command(ser, cmd, *args):
 
 
 def command(ser, cmd, *args):
-    with LOCK:
-        return _command(ser, cmd, *args)
+    return _command(ser, cmd, *args)
 
 
 def get_id(ser):

chirp/wxui/main.py

@@ -23,6 +23,7 @@
 import shutil
 import sys
 import tempfile
+import threading
 import time
 import webbrowser
 
@@ -344,11 +345,12 @@ def tab_name(self):
 
     def __init__(self, radio, *a, **k):
         self._threads = []
+        self._lock = threading.Lock()
         super().__init__(radio, *a, **k)
 
     def add_editor(self, editor, title):
         super(ChirpLiveEditorSet, self).add_editor(editor, title)
-        thread = radiothread.RadioThread(editor._radio)
+        thread = radiothread.RadioThread(editor._radio, self._lock)
         thread.start()
         self._threads.append(thread)
         editor.set_radio_thread(thread)

chirp/wxui/radiothread.py

@@ -86,9 +86,10 @@ def score(self):
 class RadioThread(threading.Thread):
     SENTINEL = RadioJob(None, 'END', [], {})
 
-    def __init__(self, radio):
+    def __init__(self, radio, lock):
         super().__init__()
         self._radio = radio
+        self._lock = lock
         self._queue = queue.PriorityQueue()
         self._log = logging.getLogger('RadioThread')
         self._waiting = []
@@ -112,7 +113,8 @@ def run(self):
             if job is self.SENTINEL:
                 self._log.info('Exiting on request')
                 return
-            job.dispatch(self._radio)
+            with self._lock:
+                job.dispatch(self._radio)
             self._waiting.append(job)
 
             for job in list(self._waiting):

tests/unit/test_wxui_radiothread.py

@@ -1,5 +1,6 @@
 import sys
 import time
+import threading
 from unittest import mock
 
 sys.modules['wx'] = wx = mock.MagicMock()
@@ -13,6 +14,7 @@
 class TestRadioThread(base.BaseTest):
     def setUp(self):
         super().setUp()
+        self._lock = threading.Lock()
 
     def test_radiojob(self):
         radio = mock.MagicMock()
@@ -38,7 +40,7 @@ def test_thread(self):
         # Simulate an edit conflict with the first event by returning
         # False for "delivered" to force us to queue an event.
         editor.radio_thread_event.side_effect = [False, True, True, True]
-        thread = radiothread.RadioThread(radio)
+        thread = radiothread.RadioThread(radio, self._lock)
         mem = mock.MagicMock()
         job1id = thread.submit(editor, 'get_memory', 12)
         job2id = thread.submit(editor, 'set_memory', mem)
@@ -85,7 +87,7 @@ def test_thread_abort_priority(self):
         radio = mock.MagicMock()
         radio.get_features.side_effect = ValueError('some error')
         editor = mock.MagicMock()
-        thread = radiothread.RadioThread(radio)
+        thread = radiothread.RadioThread(radio, self._lock)
         mem = mock.MagicMock()
         thread.submit(editor, 'get_memory', 12)
         thread.submit(editor, 'set_memory', mem)
@@ -105,3 +107,19 @@ def test_thread_abort_priority(self):
         radio.set_memory.assert_not_called()
         radio.get_features.assert_not_called()
         wx.PostEvent.assert_not_called()
+
+    def _lock_tester(self):
+        self.assertTrue(self._lock.locked)
+        return mock.sentinel.iran
+
+    def test_lock(self):
+        radio = mock.MagicMock()
+        radio.test = self._lock_tester
+        editor = mock.MagicMock()
+        thread = radiothread.RadioThread(radio, self._lock)
+        thread.submit(editor, 'test')
+        thread.start()
+        thread.end()
+        thread.join(5)
+        job = editor.radio_thread_event.call_args_list[0][0][0]
+        self.assertEqual(mock.sentinel.iran, job.result)