tdh8, tdh3: Further revise the AM handling

These radios (annoyingly) treat frequencies in airband as AM, which
really isn't how chirp was designed to work. Before, we were making
them immutable, but that brings all sorts of problems with it. Instead,
coerce the values on get/set, and return a warning from
validate_memory() so the user is warned.

In reality, we should probably be doing that more places where we have
immutable fields, where the immutable-ness depends on the content of
the memory (i.e the frequency) than the location.

Fixes: #11441

chirp/drivers/tdh8.py

@@ -1002,7 +1002,7 @@ class TDH8(chirp_common.CloneModeRadio):
     MODEL = "TD-H8"
     ident_mode = b'P31183\xff\xff'
     BAUD_RATE = 38400
-    MODES = ["FM", "NFM"]
+    MODES = ["FM", "NFM", "AM"]
     _memsize = 0x1eef
     _ranges_main = [(0x0000, 0x1eef)]
     _idents = [TD_H8]
@@ -1261,14 +1261,8 @@ def get_memory(self, number):
 
         if in_range(mem.freq, self._rxbands):
             mem.duplex = 'off'
-            mem.immutable.append('duplex')
         if in_range(mem.freq, [AIRBAND]):
-            # NOTE: AM is not in valid_modes because you can't arbitrarily
-            # enable it on this radio. However, we can expose it as immutable
-            # which will display properly in the UI and not allow the user
-            # to change those channels to FM.
             mem.mode = 'AM'
-            mem.immutable.append('mode')
 
         return mem
 
@@ -2467,6 +2461,19 @@ def _set_fm_preset(self, settings):
                 LOG.debug(element.get_name())
                 raise
 
+    def validate_memory(self, mem: chirp_common.Memory):
+        msgs = []
+        if in_range(mem.freq, [AIRBAND]) and not mem.mode == 'AM':
+            msgs.append(chirp_common.ValidationWarning(
+                _('Frequency in this range requires AM mode')))
+        if not in_range(mem.freq, [AIRBAND]) and mem.mode == 'AM':
+            msgs.append(chirp_common.ValidationWarning(
+                _('Frequency in this range must not be AM mode')))
+        if not in_range(mem.freq, self._txbands) and mem.duplex != 'off':
+            msgs.append(chirp_common.ValidationWarning(
+                _('Frequency outside TX bands must be duplex=off')))
+        return msgs + super().validate_memory(mem)
+
 
 @directory.register
 @directory.detected_by(TDH8)
@@ -2479,13 +2486,6 @@ class TDH8_HAM(TDH8):
                 (400000000, 419999000), (451000001, 521000000)]
     _txbands = [(144000000, 149000000), (420000000, 451000000)]
 
-    def check_set_memory_immutable_policy(self, existing, new):
-        # Immutable duplex handling is done in set_memory, so no need
-        # to ever obsess over it here.
-        if 'duplex' in existing.immutable:
-            existing.immutable.remove('duplex')
-        super().check_set_memory_immutable_policy(existing, new)
-
 
 @directory.register
 @directory.detected_by(TDH8)
@@ -2579,22 +2579,3 @@ class RT730(TDH8):
 
     def process_mmap(self):
         self._memobj = bitwise.parse(MEM_FORMAT_RT730, self._mmap)
-
-    def check_set_memory_immutable_policy(self, existing, new):
-        if (AIRBAND[0] <= new.freq <= AIRBAND[1] and
-                new.mode == 'AM'):
-            # This is valid, so mark mode as immutable so it doesn't get
-            # blocked, and let the radio override it during set.
-            new.immutable.append('mode')
-            existing.immutable = []
-        elif existing.mode == 'AM' and new.mode in self.MODES:
-            # If we're going from a forced-AM channel to some valid one,
-            # clear immutable so we allow the change.
-            try:
-                existing.immutable.remove('mode')
-            except ValueError:
-                pass
-        if (in_range(new.freq, self._txbands) and
-                'duplex' in existing.immutable):
-            existing.immutable.remove('duplex')
-        super().check_set_memory_immutable_policy(existing, new)

tests/test_copy_all.py

@@ -1,5 +1,6 @@
 import os
 
+from chirp import chirp_common
 from chirp.drivers import generic_csv
 from chirp import import_logic
 from tests import base
@@ -47,6 +48,16 @@ def test_copy(self):
             except import_logic.DestNotCompatible:
                 continue
 
+            warn, err = chirp_common.split_validation_msgs(
+                self.radio.validate_memory(dst_mem))
             self.radio.set_memory(dst_mem)
+
+            if warn:
+                # If the radio warned about something, we can assume it's
+                # about duplex (i.e. tx inhibit) or mode (i.e. AM only on
+                # airband)
+                ignore = ['duplex', 'mode']
+            else:
+                ignore = None
             ret_mem = self.radio.get_memory(dst_number)
-            self.assertEqualMem(dst_mem, ret_mem)
+            self.assertEqualMem(dst_mem, ret_mem, ignore=ignore)

tests/unit/test_chirp_common.py

@@ -755,6 +755,12 @@ def test_extra_comment(self):
 class TestOverrideRules(base.BaseTest):
     # You should not need to add your radio to this list. If you think you do,
     # please ask permission first.
+    # Immutable fields should really only be used for cases where the value
+    # is not changeable based on the *location* of the memory. If something
+    # is forced to be a value based on the *content* of the memory (i.e. AM
+    # for frequencies in airband), coerce them on set/get, and return a
+    # ValidationWarning in validate_memory() so the user is told that the
+    # values are being forced.
     IMMUTABLE_WHITELIST = [
         # Uncomment me when the time comes
         'Baofeng_GT-5R',
@@ -768,8 +774,6 @@ class TestOverrideRules(base.BaseTest):
         'Baofeng_UV-17ProGPS',
         'Baofeng_5RM',
         'Baofeng_K5-Plus',
-        'Radtel_RT-730',
-        'TIDRADIO_TD-H8-HAM',
     ]
 
     def _test_radio_override_immutable_policy(self, rclass):