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?<div>
<br></div><div>I've attached a patch with your suggested edits.<br><br><div class="gmail_quote">On Wed, Jul 18, 2012 at 10:15 AM, Dan Smith <span dir="ltr"><<a href="mailto:dsmith@danplanet.com" target="_blank">dsmith@danplanet.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">> Alright, updated patch with passing tests. Turns out it does support<br>
> fractional frequencies, but using a shifted BCD. I also brought in the<br>
> band edges to what this thing supports when you do the software switch<br>
> to wide RX.<br>
<br>
</div>Cool, thanks!<br>
<div class="im"><br>
> I checked the tuning steps. This Icom doesn't store the step, as<br>
> verified by diffing memory dumps and trying it out on the radio.<br>
<br>
</div>Hmm, strange, but okay :)<br>
<br>
> + mem.freq = int(_mem.freq) * 100000<br>
> + mem.freq += _mem.lastfreq * 10000<br>
> + mem.freq += int((_mem.fraction / 2.0) * 1000)<br>
> +<br>
> + mem.offset = int(_mem.offset) * 10000<br>
> + mem.rtone = chirp_common.TONES[_mem.rtone-1]<br>
> + mem.ctone = chirp_common.TONES[_mem.ctone-1]<br>
<br>
I really hate to pick these nits, but since I'm going to make you<br>
resubmit one more time, I will. PEP8 says that operators have one space<br>
around them. You do it correctly in the first block, but there are<br>
several instances elsewhere that aren't. I'm certainly not perfect here,<br>
but if you wouldn't mind tweaking them, I'd appreciate it.<br>
<br>
chirp/ict7h.py:43:6: E221 multiple spaces before operator<br>
chirp/ict7h.py:44:8: E222 multiple spaces after operator<br>
chirp/ict7h.py:94:50: E225 missing whitespace around operator<br>
<br>
> diff -r 4a3dbf10d64b -r 3deda8456d93 tests/images/Icom_IC-T7H.img<br>
> Binary file tests/images/Icom_IC-T7H.img has changed<br>
<br>
This won't apply properly and I'll commit your image separately. So, if<br>
you could pull this null hunk out, that'd be good.<br>
<br>
> diff -r 4a3dbf10d64b -r 3deda8456d93 tests/run_tests<br>
> --- a/tests/run_tests Fri Jul 13 16:43:41 2012 -0700<br>
> +++ b/tests/run_tests Tue Jul 17 08:58:28 2012 -0700<br>
> @@ -1,4 +1,4 @@<br>
> -#!/usr/bin/python<br>
> +#!/usr/bin/env python<br>
> #<br>
> # Copyright 2011 Dan Smith <<a href="mailto:dsmith@danplanet.com">dsmith@danplanet.com</a>><br>
> #<br>
<br>
This shouldn't be in the patch.<br>
<br>
Other than those three minor things, I think this is ready to apply.<br>
Thanks a lot for doing this. It's always nice to have someone just drop<br>
in and write a driver :)<br>
<br>
> send_clone_frame(radio.pipe, CMD_CLONE_END, radio.get_endframe(), raw=True)<br>
> + time.sleep(3.5)<br>
<br>
I'm not sure if you intended to include this patch as well, but I don't<br>
want to apply this as-is. If you want to try to try to poll for the end<br>
result for a certain amount of time, then that'd be fine. It doesn't<br>
work, for example, on my 2820, presumably because the radio spends more<br>
than 3.5 seconds validating the (much larger) image.<br>
<br>
Lets get the T7H driver applied first and then we can experiment with<br>
the clone result thing.<br>
<br>
Thanks a bunch Eric!<br>
<div class="HOEnZb"><div class="h5"><br>
--<br>
Dan Smith<br>
<a href="http://www.danplanet.com" target="_blank">www.danplanet.com</a><br>
KK7DS<br>
<br>
<br>
<br>
</div></div><br>_______________________________________________<br>
chirp_devel mailing list<br>
<a href="mailto:chirp_devel@intrepid.danplanet.com">chirp_devel@intrepid.danplanet.com</a><br>
<a href="http://intrepid.danplanet.com/mailman/listinfo/chirp_devel" target="_blank">http://intrepid.danplanet.com/mailman/listinfo/chirp_devel</a><br>
<br></blockquote></div><br></div>