[chirp_devel] Update to IC-2730A Driver

Dan Smith
Wed Jun 26 08:28:59 PDT 2019


Hi Rick,

> Attached is the update to the IC-2730A driver that incorporates all the Settings tabs and fixes the C0/C1 special channels lookup error.
> The referenced issue is #6711, which duplicated 2745, 4085 and 4147.

Thanks for this, but your patch removes the entire driver and re-adds it, I'm assuming because you've converted the line endings or something like that. For example:

diff -r bc8b50e0e189 -r c31a96e2f689 chirp/drivers/ic2730.py
--- a/chirp/drivers/ic2730.py	Thu May 30 10:06:52 2019 -0400
+++ b/chirp/drivers/ic2730.py	Wed Jun 26 07:30:39 2019 -0700
@@ -1,294 +1,1284 @@
-# Copyright 2018 Rhett Robinson <rrhett at gmail.com>
+# Copyright 2018 Rhett Robinson <rrhett at gmail.com>

The whole file is removed, and then re-added with windows-style newlines.

If I fix the file (using dos2unix) I can see just the delta you're trying to add. When I do, I get a large number of style failures. I see that the 2730 isn't included in the manifest, so I've added that and pushed the four changes it needed to pass. I've included that list at the bottom of this mail for reference. Can you please rebase on the current tip, fix your style issues and resubmit without changing the encoding of the file? If you need help with this, please speak up.

On the actual content of the patch, most of it looks pretty good modulo the style issues, and others that the checker won't catch. There are a number of non-functional changes to existing lines -- please try to avoid that if possible.

> +    def get_prompts(cls):
> +        rp = chirp_common.RadioPrompts()
> +        rp.info = \
> +            ('NOTE 1: Click the Special Channels tab on the main screen to '
> +             'view the C0 and C1 frequencies as channels 1000 and 1001.\n'
> +             'NOTE 2: CI-V is the control inteface protocol used to both'
> +             ' clone and control the rig. The factory-standard CI-V address'
> +             ' for the IC-2730a is hex 90.\n'
> +             'NOTE 3: If there is no audio from the B-side, check that the '
> +             'programming interface cable is removed from speaker jack 2.\n'
> +             'NOTE 4: Enabling Weather Alert will cause an interupt every '
> +             '5 seconds when the band is recieving non-weather signals.\n'
> +             'Note 5: Bank names can only be editted in the radio menu.\n'
> +             'Note 6: CLONE ERROR on upload is usaually the result of an '
> +             'invalid frequency.\n'
> +            )


IMHO, this "general errata" information is not really appropriate to encode inside chirp. I think you should remove it.

> +        elif mem.freq % 8333.3333 == 0:
> +            frequency_flags |= 0x10
> +            frequency_multiplier = 8333.3333


This will yield a float, which isn't appropriate to set in the raw memory. Elsewhere we just divide the frequency by 8333 when needed -- is there some reason this isn't sufficient for this case?

> +    def get_settings(self):

This method, while impressive, is super massive. Can you break it up into sections?

Other than style, it seems to pass tests, which is good. I assume you'll have hand-tested the setting stuff a lot.

Thanks!

--Dan

./chirp/drivers/ic2730.py:22:1: E402 module level import not at top of file
./chirp/drivers/ic2730.py:23:1: E402 module level import not at top of file
./chirp/drivers/ic2730.py:24:1: E402 module level import not at top of file
./chirp/drivers/ic2730.py:25:1: E402 module level import not at top of file
./chirp/drivers/ic2730.py:28:42: E231 missing whitespace after ','
./chirp/drivers/ic2730.py:30:1: E402 module level import not at top of file
./chirp/drivers/ic2730.py:227:8: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:235:9: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:236:9: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:237:9: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:238:9: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:240:1: E302 expected 2 blank lines, found 1
./chirp/drivers/ic2730.py:250:1: E302 expected 2 blank lines, found 1
./chirp/drivers/ic2730.py:254:1: E302 expected 2 blank lines, found 1
./chirp/drivers/ic2730.py:260:1: E302 expected 2 blank lines, found 1
./chirp/drivers/ic2730.py:271:21: E261 at least two spaces before inline comment
./chirp/drivers/ic2730.py:275:17: E127 continuation line over-indented for visual indent
./chirp/drivers/ic2730.py:298:13: E124 closing bracket does not match visual indentation
./chirp/drivers/ic2730.py:357:17: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:423:30: E701 multiple statements on one line (colon)
./chirp/drivers/ic2730.py:424:30: E701 multiple statements on one line (colon)
./chirp/drivers/ic2730.py:511:13: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:531:21: E265 block comment should start with '# '
./chirp/drivers/ic2730.py:550:40: E701 multiple statements on one line (colon)
./chirp/drivers/ic2730.py:604:28: E211 whitespace before '('
./chirp/drivers/ic2730.py:621:28: E211 whitespace before '('
./chirp/drivers/ic2730.py:637:28: E261 at least two spaces before inline comment
./chirp/drivers/ic2730.py:646:25: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:716:25: E701 multiple statements on one line (colon)
./chirp/drivers/ic2730.py:724:25: E701 multiple statements on one line (colon)
./chirp/drivers/ic2730.py:728:17: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:739:17: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:744:17: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:748:23: E211 whitespace before '('
./chirp/drivers/ic2730.py:758:21: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:782:13: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:804:17: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:826:17: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:830:21: E127 continuation line over-indented for visual indent
./chirp/drivers/ic2730.py:833:21: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:839:13: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:845:21: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:850:21: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:855:17: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:861:21: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:873:21: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:877:17: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:880:17: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:886:17: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:892:17: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:897:17: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:902:17: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:908:17: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:920:13: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:921:13: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:925:13: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:932:17: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:948:17: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:953:17: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:980:17: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:1006:17: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:1011:17: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:1016:17: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:1021:17: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:1026:17: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:1031:17: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:1036:17: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:1053:13: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:1074:9: E265 block comment should start with '# '
./chirp/drivers/ic2730.py:1100:9: E265 block comment should start with '# '
./chirp/drivers/ic2730.py:1111:21: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:1118:27: E211 whitespace before '('
./chirp/drivers/ic2730.py:1123:21: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:1129:41: E701 multiple statements on one line (colon)
./chirp/drivers/ic2730.py:1130:41: E701 multiple statements on one line (colon)
./chirp/drivers/ic2730.py:1135:41: E701 multiple statements on one line (colon)
./chirp/drivers/ic2730.py:1136:41: E701 multiple statements on one line (colon)
./chirp/drivers/ic2730.py:1145:21: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:1147:25: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:1152:17: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:1154:25: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:1162:25: E127 continuation line over-indented for visual indent
./chirp/drivers/ic2730.py:1190:44: E701 multiple statements on one line (colon)
./chirp/drivers/ic2730.py:1191:28: E701 multiple statements on one line (colon)
./chirp/drivers/ic2730.py:1195:21: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:1201:21: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:1211:27: E211 whitespace before '('
./chirp/drivers/ic2730.py:1216:17: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:1238:21: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:1240:17: E128 continuation line under-indented for visual indent
./chirp/drivers/ic2730.py:1247:5: E303 too many blank lines (2)
./chirp/drivers/ic2730.py:1284:1: W391 blank line at end of file


More information about the chirp_devel mailing list