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

[Jaroslav Skarvada]

----- Original Message -----
> 
> 
> ----- Original Message -----
> > 
> > 
> > ----- 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.
> > 
> Attached patch, it's not perfect, but works for me.
> 
> Another thing, the final clone result check could be probably rewritten.
> In the IcfFrame class the cmd code 0xE6 for final clone result check is
> already defined (and I can see it in the USB sniffs), but it's never used
> in the code. Instead the current code blindly reads the garbage and
> checks whether the final byte is 0x00. I think it should check for the cmd
> 0xE6 and read the following result code.
> 
> thanks & regards
> 
> Jaroslav
> 

Hi Dan,

are you OK with the proposed patch? Or do you have different patch?

thanks & regards

Jaroslav