Re: [chirp_devel] Patch for #681: Add support for band plans

Wow, this is a lot of work, thanks for doing it!

You can select multiple plans because, for example, you might want both an IARU plan and a country plan.

So, from reading part of the code, I got the impression that perhaps the plans are/should be hierarchical. Meaning, that the NA plan is a subset of IARU 2. Is that the case? I ask because I'm wondering if it makes more sense to let folks choose IARU 2 or NA (which would inherit from IARU, and perhaps override some bits) instead of this mix-and-match behavior. I don't think it would make much sense to have NA and IARU 1 enabled at the same time, for example, but maybe I'm wrong. Of course, that has me thinking that we should be utilizing inheritance...

The format in bandplan_au.py is the format I prefer. The format in bandplan_na.py is carried over from chirp_common.py and should be replaced by the new format once we agree on how it should look.

It's very close to what I pulled out of thin air earlier, and overall I think I'm fine with it. I do wonder if we should be using objects instead of just primitives, but I'm not sure.

While testing this I noticed that there are multiple instances of the config class, with different and overlapping lifetimes. This means that sometimes configuration changes are applied and sometimes they are not. That is a separate issue.

Hmm? Config is a singleton, and the various proxy objects should just be views into it. I've not seen any such issues, but if you find them, we should definitely get them resolved.

• "bands": {
• (28.000e6, 29.700e6): {

I don't like the specification of these as floats. Can we use ints, perhaps with easy shortcut helpers such as:

(k(28000), k(28700))

and

(m(146), m(148))

?

+def _walk_bandplan(freq, bandplan, defaults):

• # Override defaults with values from this level.
• for name in defaults:

•    defaults[name] = bandplan.get(name, defaults[name])
  

• # Go more specific if the frequency fits within an available

sublevel.

• for key, vals in bandplan.items():

•    if key in defaults:
  

•        pass
  

•    elif isinstance(key, tuple):
  

•        if key[0] > key[1]:
  

•            raise Exception("Invalid range %s - %s" % key)
  

•        if int(freq) >= key[0] and int(freq) < key[1]:
  

•            _walk_bandplan(freq, vals, defaults)
  

•            break
  

•    else:
  

•        raise Exception("Invalid key %s" % key)
  

This just screams at me for the use of an object structure instead of primitives. Is there a reason you decided against it? (other than me firing off a half-baked concept earlier, of course :P)

•  <menu action="bandplan" name="bandplan">
  

•    <menuitem action="north_america"/>
  

•    <menuitem action="australia"/>
  

•  </menu>
  

This is fine, of course, and we don't have many here right now, but we can automate the generation of the submenu so that adding a new bandplan is a little less involved.

•    # Migrate old "automatic repeater offset" setting to
  

Thanks for this!

Otherwise, I think it looks fine. Again, thanks a lot for taking on this non-trivial thing :)