Re: [chirp_devel] [PATCH] [thd74] Add a Kenwood d74 driver

Hi Angus,

I did a fairly deep dive on this radio a little while back. While I haven't had the cycles to organize my notes and turn them into a complete driver since, I checked them against this one, and found several issues.

Despite the name, the TH-D74 really isn't a variant of the TH-D72. They have quite different read/write protocols, memory formats, and settings. The D72 driver suggests it supports multiple baud rates for MCP programming mode, while the D74 only supports 9600 AFAICT. The D74 supports D-Star, TX in the 1.25m band, and has a much bigger RX range. From what I can see, they really only share a few field enumerations, and all the important methods in your subclass are overridden anyways. In light of this, it's probably a better idea to either factor out a shared base class for the D72 and D74 that contains their enumerations, or just subclass chirp_common.CloneModeRadio and reference the enumerations directly out of the D72 driver. Beyond all of that, there's some specific issues in this implementation, comments inlined.

+from . import thd72

Should be: from chirp.drivers import thd72

+# memory channel

+# 0 1 2 3 4 5 6 7 8 9 a b c d e f +# [freq ] ? mode tmode/duplex rtone ctone dtcs cross_mode [offset] ?

+# frequency is 32-bit unsigned little-endian Hz

This is the D72's memory format, not the D74's.

+DEFAULT_PROG_VFO = (

• (136000000, 174000000),
• (410000000, 470000000),
• (118000000, 136000000),
• (136000000, 174000000),
• (320000000, 400000000),
• (400000000, 524000000),

+)

This is specific to the D72 memory entries, which have a flag that links them into the appropriate programmable VFO range, something that doesn't appear to apply to the D74. See bug #1611 for further details.

+# Some of the blocks written by MCP have a length set of less than 0x00/256 +BLOCK_SIZES = {

• 0x0003: 0x00B4,
• 0x0007: 0x0068,

+}

Why obfuscate these values by writing them in hex? Block size is also 8-bits not 16-bits, explained further down.

+#seekto 0x04000; +// TODO: deal with the 16-byte trailers of every block +struct {

• struct {

•  ul32 freq;
  

•  ul32 offset;
  

•  u8   tuning_step:4,
  

•       unk:4;
  

•  u8   mode:4,
  

•       unk1:4;
  

•  u8   tone_mode:4,
  

•       duplex:4;
  

•  u8   rtone;
  

•  u8   ctone;
  

•  u8   dtcs;
  

•  u8   cross_mode:4
  

•       digital_squelch:4;
  

•  char urcall[8];
  

•  char rpt1[8];
  

•  char rpt2[8];
  

•  u8   digital_squelch_code;
  

• } mem[6];
• u8 pad[16];

+} memory[1167]; // TODO: correct number of memories

Typo, this should be 167, for a total of 1002 channels (the two extra entries are formatted like an uninitialized block but aren't used/accessible by the radio).

+#seekto 0x14700;

+struct {

• char name[16];

+} wx_name[10];

+#seekto 0x144d0; +struct {

• char name[16];

+} call_name[6];

My notes suggest these locations are inverted.

+class SProxy(object):

• def __init__(self, delegate):

•    self.delegate = delegate
  

• def read(self, len):

•    r = self.delegate.read(len)
  

•    LOG.debug("READ\n" + hex(r))
  

•    return r
  

• def write(self, data):

•    LOG.debug("WRITE\n" + hex(data))
  

•    return self.delegate.write(str(data))
  

• @property
• def timeout(self):

•    return self.delegate.timeout
  

• @timeout.setter
• def timeout(self, timeout):

•    self.delegate.timeout = timeout
  
  

Debug stuff that doesn't belong in the driver.

+@directory.register

+class THD74Radio(thd72.THD72Radio):

• MODEL = "TH-D74 (clone mode)"
• #MODEL = "TH-D74"
• _memsize = 500480
• # I think baud rate might be ignored by USB-Serial stack of the D74's
• # on-board FTDI chip, but it doesn't seem to hurt.
• BAUD_RATE = 115200

MCP-74 always uses a baud rate of 9600 in the captures I did.

• #def __init__(self, pipe):
• # pipe = SProxy(pipe)
• # super(THD74Radio, self).__init__(pipe)

More debug cruft.

• def get_features(self):

•    rf = super(THD74Radio, self).get_features()
  

•    rf.has_tuning_step = True
  

•    return rf
  
  

This returns the wrong rf.memory_bounds and rf.valid_bands for the TH-D74.

• def write_block(self, block, map, count=256):

•    #print("Write block ", block )
  

•    c = struct.pack(">cHH", b"W", block, count%256)
  

•    base = block * 256
  

•    data = map[base:base + count]
  

