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

[Sean Burford]

On Wed, Mar 13, 2013 at 11:52 AM, Dan Smith <dsmith at danplanet.com> wrote:

> 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?


Yes, however a North American Marine band plan and a North American Amateur
band plan would both be under an ITU region 2 band plan, and you might want
to use both.


> 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...
>

You're right.  I'll try it with objects and see how it looks.  I expect
that will make it easier to calculate and claim the repeater input blocks
too (with opposite offsets).


> > 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.
>

I've filed bug #683


> > +  "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))
>

Sounds good to me.


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

I'll try what you suggest.


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

No worries.

-- 
Sean Burford <sburford at google.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://intrepid.danplanet.com/pipermail/chirp_devel/attachments/20130313/884f01c9/attachment-0001.html