[chirp_devel] [PATCH] [thd74] Add a Kenwood d74 driver
James King
Thu Jun 18 21:06:56 PDT 2020
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.
+ at 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://intrepid.danplanet.com/pipermail/chirp_devel/attachments/20200618/d439361a/attachment-0001.html
More information about the chirp_devel
mailing list