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

[Dan Smith]

Sorry I meant to reply to this earlier.

> Regarding the clone status check, I spent some time on digging into the USB sniffs
> and it seems there is a bug in the Chirp code. After adding:
> 
> --- a/chirp/drivers/icf.py
> +++ b/chirp/drivers/icf.py
> @@ -183,6 +183,7 @@ def send_clone_frame(radio, cmd, data, raw=False, checksum=False):
>         pass
> 
>     radio.pipe.write(frame)
> +    get_clone_resp(radio.pipe)
> 
>     return frame
> 
> I.e. after calling already defined, but never used function, it started to work
> even with the clone status check enabled. In the original Icom SW sniffs
> the clone status check response is clearly visible. It seems the icf driver doesn't
> read the radio responses during the clone process and when it tries to read the last
> response code, there is already buffer full of previous data which it cannot
> correctly process.
> 
> But there is a problem with this patch, it breaks the tests so it waits forever on
> the pipe:
> 
> $ ./run_tests.py -d 'Icom_IC-E90'
>     Icom IC-E90        Detect      PASSED: All tests
>     Icom IC-E90        Settings    PASSED: All tests
> ^CTraceback (most recent call last):
>  File "./run_tests.py", line 1300, in <module>
>    failed = tr.run_one(options.driver)
>  File "./run_tests.py", line 1232, in run_one
>    "%s.img" % drv_name))
>  File "./run_tests.py", line 1210, in run_rclass_image
>    failed += self.run_rclass_image(dev.__class__, image, dst=dev)
>  File "./run_tests.py", line 1213, in run_rclass_image
>    return self._run_one(rclass, image, dst=dst)
>  File "./run_tests.py", line 1167, in _run_one
>    failures = tc.run()
>  File "./run_tests.py", line 984, in run
>    self._run(self.SerialNone())
>  File "./run_tests.py", line 947, in _run
>    radio.sync_in()
>  File "../chirp/drivers/icx90.py", line 598, in sync_in
>    icf.IcomCloneModeRadio.sync_in(self)
>  File "../chirp/drivers/icf.py", line 627, in sync_in
>    self._mmap = clone_from_radio(self)
>  File "../chirp/drivers/icf.py", line 289, in clone_from_radio
>    return _clone_from_radio(radio)
>  File "../chirp/drivers/icf.py", line 237, in _clone_from_radio
>    md = get_model_data(radio)
>  File "../chirp/drivers/icf.py", line 140, in get_model_data
>    send_clone_frame(radio, 0xe0, mdata, raw=True)
>  File "../chirp/drivers/icf.py", line 186, in send_clone_frame
>    get_clone_resp(radio.pipe)
>  File "../chirp/drivers/icf.py", line 162, in get_clone_resp
>    while not exit_criteria(resp, length):
>  File "../chirp/drivers/icf.py", line 157, in exit_criteria
>    return buf.endswith("\xfd")
> KeyboardInterrupt
> 
> So this needs to be addressed somehow

Yeah, so I'm surprised we're not doing this already and it definitely seems like the right thing to do is make the change you've proposed above. The reason it fails is one of the clone edge condition tests provides an endless stream of garbage, as if you selected the wrong serial port to do a download from, in an attempt to make sure the driver is robust enough to notice and fail. Since get_clone_resp() is pretty dumb, it ... fails.

On the one hand I'd really like to get that change in there, but it's also been (from looking at the history) not checking the response to each frame for the entire lifetime of that code (so ~10 years). I would hate to introduce it now and cause some instability for Icom users, which have always enjoyed pretty stable support. I'm not sure what to do here.

However, to get you out of jail here, maybe we could do something to just affect your model and then potentially enable it per-driver on other models as we're able to test them. Some some radio.MUNCH_CLONE_RESP=True flag that we check in the icf code to know whether or not to do that? What do you think?

--Dan