[chirp_devel] Kenwood TM-D710/G driver

newer
[chirp_devel] Windows Dev Debug.log

older
[chirp_devel] Build test results:...

Rick DeWitt AA0RD

14 Apr 2020 14 Apr '20

6:23 p.m.

Woo-Hoo! Finally; the Kenwood TM-D710 /D710G combined driver patch file is attached, with the manifest and test images. Should be Py3 syntax compliant using bytes() and HAS_FUTURE global. It took forever due to having to test D710 model by email > Load Module....

-- Rick DeWitt AA0RD Sequim, Washington, USA 98382 (360) 681-3494

Attachments:

tmd10_new.patch (text/plain — 76.2 KB)
cpep8.manifest (text/plain — 3.8 KB)
Kenwood_TM-D710G_CloneMode.img (application/octet-stream — 95.4 KB)
Kenwood_TM-D710_CloneMode.img (application/octet-stream — 39.1 KB)

Show replies by date

Dan Smith

16 Apr 16 Apr

11:38 p.m.

...

Woo-Hoo! Finally; the Kenwood TM-D710 /D710G combined driver patch file is attached, with the manifest and test images.

I've been wondering if you were still working on this -- glad to hear you are.

...

Should be Py3 syntax compliant using bytes() and HAS_FUTURE global. It took forever due to having to test D710 model by email > Load Module....

Note that it fails the settings test on python3. I haven't looked into why, but it seems like a bytes/str issue:

...

def _pswd_vfy(setting, obj, atrb):
    """ Verify password is 1-6 chars, numbers 1-5 """
    str1 = str(setting.value).strip()      # initial string
    str2 = filter(lambda c: c in '12345', str1)    # valid chars
    if str1 != str2:
        # Two lines due to python 73 char limit
        sx = "Bad characters in Password"

...

      raise errors.RadioError(sx)

E chirp.errors.RadioError: Bad characters in Password

If it's easier I can just try to figure this out later.

...

+def _dbg_dump(stb, kn=32, cptn="DBG", ax=False):
""" Debugging tool to print up to kn hex values of byte array stb """

# ax: flag to append ascii

global DMPKNT

sx = ""

stz = stb.decode("latin-1") # bytes to string

kx = len(stz)

if kx > kn:
   kx = kn
for ix in range(kx):
   sx += "%02x " % ord(stz[ix])
if ax: # append as ascii
   sx += " ["
   for ix in range(kx):
       if ord(stz[ix]) > 31 and ord(stz[ix]) < 127:
           sx += "%s" % stz[ix]
       else:
           sx += "."
   sx += "]"
if DMPKNT > 0:
   if DMP & 1:
       LOG.warning("%s: %s" % (cptn, sx))
   if DMP & 2:
       LOG.debug("%s: %s" % (cptn, sx))
   if DMP & 4:
       DMPKNT -= 1
return

First off, you might want to look at util.hexprint(), which will do the hex dumping for you. Second, this really isn't the way to do this -- using a global. If you think it's important to log this much, just do it at debug level. If debug logging is turned on, you'll see it, and if it's not, you won't. That's the whole point of the level-based logger. Just doing this will cause you to see it in the terminal:

$ CHIRP_DEBUG=y ./chirpw

or set it in your environment on Windows.

...

+def _connect_radio(radio):
"""Determine baud rate and verify radio on-line"""

global BAUD # Declaration allows modification

bauds = [57600, 38400, 19200, 9600]

xid = radio.MODEL.split("_")[0]

if BAUD > 0:
   bauds.insert(0, BAUD)    # Make the detected one first

Why can't you use kenwood_live.get_id() for this?

...

+def _read_mem(radio):
""" Load the memory map """

global DMPKNT # to allow mod

status = chirp_common.Status()

status.cur = 0

val = 0

for mx in range(0, radio._num_blocks):
   val += radio._num_packets[mx]
status.max = val

status.msg = "Reading %i packets" % val

radio.status_fn(status)

DMPKNT = 25

You're setting this immediately, even though it's global. So you're just using this to communicate with the dump function through a global right? That makes it non-reentrant, which isn't okay. Please, just log debug info at debug level and use that to control what we do or don't see. Can you explain what you're trying to do with this signaling so maybe I can help suggest something more conventional?

...

data = bytes()
cmc = "0M PROGRAM" + TERM
resp0 = _command(radio.pipe, cmc, 3, W8S)
if radio.SHORT == "G": # D710G

   radio.pipe.baudrate = 57600     # PROG mode is always 57.6

   LOG.debug("Switching to 57600 baud download.")

   junk = radio.pipe.read(1)       # trailing byte

   for blkn in range(0, radio._num_blocks):

       for bkx in range(0, radio._num_packets[blkn]):

           cmc = "R" + _make_address(radio.SHORT,

                                     radio._block_addr[blkn], bkx, 0, 0)

           resp0 = _command(radio.pipe, cmc,

                            radio._packet_size[blkn], W8S)

           if len(resp0) < radio._packet_size[blkn]:

               junk = _command(radio.pipe, "E", 0, W8S)

               sx = "Block %x %x read error, %i bytes." % \

                   (blkn, bkx, len(resp0))

               raise errors.RadioError(sx)

