[chirp_devel] Review: IC-T7H support
Hey guys, I've written up a configuration for the IC-T7H that supports every feature I'm aware of in common between CHIRP and ICOM. I'm having a bit of trouble with uploads to the radio, though: the serial port seems to be buffering so much that if I don't sleep a bit between writing the data and deciding whether we're done it hangs half way through. Has anybody else encountered similar issues using a Prolific PL2303-based cable on OS X?
Hi Eric,
diff -r 4a3dbf10d64b chirp/ict7h.py --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/chirp/ict7h.py Mon Jul 16 21:40:56 2012 -0700 @@ -0,0 +1,109 @@ +# Copyright 2010 Dan Smith dsmith@danplanet.com
This should be your copyright, and a current date.
Otherwise it looks pretty good. Ideally, I'd like the patch in "hg export" format so that it's properly attributed to you in the tree.
Does it pass all the tests (cd tests && ./run_tests)? Please attach a T7H .img file for me to add to the tests/images directory when I apply.
Thanks!
Updated patch with copyright changes and hg export format. I've also attached an image file from my T7H.
Tests are failing like crazy, I think because of fractional frequencies, which AFAIK the T7H doesn't support. I'll take a look later this week.
On Tue, Jul 17, 2012 at 7:38 AM, Dan Smith dsmith@danplanet.com wrote:
Hi Eric,
diff -r 4a3dbf10d64b chirp/ict7h.py --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/chirp/ict7h.py Mon Jul 16 21:40:56 2012 -0700 @@ -0,0 +1,109 @@ +# Copyright 2010 Dan Smith dsmith@danplanet.com
This should be your copyright, and a current date.
Otherwise it looks pretty good. Ideally, I'd like the patch in "hg export" format so that it's properly attributed to you in the tree.
Does it pass all the tests (cd tests && ./run_tests)? Please attach a T7H .img file for me to add to the tests/images directory when I apply.
Thanks!
-- Dan Smith www.danplanet.com KK7DS
chirp_devel mailing list chirp_devel@intrepid.danplanet.com http://intrepid.danplanet.com/mailman/listinfo/chirp_devel
Updated patch with copyright changes and hg export format. I've also attached an image file from my T7H.
Cool, thanks.
Tests are failing like crazy, I think because of fractional frequencies, which AFAIK the T7H doesn't support. I'll take a look later this week.
Yep, see below.
+STEPS = [5.0, 10.0, 12.5, 15.0, 20.0, 25.0, 30.0, 50.0]
<snip>
- def get_features(self):
rf = chirp_common.RadioFeatures()rf.memory_bounds = (0, 59)rf.valid_modes = list(MODES)rf.valid_tmodes = list(TMODES)rf.valid_duplexes = list(DUPLEX)rf.valid_bands = [( 30000000, 823995000),(849000000, 868995000),(894000000, 1309995000)]
One of the tests is trying to set the claimed top-end frequency of 1309MHz and getting back 309MHz. If the radio really supports this, then something isn't right in the driver to store/fetch that value.
rf.valid_skips = ["", "S"]
You define STEPS above, but don't tell CHIRP what they are. If you do that, then the tests won't try to set frequencies that require 6.25 and 12.5 kHz steps. This will cause all of the tests (other than the one mentioned above) to pass.
rf.has_tuning_step = False
Note that all Icoms I've ever seen store the tuning step in the memory channels. I don't know why they do this (other than to support tuning off of a memory) but they do. It's fine if that one doesn't, or if you don't want to support it, but all of the other Icom drivers do, just FYI.
Thanks!
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.
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.
On Tue, Jul 17, 2012 at 9:32 AM, Dan Smith dsmith@danplanet.com wrote:
Updated patch with copyright changes and hg export format. I've also attached an image file from my T7H.
Cool, thanks.
Tests are failing like crazy, I think because of fractional frequencies, which AFAIK the T7H doesn't support. I'll take a look later this week.
Yep, see below.
+STEPS = [5.0, 10.0, 12.5, 15.0, 20.0, 25.0, 30.0, 50.0]
<snip>
- def get_features(self):
rf = chirp_common.RadioFeatures()rf.memory_bounds = (0, 59)rf.valid_modes = list(MODES)rf.valid_tmodes = list(TMODES)rf.valid_duplexes = list(DUPLEX)rf.valid_bands = [( 30000000, 823995000),(849000000, 868995000),(894000000, 1309995000)]One of the tests is trying to set the claimed top-end frequency of 1309MHz and getting back 309MHz. If the radio really supports this, then something isn't right in the driver to store/fetch that value.
rf.valid_skips = ["", "S"]You define STEPS above, but don't tell CHIRP what they are. If you do that, then the tests won't try to set frequencies that require 6.25 and 12.5 kHz steps. This will cause all of the tests (other than the one mentioned above) to pass.
rf.has_tuning_step = FalseNote that all Icoms I've ever seen store the tuning step in the memory channels. I don't know why they do this (other than to support tuning off of a memory) but they do. It's fine if that one doesn't, or if you don't want to support it, but all of the other Icom drivers do, just FYI.
Thanks!
-- Dan Smith www.danplanet.com KK7DS
chirp_devel mailing list chirp_devel@intrepid.danplanet.com http://intrepid.danplanet.com/mailman/listinfo/chirp_devel
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) * 100000mem.freq += _mem.lastfreq * 10000mem.freq += int((_mem.fraction / 2.0) * 1000)mem.offset = int(_mem.offset) * 10000mem.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!
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@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) * 100000mem.freq += _mem.lastfreq * 10000mem.freq += int((_mem.fraction / 2.0) * 1000)mem.offset = int(_mem.offset) * 10000mem.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!
-- Dan Smith www.danplanet.com KK7DS
chirp_devel mailing list chirp_devel@intrepid.danplanet.com http://intrepid.danplanet.com/mailman/listinfo/chirp_devel
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?
You can, but then I have to commit it and manually specify your username.
If you enable the mq extension in mercurial, you can "qimport" a revision, change it, "qrefresh" it, and then "qfinish" it back into a changeset. However, it's much easier if you never actually commit it and just use "qnew" and "qrefresh" to edit it. It's quilt-like functionality integrated into mercurial, which is really ideal for preparing patches for upstream. It's so handy for that, I loathe using git for similar operations.
I've attached a patch with your suggested edits.
Applied, thanks very much for your work and patience!
Ahh, extensions. Thanks!
On Wed, Jul 18, 2012 at 7:50 PM, Dan Smith dsmith@danplanet.com wrote:
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?
You can, but then I have to commit it and manually specify your username.
If you enable the mq extension in mercurial, you can "qimport" a revision, change it, "qrefresh" it, and then "qfinish" it back into a changeset. However, it's much easier if you never actually commit it and just use "qnew" and "qrefresh" to edit it. It's quilt-like functionality integrated into mercurial, which is really ideal for preparing patches for upstream. It's so handy for that, I loathe using git for similar operations.
I've attached a patch with your suggested edits.
Applied, thanks very much for your work and patience!
-- Dan Smith www.danplanet.com KK7DS
chirp_devel mailing list chirp_devel@intrepid.danplanet.com http://intrepid.danplanet.com/mailman/listinfo/chirp_devel
Glad to hear it! Thanks for making an awesome piece of software. CHIRP has totally changed how I use my HT. Happy to be of use contributing new memory maps.
On Wed, Jul 18, 2012 at 7:50 PM, Dan Smith dsmith@danplanet.com wrote:
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?
You can, but then I have to commit it and manually specify your username.
If you enable the mq extension in mercurial, you can "qimport" a revision, change it, "qrefresh" it, and then "qfinish" it back into a changeset. However, it's much easier if you never actually commit it and just use "qnew" and "qrefresh" to edit it. It's quilt-like functionality integrated into mercurial, which is really ideal for preparing patches for upstream. It's so handy for that, I loathe using git for similar operations.
I've attached a patch with your suggested edits.
Applied, thanks very much for your work and patience!
-- Dan Smith www.danplanet.com KK7DS
chirp_devel mailing list chirp_devel@intrepid.danplanet.com http://intrepid.danplanet.com/mailman/listinfo/chirp_devel
Hey Dan, I was just about to recommend CHIRP to a friend with a T7H, but I realized release 0.2.3 doesn't appear to include T7H support, as evidenced by the test report and lack of a ict7h.py file in the tarball. Is it being held back due to limited field testing at this point?
On Wed, Jul 18, 2012 at 7:50 PM, Dan Smith dsmith@danplanet.com wrote:
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?
You can, but then I have to commit it and manually specify your username.
If you enable the mq extension in mercurial, you can "qimport" a revision, change it, "qrefresh" it, and then "qfinish" it back into a changeset. However, it's much easier if you never actually commit it and just use "qnew" and "qrefresh" to edit it. It's quilt-like functionality integrated into mercurial, which is really ideal for preparing patches for upstream. It's so handy for that, I loathe using git for similar operations.
I've attached a patch with your suggested edits.
Applied, thanks very much for your work and patience!
-- Dan Smith www.danplanet.com KK7DS
chirp_devel mailing list chirp_devel@intrepid.danplanet.com http://intrepid.danplanet.com/mailman/listinfo/chirp_devel
I was just about to recommend CHIRP to a friend with a T7H, but I realized release 0.2.3 doesn't appear to include T7H support, as evidenced by the test report and lack of a ict7h.py file in the tarball. Is it being held back due to limited field testing at this point?
No, it's because the stable release is only bug fixes against the original (0.2.0). The T7H driver is in the daily builds, which is what will become 0.3.0. We need to snap 0.3.0 real soon now, but a crowded summer schedule has me putting it off a bit.
Thanks!
Ahhh, gotcha. Thanks for the details!
On Thu, Sep 13, 2012 at 9:31 AM, Dan Smith dsmith@danplanet.com wrote:
I was just about to recommend CHIRP to a friend with a T7H, but I realized release 0.2.3 doesn't appear to include T7H support, as evidenced by the test report and lack of a ict7h.py file in the tarball. Is it being held back due to limited field testing at this point?
No, it's because the stable release is only bug fixes against the original (0.2.0). The T7H driver is in the daily builds, which is what will become 0.3.0. We need to snap 0.3.0 real soon now, but a crowded summer schedule has me putting it off a bit.
Thanks!
-- Dan Smith www.danplanet.com KK7DS
chirp_devel mailing list chirp_devel@intrepid.danplanet.com http://intrepid.danplanet.com/mailman/listinfo/chirp_devel
participants (2)
-
Dan Smith -
Eric Allen