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

[Jaroslav Skarvada]

----- Original Message -----
> 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.
>
So what do you propose here? Could we add e.g. some counter which will abort
get_clone_resp() after some number of characters are received (e.g. twice
the clone frame)?

> 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?
>
OK, NP with that. I can rewrite the code.

Jaroslav