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

[Dan Smith]

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 :)

-- 
Dan Smith
www.danplanet.com
KK7DS
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
Url : http://intrepid.danplanet.com/pipermail/chirp_devel/attachments/20130312/f5137079/attachment-0001.bin