Refactor detection for class isolation

This makes us keep detected models separate per class that they
were @detected_by. Before this, we would end up with a mashup of all
the detected subclasses in the highest parent class that was a detect
target (i.e. TID H3 and H8 mixed together). Now, we keep them
separate at import/class define time so we don't have to further
filter them. Also, this builds in the include-self behavior to
detected_models() to avoid having to repeat that everwhere.

This also fixes a download..upload..download bug where we will corrupt
the last_model with the detected model name, which isn't selectable
on download (since it's detected and not listed).
This commit is contained in:
Dan Smith 2024-05-01 23:08:29 -07:00 committed by Dan Smith
parent 83084f72ff
commit d568940259
7 changed files with 101 additions and 24 deletions

View File

@ -1387,9 +1387,11 @@ class FileBackedRadio(Radio):
pass
class DetectableInterface:
DETECTED_MODELS = None
def class_detected_models_attribute(cls):
return 'DETECTED_MODELS_%s' % cls.__name__
class DetectableInterface:
@classmethod
def detect_from_serial(cls, pipe):
"""Communicate with the radio via serial to determine proper class
@ -1398,14 +1400,28 @@ class DetectableInterface:
RadioError if not. If NotImplemented is raised, we assume that no
detection is possible or necessary.
"""
assert cls.DETECTED_MODELS is None, (
detected = getattr(cls, class_detected_models_attribute(cls), None)
assert detected is None, (
'Class has detected models but no detect_from_serial() '
'implementation')
raise NotImplementedError()
@classmethod
def detected_models(cls):
return list(cls.DETECTED_MODELS or [])
def detected_models(cls, include_self=True):
detected = getattr(cls, class_detected_models_attribute(cls), [])
# Only include this class if it is registered
if include_self and hasattr(cls, '_DETECTED_BY'):
extra = [cls]
else:
extra = []
return extra + list(detected)
@classmethod
def detect_model(cls, detected_cls):
detected_attr = class_detected_models_attribute(cls)
if getattr(cls, detected_attr, None) is None:
setattr(cls, detected_attr, [])
getattr(cls, detected_attr).append(detected_cls)
class CloneModeRadio(FileBackedRadio, ExternalMemoryProperties,

View File

@ -63,8 +63,8 @@ def register(cls):
DRV_TO_RADIO[ident] = cls
RADIO_TO_DRV[cls] = ident
if not hasattr(cls, '_DETECTED_MODEL'):
cls._DETECTED_MODEL = False
if not hasattr(cls, '_DETECTED_BY'):
cls._DETECTED_BY = None
return cls
@ -81,10 +81,8 @@ def detected_by(manager_class):
def wrapper(cls):
assert issubclass(cls, chirp_common.CloneModeRadio)
cls._DETECTED_MODEL = True
if manager_class.DETECTED_MODELS is None:
manager_class.DETECTED_MODELS = []
manager_class.DETECTED_MODELS.append(cls)
cls._DETECTED_BY = manager_class
manager_class.detect_model(cls)
return cls
return wrapper

View File

@ -394,7 +394,7 @@ class LeixenVV898Radio(chirp_common.CloneModeRadio):
_addr, _data = recv(radio)
ident = _data[8:14]
LOG.debug('Got ident from radio:\n%s' % util.hexprint(ident))
for rclass in [cls] + cls.detected_models():
for rclass in cls.detected_models():
if ident == rclass._model_ident:
return rclass
# Reset the radio if we didn't find a match

View File

@ -852,9 +852,8 @@ class TDH8(chirp_common.CloneModeRadio):
@classmethod
def detect_from_serial(cls, pipe):
ident = _do_ident(pipe, cls._idents[0])
for rclass in [cls] + cls.detected_models():
if (rclass.ident_mode == ident and
rclass.MODEL.startswith(cls.MODEL)):
for rclass in cls.detected_models():
if rclass.ident_mode == ident:
return rclass
LOG.error('No model match found for %r', ident)
raise errors.RadioError('Unsupported model')

View File

@ -2071,7 +2071,7 @@ class UVK5Radio(UVK5RadioBase):
@classmethod
def detect_from_serial(cls, pipe):
firmware = _sayhello(pipe)
for rclass in [UVK5Radio] + cls.detected_models():
for rclass in cls.detected_models():
if rclass.k5_approve_firmware(firmware):
return rclass
raise errors.RadioError('Firmware %r not supported' % firmware)

View File

@ -242,6 +242,10 @@ def port_sort_key(port):
return key
def model_value(rclass):
return ('%s %s' % (rclass.MODEL, rclass.VARIANT)).strip()
# Make this global so it sticks for a session
CUSTOM_PORTS = []
@ -310,7 +314,7 @@ class ChirpCloneDialog(wx.Dialog):
if (not issubclass(rclass, chirp_common.CloneModeRadio) and
not issubclass(rclass, chirp_common.LiveRadio)):
continue
if (getattr(rclass, '_DETECTED_MODEL', False) and
if (getattr(rclass, '_DETECTED_BY', None) and
not allow_detected_models):
continue
self._vendors[rclass.VENDOR].append(rclass)
@ -445,9 +449,7 @@ class ChirpCloneDialog(wx.Dialog):
self.FindWindowById(wx.ID_OK).Disable()
def _persist_choices(self):
CONF.set('last_vendor', self._vendor.GetStringSelection(), 'state')
CONF.set('last_model', self._model.GetStringSelection(), 'state')
CONF.set('last_port', self.get_selected_port(), 'state')
raise NotImplementedError()
def _selected_port(self, event):
if self._port.GetStringSelection() == CUSTOM:
@ -464,7 +466,7 @@ class ChirpCloneDialog(wx.Dialog):
self._persist_choices()
def _select_vendor(self, vendor):
models = [('%s %s' % (x.MODEL, x.VARIANT)).strip()
models = [model_value(x)
for x in self._vendors[vendor]]
self._model.Set(models)
self._model.SetSelection(0)
@ -617,6 +619,12 @@ class ChirpDownloadDialog(ChirpCloneDialog):
self._clone_thread = CloneThread(self._radio, self, 'sync_in')
self._clone_thread.start()
def _persist_choices(self):
# On download, persist the selections from the actual UI boxes
CONF.set('last_vendor', self._vendor.GetStringSelection(), 'state')
CONF.set('last_model', self._model.GetStringSelection(), 'state')
CONF.set('last_port', self.get_selected_port(), 'state')
class ChirpUploadDialog(ChirpCloneDialog):
def __init__(self, radio, *a, **k):
@ -624,9 +632,7 @@ class ChirpUploadDialog(ChirpCloneDialog):
**k)
self._radio = radio
self.select_vendor_model(
self._radio.VENDOR,
('%s %s' % (self._radio.MODEL, self._radio.VARIANT)).strip())
self.select_vendor_model(self._radio.VENDOR, model_value(self._radio))
self.disable_model_select()
if isinstance(self._radio, chirp_common.LiveRadio):
@ -691,3 +697,13 @@ class ChirpUploadDialog(ChirpCloneDialog):
self._clone_thread = CloneThread(self._radio, self, 'sync_out')
self._clone_thread.start()
def _persist_choices(self):
# On upload, we may have a detected-only subclass, which won't be
# selectable normally. If so, use the detected_by instead of the
# actual driver
parent = getattr(self._radio, '_DETECTED_BY', None)
model = model_value(parent or self._radio)
CONF.set('last_vendor', self._vendor.GetStringSelection(), 'state')
CONF.set('last_model', model, 'state')
CONF.set('last_port', self.get_selected_port(), 'state')

View File

@ -4,6 +4,7 @@ import json
import os
import shutil
import tempfile
from unittest import mock
import yaml
@ -143,3 +144,50 @@ class TestAliasMap(base.BaseTest):
# before we add it to ensure there are no duplicates
self.assertNotIn(model['model'], directory_models[vendor])
directory_models[vendor].add(model['model'])
class TestDetectedBy(base.BaseTest):
@mock.patch('chirp.directory.DRV_TO_RADIO', new={})
@mock.patch('chirp.directory.RADIO_TO_DRV', new={})
def test_detected_isolation(self):
@directory.register
class BaseRadio(chirp_common.CloneModeRadio):
VENDOR = 'CHIRP'
MODEL = 'Base'
@directory.register
class SubRadio1(BaseRadio):
MODEL = 'Sub1'
@directory.register
@directory.detected_by(SubRadio1)
class SubRadio2(SubRadio1):
MODEL = 'Sub2'
# BaseRadio should not think it detects the subs
self.assertEqual([BaseRadio], BaseRadio.detected_models())
# Sub1 detects both itself and Sub2
self.assertEqual([SubRadio1, SubRadio2], SubRadio1.detected_models())
# If include_self=False, Sub1 should not include itself
self.assertEqual([SubRadio2],
SubRadio1.detected_models(include_self=False))
# Sub2 does not also detect Sub1
self.assertEqual([SubRadio2], SubRadio2.detected_models())
@mock.patch('chirp.directory.DRV_TO_RADIO', new={})
@mock.patch('chirp.directory.RADIO_TO_DRV', new={})
def test_detected_include_self(self):
class BaseRadio(chirp_common.CloneModeRadio):
VENDOR = 'CHIRP'
MODEL = 'Base'
@directory.register
@directory.detected_by(BaseRadio)
class SubRadio(BaseRadio):
MODEL = 'Sub'
# BaseRadio should not include itself since it is not registered
self.assertEqual([SubRadio], BaseRadio.detected_models())