This error message (and several others) will be pretty cryptic for the user. Can you just log.error() all the details and then raise a message like "Error reading block from radio" ?

...

```
               return ""
```

This return never gets run, because it's after a raise.

...

           if blkn == 0 and bkx == 0:   # 1st packet of 1st block

               mht = resp0[5:9]   # Magic Header Thingy after cmd echo

```
               data += mht[0:1]
```

               data += chr(255) + chr(255) + chr(255)

Why are you adding things to the data we're returning, other than exactly what we got from the radio? Every Kenwood I've implemented clone mode for has had a very straightforward "give me 0x40 bytes of memory at address 0x1234" protocol, which seems to be the case here. I'm not sure where all this complexity here comes from. The resulting chirp image should be basically the same as what the MCP software stores (aside from some header info), assuming it's like all the other Kenwood OEM software I've used. Have you compared your image to that to make sure you're faithfully representing the radio's memory layout?

...

```
               data += resp0[9:]
```
```
           else:
```

               data += resp0[5:]       # skip cmd echo

           _update_status(radio, status)        # UI Update

```
   # Exit Prog mode, no TERM
```

   resp = _command(radio.pipe, "E", 0, W8S)

else: # D710

   junk = radio.pipe.read(16)       # flushit

```
   for bkx in range(0, 0x09c):
```

       if bkx != 0x07f:            # Skip block 7f !!??

           cmc = "R" + chr(bkx) + chr(0) + chr(0)

           resp0 = _command(radio.pipe, cmc, 260, W8S)

           junk = _command(radio.pipe, ACK, 1, W8S)

```
           if len(resp0) < 260:
```

               junk = _command(radio.pipe, "E", 2, W8S)

               sx = "Block %x read error, %i bytes." % \

```
                   (bkx, len(resp0))
```

               raise errors.RadioError(sx)

```
               return ""
```

           if bkx == 0:   # 1st packet of 1st block

               mht = resp0[4:7]   # [57 00 00 00] 03 4b 01 ff ff ...

               data = resp0[5:6]  # Store as 4b 4b 01 ff ff ff ...

```
               data += resp0[5:]
```

Why are you doing this? Why not just:

data = resp0[5:]

? Aren't you repeating the [5] byte in what you're adding into data?

...

```
           else:
```

               data += resp0[4:]       # skip cmd echo

           _update_status(radio, status)        # UI Update

   DMPKNT = 10  # Reset dump count to do these

   cmc = "R" + chr(0x0fe) + chr(0x0f0) + chr(0x010)

   resp0 = _command(radio.pipe, cmc, 0x014, W8S)

```
   data += resp0[4:]
```

   junk = _command(radio.pipe, ACK, 1, W8S)

```
   _update_status(radio, status)
```

   cmc = "R" + chr(0x0ff) + chr(0) + chr(0x090)

   resp0 = _command(radio.pipe, cmc, 0x094, W8S)

```
   data += resp0[4:]
```

   junk = _command(radio.pipe, ACK, 1, W8S)

```
   _update_status(radio, status)
```
```
   # Exit Prog mode, no TERM
```

   resp = _command(radio.pipe, "E", 2, W8S)     # Rtns 06 0d

radio.pipe.baudrate = BAUD

This function is basically two completely separate functions with a giant if..else in the middle. I would rather just have two functions, one for the 710 and one for the 710G. You can put it on the actual class for each if it makes it easier. Even better if you can reduce them down further so only the difference in behavior is conditional.

...

diff -r 37379cfd09d9 -r 7b2152434e12 tools/cpep8.manifest --- a/tools/cpep8.manifest Mon Mar 02 20:28:45 2020 -0600 +++ b/tools/cpep8.manifest Tue Apr 14 11:15:22 2020 -0700 @@ -82,6 +82,7 @@ ./chirp/drivers/thuv1f.py ./chirp/drivers/tk8102.py ./chirp/drivers/tk8180.py +./chirp/drivers/tmvd710.py

Note you typo'd this, but I corrected it locally and it passed the style check.

I'm really concerned about the clone routines getting a faithful copy of the radio's memory, as I've mentioned before. The complexity in those routines being somewhat suspect based on what I know of other models, as well as the thing I mentioned above about returning data that isn't actually in the radio (or the duplicated byte) are things I really want to make sure we get right. Other stuff in the actual driver that won't affect the stability of the file format we can work on later. However, if we're not pulling out the memory exactly, then we'll have images which could corrupt the radio if we push them in later after changing/fixing the clone routines.

--Dan

1710

Age (days ago)

1712

Last active (days ago)

List overview

Download

1 comments

2 participants

tags (0)

participants (2)

Dan Smith
Rick DeWitt AA0RD