[chirp_devel] #8987 Adding settings to IC-7300 live mode

[Dan Smith]

> @@ -80,6 +81,31 @@
>  char name[16];             // 52-60 Name of station
>  """
>  
> +MEM_IC7300_FORMAT = """
> +bbcd number[2];            // 1,2
> +u8   spl:4,                // 3 split and select memory settings

<snip>

>  MEM_IC910_FORMAT = """
>  u8   bank;                 // 1 bank number
>  bbcd number[2];            // 2,3
> @@ -114,31 +140,6 @@
>  char name[9];
>  """
>  
> -MEM_IC7300_FORMAT = """
> -bbcd number[2];            // 1,2
> -u8   spl:4,                // 3 split and select memory settings

Why are you moving this? This just generates churn in the diff and makes it hard to tell what all you're changing. You're moving the driver definition and the IC7300MemFrame as well. I'm assuming this was just accidental, so can you move them back so the diff is clean please?

> @@ -561,7 +562,8 @@
>              mem.duplex = self._rf.valid_duplexes[memobj.duplex]
>  
>          if self._rf.can_odd_split and memobj.spl:
> -            mem.duplex = "split"
> +            if hasattr(memobj, "duplex"):
> +                mem.duplex = "split"

This looks like a fix that is separate from the settings feature, right? If so, please split this into a separate patch.

> +    def get_settings(self):
> +        bas = RadioSettingGroup("basic", "BASIC")
> +        grps = RadioSettings(bas)
> +
> +        global scrsav
> +        scrsav = ["Off", "15 Mins", "30 Mins", "60 Mins"]

Please do not do this. This will create a non-reentrant dependency between two instances of this driver, such that they can't be running at the same time without stepping all over each other. If you really need to do this, set it on self, which will be local to the instance:

  self._scrsav = ["Off", "15 Mins", "30 Mins", "60 Mins"]

Then you can access that from set_settings().

Same goes for the other globals you're setting.

> +            if typ == 1:        # Boolean
> +                vlu = ord(resp)
> +                rs = RadioSetting(name, disp,
> +                                  RadioSettingValueBoolean(vlu))
> +            elif typ == 2 or typ == 3:       # Integer
> +                if typ == 2:        # 1 byte
> +                    vlu = int("%02x" % ord(resp))
> +                else:      # 2 bytes, 0-255 as 0-100 %, encodes 138 as 01 38
> +                    vlu = int("%02x" % ord(resp[0])) * 100
> +                    vlu += int("%02x" % ord(resp[1]))
> +                    vlu = round(vlu / 2.55)
> +                rs = RadioSetting(name, disp,
> +                                  RadioSettingValueInteger(v1, v2, vlu))
> +            elif typ == 4:      # List
> +                vlu = int("%02x" % ord(resp))
> +                rs = RadioSetting(name, disp,
> +                                  RadioSettingValueList(v1, v1[vlu]))
> +            elif typ == 5:      # String
> +                stx = resp
> +                rs = RadioSetting(name, disp,
> +                                  RadioSettingValueString(0, lnx, stx))
> +            elif typ == 6:      # Offset Freq

Can you just define these constants and use them here? Seems like it'll be a lot cleaner:

 TYPE_BOOLEAN = 1
 TYPE_INTEGER = 2
  ..
 if typ == TYPE_BOOLEAN:
     ...
 elif typ == TYPE_INTEGER:
     ...

Otherwise looks okay, thanks!

--Dan