I've attached a patch with your suggested edits.
On Wed, Jul 18, 2012 at 10:15 AM, Dan Smith
<dsmith@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@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!
_______________________________________________
chirp_devel mailing list
chirp_devel@intrepid.danplanet.com
http://intrepid.danplanet.com/mailman/listinfo/chirp_devel