[chirp_devel] Review: IC-T7H support

[Eric Allen]

Is there a more efficient way I can submit patches in the future, a lá
github pull requests? I'm really struggling to construct the patches you
desire using mercurial. I'm used to git's extensive history rewriting
facilities. Can I submit a git diff instead of an hg export?

I've attached a patch with your suggested edits.

On Wed, Jul 18, 2012 at 10:15 AM, Dan Smith <dsmith at danplanet.com> wrote:

> > Alright, updated patch with passing tests. Turns out it does support
> > fractional frequencies, but using a shifted BCD. I also brought in the
> > band edges to what this thing supports when you do the software switch
> > to wide RX.
>
> Cool, thanks!
>
> > I checked the tuning steps. This Icom doesn't store the step, as
> > verified by diffing memory dumps and trying it out on the radio.
>
> Hmm, strange, but okay :)
>
> > +        mem.freq = int(_mem.freq) * 100000
> > +        mem.freq += _mem.lastfreq * 10000
> > +        mem.freq += int((_mem.fraction / 2.0) * 1000)
> > +
> > +        mem.offset = int(_mem.offset) * 10000
> > +        mem.rtone = chirp_common.TONES[_mem.rtone-1]
> > +        mem.ctone = chirp_common.TONES[_mem.ctone-1]
>
> I really hate to pick these nits, but since I'm going to make you
> resubmit one more time, I will. PEP8 says that operators have one space
> around them. You do it correctly in the first block, but there are
> several instances elsewhere that aren't. I'm certainly not perfect here,
> but if you wouldn't mind tweaking them, I'd appreciate it.
>
> chirp/ict7h.py:43:6: E221 multiple spaces before operator
> chirp/ict7h.py:44:8: E222 multiple spaces after operator
> chirp/ict7h.py:94:50: E225 missing whitespace around operator
>
> > diff -r 4a3dbf10d64b -r 3deda8456d93 tests/images/Icom_IC-T7H.img
> > Binary file tests/images/Icom_IC-T7H.img has changed
>
> This won't apply properly and I'll commit your image separately. So, if
> you could pull this null hunk out, that'd be good.
>
> > diff -r 4a3dbf10d64b -r 3deda8456d93 tests/run_tests
> > --- a/tests/run_tests Fri Jul 13 16:43:41 2012 -0700
> > +++ b/tests/run_tests Tue Jul 17 08:58:28 2012 -0700
> > @@ -1,4 +1,4 @@
> > -#!/usr/bin/python
> > +#!/usr/bin/env python
> >  #
> >  # Copyright 2011 Dan Smith <dsmith at danplanet.com>
> >  #
>
> This shouldn't be in the patch.
>
> Other than those three minor things, I think this is ready to apply.
> Thanks a lot for doing this. It's always nice to have someone just drop
> in and write a driver :)
>
> >      send_clone_frame(radio.pipe, CMD_CLONE_END, radio.get_endframe(),
> raw=True)
> > +    time.sleep(3.5)
>
> I'm not sure if you intended to include this patch as well, but I don't
> want to apply this as-is. If you want to try to try to poll for the end
> result for a certain amount of time, then that'd be fine. It doesn't
> work, for example, on my 2820, presumably because the radio spends more
> than 3.5 seconds validating the (much larger) image.
>
> Lets get the T7H driver applied first and then we can experiment with
> the clone result thing.
>
> Thanks a bunch Eric!
>
> --
> Dan Smith
> www.danplanet.com
> KK7DS
>
>
>
>
> _______________________________________________
> chirp_devel mailing list
> chirp_devel at intrepid.danplanet.com
> http://intrepid.danplanet.com/mailman/listinfo/chirp_devel
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://intrepid.danplanet.com/pipermail/chirp_devel/attachments/20120718/1103c8db/attachment-0001.html 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: t7h-v4.patch
Type: application/octet-stream
Size: 4181 bytes
Desc: not available
Url : http://intrepid.danplanet.com/pipermail/chirp_devel/attachments/20120718/1103c8db/attachment-0001.obj