[chirp_devel] Issue #7409 patch
Rick DeWitt AA0RD
Sat Dec 14 07:46:14 PST 2019
Okay.
I will release a patch that addresses just the match_model:False fix for
issue #7409.
The auto-baud change is to address a Mac-related problem where the USB
interface pre-pended one of the "?" responses to the correct ID response
string ("?ID023;"), causing the exact match logic to fail. I do not have
access to a Mac, so I can only guess at the best fix.
I will consolidate the other changes into a new issue/patch.
FYI: I'm changing the radio class names so that the clone-mode class is
NOT a duplicate of those already declared in kenwood_live.py.
On 12/13/2019 11:12 AM, Dan Smith via chirp_devel wrote:
>> # HG changeset patch
>> # User Rick DeWitt <aa0rd at yahoo.com>
>> # Date 1576163136 28800
>> # Thu Dec 12 07:05:36 2019 -0800
>> # Node ID 62a26da6eaddf9d563f9a2e2753aec6674a04ed9
>> # Parent b5589aa94c1e6a424d0f713017f68d39caa29be9
>> [ts590] Resolving issue #7409
>> Add match_model to satisfy File > New csv generation. Improve auto-baud. Unique class ID.
> This is too much change, especially for a regression fix.
>
>> diff -r b5589aa94c1e -r 62a26da6eadd chirp/drivers/ts590.py
>> --- a/chirp/drivers/ts590.py Thu Dec 05 21:14:35 2019 +1100
>> +++ b/chirp/drivers/ts590.py Thu Dec 12 07:05:36 2019 -0800
>> @@ -19,7 +19,6 @@
>> import logging
>> import re
>> import math
>> -import threading
> This is gardening, should be consolidated to just a patch of cleanups.
>
>> from chirp import chirp_common, directory, memmap
>> from chirp import bitwise, errors, util
>> from chirp.settings import RadioSettingGroup, RadioSetting, \
>> @@ -121,12 +120,10 @@
>> u8 ex099;
>> } exset[2];
>>
>> - char mdl_name[9]; // appended model name, first 9 chars
>> -
> This changes the image format. Maybe I missed this before, but is this not part of the actual radio image?
>
>> """
>>
>> +MEMSIZE = 0x0bf8 # Img file size withoud metadata
> This is probably not needed (see below). Also "without" is misspelled.
>
>> STIMEOUT = 2
>> -LOCK = threading.Lock()
> Chuck this in the gardening patch.
>
>> BAUD = 0 # Initial baud rate
>> MEMSEL = 0 # Default Menu A
>> BEEPVOL = 4 # Default beep volume
>> @@ -163,7 +160,6 @@
>> # If rsplen = 0 then do not read after write
>>
>> start = time.time()
>> - # LOCK.acquire()
> ...and this.
>
>> stx = cmd # preserve cmd for response check
>> stx = stx + exts + ";" # append arguments
>> ser.write(stx)
>> @@ -176,7 +172,6 @@
>> result = ser.read(rsplen)
>> LOG.debug("RADIO->PC [%s]" % result)
>> result = result[:-1] # remove terminator
>> - # LOCK.release()
> ...and this.
>
>> return result.strip()
>>
>>
>> @@ -193,15 +188,16 @@
>> radio.pipe.timeout = STIMEOUT
>>
>> for bd in bauds:
>> + junk = radio.pipe.read(16)
>> radio.pipe.baudrate = bd
>> BAUD = bd
>> radio.pipe.write(";")
>> radio.pipe.write(";")
>> - resp = radio.pipe.read(4)
>> + resp = radio.pipe.read(16)
>> radio.pipe.write("ID;")
>> - resp = radio.pipe.read(6)
>> -
>> - if resp == radio.ID: # Good comms
>> + resp = radio.pipe.read(16)
>> + LOG.debug("Radio sent ID: %s for baud %i" % (resp, BAUD))
>> + if resp.find(radio.ID) >= 0: # Good comms
>> resp = command(radio.pipe, "AI0", 0, W8L)
>> return
>> elif resp in RADIO_IDS.keys():
> This change to the detection routine should be in its own patch, with its own justification so we can discuss it. In the py3 branch I was messing with my D700 and the changes you made to this code made it nearly impossible to use one of the regular live models in a single session because of all the delimiter stuff mixed in. I made a couple small tweaks there, which I should probably bring over to the main branch and then we can discuss if the above is really what we want.
>
>> @@ -423,8 +419,7 @@
>> nc += 1
>> status.cur = nc
>> radio.status_fn(status)
>> - setts += radio.MODEL.ljust(9)
>> - # Now set the inidial menu selection back
>> + # Now set the initial menu selection back
> Gardening.
>
>> result0 = command(radio.pipe, "MF", 0, W8L, "%1i" % MEMSEL)
>> # And the original Beep Volume
>> result0 = command(radio.pipe, "EX0050000%2i" % BEEPVOL, 0, W8L)
>> @@ -562,10 +557,9 @@
>> return
>>
>>
>> -# Bug #7409
>> -# @directory.register
>> -class TS590Radio(chirp_common.CloneModeRadio):
>> - """Kenwood TS-590"""
>> + at directory.register
>> +class TS590_CRadio(chirp_common.CloneModeRadio):
>> + """ Kenwood TS-590 using CAT """
>> VENDOR = "Kenwood"
>> MODEL = "TS-590SG_CloneMode"
>> ID = "ID023;"
>> @@ -1614,7 +1608,6 @@
>>
>> def set_settings(self, settings):
>> _settings = self._memobj.settings
>> - _mem = self._memobj
> Gardening.
>
>> for element in settings:
>> if not isinstance(element, RadioSetting):
>> self.set_settings(element)
>> @@ -1647,9 +1640,18 @@
>> LOG.debug(element.get_name())
>> raise
>>
>> + @classmethod
>> + def match_model(cls, fdata, fyle):
>> + """ Included to prevent 'File > New' error """
>> + # Test the file data size
>> + if len(fdata) == MEMSIZE:
>> + return True
>> + else:
>> + return False
> Since your driver is very new, I think you can and should just return False here, and rely on the metadata-based model selection. Wouldn't that solve the problem? We shouldn't be adding entirely new drivers that depend on match_model guessing. Returning False here will cause the directory to never match a file against this unless the metadata specifically says it's for this model.
>
>> +
> This adds whitespace, which probably isn't what you want.
>
>>
>> @directory.register
>> -class TS590SRadio(TS590Radio):
>> +class TS590S_CRadio(TS590_CRadio):
> Why are you changing this? This will cause us to have some old images that reference radio class names that are no longer valid. The directory currently looks at vendor/model for its work, but radio class is in there for the future. Unless there's an important reason to do this, I'd rather we not.
>
> So in summary, if you could please strip out just the changes required to fix the matching bug into one patch (which includes the change to register the driver again), that would be good. Then a patch to do the gardening you want. If you want to hold off on the baud changes I'll bring my changes over to the default branch and then you can see what, if anything, you want to tweak on top of that.
>
> Okay? Thanks for figuring out and working on the problem.
>
> --Dan
> _______________________________________________
> chirp_devel mailing list
> chirp_devel at intrepid.danplanet.com
> http://intrepid.danplanet.com/mailman/listinfo/chirp_devel
> Developer docs: http://chirp.danplanet.com/projects/chirp/wiki/Developers
--
Rick DeWitt
AA0RD
Sequim, Washington, USA 98382
(360) 681-3494
More information about the chirp_devel
mailing list