[chirp_devel] Add new radio Icom IC-E90/T90/T90A

[Dan Smith]

> The radio is a bit tricky - it has multiple special channels and
> TV channels (well analog TV is a bit off-topic today, but
> I wanted to support all features of this radio). I added
> TV channels as a subdevice, because I have no better idea
> how to do it.

Look at the ft817 (and related) radios. They have oodles of special channels, and I'd definitely prefer that it be implemented that way. I see you've got some special channels in your patch here already -- why not make the TV channels the same?

> Also I had to modify the icf driver a bit, because it seems
> the clone status doesn't work with this radio. Maybe I am
> just missing something here, but I wasn't able to persuade
> my radio to behave this way. The proposed icf driver change
> should be harmless for other ICOM radios, because it's adding
> new option and the memory size calculation should be backward
> compatible.

This makes me nervous. Every icom radio under the sun uses the same exact protocol module. The only difference is that the newer ones have an extended format to be able to address larger memories. It doesn't make sense to me that this one would be different.

> One test from the testsuite is failing:
> 
> ./run_tests.py -d 'Icom_IC-E90'
>     Icom IC-E90        Detect      PASSED: All tests
>     Icom IC-E90        Settings    PASSED: All tests
>     Icom IC-E90        Clone       PASSED: All tests
>     Icom IC-E90        Edges       FAILED: Field `name' is ` ', expected `' 

This means you're not rstrip()ing the memory name. If you do that, I think you'll see this pass.

>  def clone_to_radio(radio):
> @@ -400,7 +404,7 @@ def convert_data_line(line):
>  
>      line = line.strip()
>  
> -    if len(line) == 38:
> +    if len(line) % 8 == 6:

Why this change?

> diff --git a/chirp/drivers/icx90.py b/chirp/drivers/icx90.py
> new file mode 100644
> index 0000000..c76496a
> --- /dev/null
> +++ b/chirp/drivers/icx90.py
> @@ -0,0 +1,826 @@
> +# -*- coding: utf-8 -*-
> +# Copyright 2018-2019 Jaroslav Škarvada <jskarvad at redhat.com>
> +# Based on various code from the CHIRP
> +
> +# This program is free software: you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation, either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +ICX90_MEM_FORMAT = """

Please put the import lines above everything else like all the other files.

> +class ICT90_Alias(chirp_common.Alias):
> +    VENDOR = "Icom"
> +    MODEL = "IC-T90"
> +
> +class ICT90A_Alias(chirp_common.Alias):
> +    VENDOR = "Icom"
> +    MODEL = "IC-T90A"
> +
> +
> + at directory.register
> +class ICx90Radio(icf.IcomCloneModeRadio):
> +    """Icom IC-E/T90"""
> +    VENDOR = "Icom"
> +    MODEL = "IC-E90"
> +
> +    ALIASES = [ICT90_Alias, ICT90A_Alias]

I can see adding one alias for T90 instead of E90, but adding T90 and T90A is not necessary, IMHO.

> +    def __init__(self, pipe):
> +        icf.IcomCloneModeRadio.__init__(self, pipe)
> +        self.init_special()

Seems like this could just be done in get_features(), but maybe I'm missing somewhere else that you're using self.special...

Also, this needs a commit message and a bug reference in that in order to be applied. You should be okay sending a git formatted patch, but it needs to be importable with those piece of info for me to digest.

https://chirp.danplanet.com/projects/chirp/wiki/DevelopersProcess#Patch-Guidelines

--Dan