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

[Jaroslav Skarvada]

----- 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: get_clone_resp.patch
Type: text/x-patch
Size: 2239 bytes
Desc: not available
Url : http://intrepid.danplanet.com/pipermail/chirp_devel/attachments/20191007/6dda4cc3/attachment-0001.bin