1) Who moved that stuff? Did you do that?😕 Anyway, I will revert to the current repository release for the settings patch. 2) Oops; forgot that I had to make that change at 561. I will create a bug issue and submit based on said release. 3) I  thought using a config file import was the only way to share "globals" between driver instances.  I will use the self declaration and re-submit the settings patch with your "TYPE_" constants suggestion; once I can get my hands on a loaner radio again.

On 4/27/2021 3:00 PM, Dan Smith via chirp_devel wrote:

...

@@ -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 _______________________________________________ chirp_devel mailing list chirp_devel@intrepid.danplanet.com http://intrepid.danplanet.com/mailman/listinfo/chirp_devel Developer docs: http://chirp.danplanet.com/projects/chirp/wiki/Developers