•    # It's crucial that these are written together. Otherwise the
  

radio

•    # will fail to ACK under some conditions.
  

•    c_d = ''.join(chr(b) for b in c) + data
  

•    self.pipe.write(c_d)
  

•    ack = self.pipe.read(1)
  

•    if len(ack) == 0:
  

•        print("read timed out block %d - trying again" % block )
  

•        time.sleep(0.5)
  

•        ack = self.pipe.read(1)
  

•    if ack != chr(0x06):
  

•        print("Block %d write failed %d" % ( block, ord(ack)))
  

•    return ack == chr(0x06)
  

• def _unlock(self):

•    """Voodoo sequence of operations to get the radio to accept our
  

programming."""

•    h = self.read_block(0, 6)
  

•    unlock =
  

("\x57\x00\x00\x00\x30\xff\x01\xff\x00\x00\xff\xff\xff\xff\xff\x01" +

"\x00\x00\x00\x03\x01\x00\x00\x00\x00\x02\x00\x30\x30\x30\x00\xff" +

"\xff\xff\xff\xff\xff\xff\x00\xff\xff\xff\xff\xff\xff\xff\xff\xff" +

•        "\xff\xff\xff\xff\xff")
  

•    self.pipe.write(unlock)
  

•    ack = self.pipe.read(1)
  

•    if ack != chr(0x06):
  

•        raise errors.RadioError("Expected ack but got {}
  

({})".format(ord(ack), type(ack)))

•    c = struct.pack(">cHH", b"W", 0, 0x38C8)
  

•    self.pipe.write(''.join(chr(b) for b in c))
  

•    # magic unlock sequence
  

•    unlock = [0xFF] * 8 + [0] * 160 + [0xFF] * 32
  

•    unlock = "".join([chr(x) for x in unlock])
  

•    self.pipe.write(unlock)
  
  

This is where everything starts to go off the rails. The structure format is wrong, it's should be ">cHBB", representing the character command, the block number, the offset from the start of the block to start reading/writing from, and the size to read/write. The "magic unlock sequence" is actually just a segmented write of block 0, which has a small write-protected hole in the middle of it. It's probably not a good idea to use these hardcoded values, since it's not clear what block 0 actually represents, and could even vary between different radios (e.g. between the A/E models, different firmware revisions, etc). Far better to just write back the original data pulled from each radio.

•    # For some reason MCP-D74 skips this block. If we don't, we'll
  

get a NACK

•    # on the next one. There is also a more than 500 ms delay for the
  

ACK.

•    if 1279 in blocks:
  

•        blocks.remove(1279)
  
  

MCP-74 skips over many more blocks than this.

•        if self.status_fn:
  

•            s = chirp_common.Status()
  

•            s.msg = "Cloning to radio"
  

•            s.max = total
  

•            s.cur = count
  

•            self.status_fn(s)
  

•    lock = ("\x57\x00\x00\x00\x06\x02\x01\xff\x00\x00\xff")
  

•    self.pipe.write(lock)
  
  

Similar magic value issue here. This is a binary representation of the a write command of the first 6 bytes of block 0, which seems to prepare the radio for the final "F" command to exit out of MCP programming mode.

• def set_channel_name(self, number, name):

•    name = name[:16] + '\x00' * 16
  

•    if number < 999:
  

•        self._memobj.channel_name[number].name = name[:16]
  

•        self.add_dirty_block(self._memobj.channel_name[number])
  

•    elif number >= 1020 and number < 1030:
  

•        number -= 1020
  

•        self._memobj.wx_name[number].name = name[:16]
  

•        self.add_dirty_block(self._memobj.wx_name[number])
  
  

I know the D72 driver also does this, but the name is the only value you can set for for WX, even the frequency is immutable. Shouldn't this just be moved into a separate configuration tab as 10 fixed strings instead of jumping through the hoops of treating them as special pseudo-channels throughout the code?

•    _mem.tone_mode = thd72.TMODES_REV[mem.tmode]
  

•    _mem.rtone = chirp_common.TONES.index(mem.rtone)
  

•    _mem.ctone = chirp_common.TONES.index(mem.ctone)
  

•    _mem.dtcs = chirp_common.DTCS_CODES.index(mem.dtcs)
  

•    _mem.cross_mode = chirp_common.CROSS_MODES.index(mem.cross_mode)
  

•    _mem.duplex = thd72.DUPLEX_REV[mem.duplex]
  

•    _mem.offset = mem.offset
  

•    _mem.mode = thd72.MODES_REV[mem.mode]
  

•    prog_vfo = thd72.get_prog_vfo(mem.freq)
  

•    #flag.prog_vfo = prog_vfo
  
  

Same D72 programmable VFO issue mentioned earlier.

Regards, James King VE7JWK