<div dir="ltr"><div>Hi Angus,<br></div><div dir="ltr"><br></div><div dir="ltr">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.</div><div><br></div><div>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.</div><div dir="ltr"><div><br></div></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+from . import thd72<br></blockquote><div><br></div><div>Should be:<br>from chirp.drivers import thd72</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">+# memory channel<br>
+# 0 1 2 3 4 5 6 7 8 9 a b c d e f<br>
+# [freq ] ? mode tmode/duplex rtone ctone dtcs cross_mode [offset] ?<br>
+<br>
+# frequency is 32-bit unsigned little-endian Hz<br></blockquote><div><br></div><div>This is the D72's memory format, not the D74's.<br></div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+DEFAULT_PROG_VFO = (<br>
+ (136000000, 174000000),<br>
+ (410000000, 470000000),<br>
+ (118000000, 136000000),<br>
+ (136000000, 174000000),<br>
+ (320000000, 400000000),<br>
+ (400000000, 524000000),<br>
+)<br></blockquote><div><br></div><div>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.<br></div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+# Some of the blocks written by MCP have a length set of less than 0x00/256<br>
+BLOCK_SIZES = {<br>
+ 0x0003: 0x00B4,<br>
+ 0x0007: 0x0068,<br>
+}<br></blockquote><div><br></div><div>Why obfuscate these values by writing them in hex? Block size is also 8-bits not 16-bits, explained further down.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">+#seekto 0x04000;<br>
+// TODO: deal with the 16-byte trailers of every block<br>
+struct {<br>
+ struct {<br>
+ ul32 freq;<br>
+ ul32 offset;<br>
+ <br>
+ u8 tuning_step:4,<br>
+ unk:4;<br>
+ u8 mode:4,<br>
+ unk1:4;<br>
+ u8 tone_mode:4,<br>
+ duplex:4;<br>
+ u8 rtone;<br>
+ <br>
+ u8 ctone;<br>
+ u8 dtcs;<br>
+ u8 cross_mode:4<br>
+ digital_squelch:4;<br>
+ char urcall[8];<br>
+ char rpt1[8];<br>
+ char rpt2[8];<br>
+ <br>
+ u8 digital_squelch_code;<br>
+ <br>
+ } mem[6];<br>
+ <br>
+ u8 pad[16];<br>
+} memory[1167]; // TODO: correct number of memories<br></blockquote><div><br></div><div>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).</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">+#seekto 0x14700;<br>
+struct {<br>
+ char name[16];<br>
+} wx_name[10];<br>
+<br>
+#seekto 0x144d0;<br>
+struct {<br>
+ char name[16];<br>
+} call_name[6];<br></blockquote><div><br></div><div>My notes suggest these locations are inverted.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">+class SProxy(object):<br>
+ def __init__(self, delegate):<br>
+ self.delegate = delegate<br>
+<br>
+ def read(self, len):<br>
+ r = self.delegate.read(len)<br>
+ LOG.debug("READ\n" + hex(r))<br>
+ return r<br>
+<br>
+ def write(self, data):<br>
+ LOG.debug("WRITE\n" + hex(data))<br>
+ return self.delegate.write(str(data))<br>
+<br>
+ @property<br>
+ def timeout(self):<br>
+ return self.delegate.timeout<br>
+<br>
+ @timeout.setter<br>
+ def timeout(self, timeout):<br>
+ self.delegate.timeout = timeout<br>
</blockquote><div><br></div><div>Debug stuff that doesn't belong in the driver.<br></div></div><div class="gmail_quote"><br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+@directory.register<br>
+class THD74Radio(thd72.THD72Radio):<br>
+ MODEL = "TH-D74 (clone mode)"<br>
+ #MODEL = "TH-D74"<br>
+ _memsize = 500480<br>
+ # I think baud rate might be ignored by USB-Serial stack of the D74's<br>
+ # on-board FTDI chip, but it doesn't seem to hurt.<br>
+ BAUD_RATE = 115200<br>
</blockquote><div><br></div><div>MCP-74 always uses a baud rate of 9600 in the captures I did.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">+ #def __init__(self, pipe):<br>
+ # pipe = SProxy(pipe)<br>
+ # super(THD74Radio, self).__init__(pipe)<br></blockquote><div><br></div><div>More debug cruft.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+ def get_features(self):<br>
+ rf = super(THD74Radio, self).get_features()<br>
+ rf.has_tuning_step = True<br>
+ return rf<br></blockquote><div><br></div><div>This returns the wrong rf.memory_bounds and rf.valid_bands for the TH-D74.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">+ def write_block(self, block, map, count=256):<br>
+ #print("Write block ", block )<br>
+ c = struct.pack(">cHH", b"W", block, count%256)<br>
+ base = block * 256<br>
+ data = map[base:base + count]<br>
+ # It's crucial that these are written together. Otherwise the radio<br>
+ # will fail to ACK under some conditions.<br>
+ c_d = ''.join(chr(b) for b in c) + data<br>
+ self.pipe.write(c_d)<br>
+<br>
+ ack = self.pipe.read(1)<br>
+<br>
+ if len(ack) == 0:<br>
+ print("read timed out block %d - trying again" % block )<br>
+ time.sleep(0.5)<br>
+ ack = self.pipe.read(1)<br>
+<br>
+ if ack != chr(0x06):<br>
+ print("Block %d write failed %d" % ( block, ord(ack)))<br>
+<br>
+ return ack == chr(0x06)<br>
+<br>
+ def _unlock(self):<br>
+ """Voodoo sequence of operations to get the radio to accept our programming."""<br>
+<br>
+ h = self.read_block(0, 6)<br>
+<br>
+ unlock = ("\x57\x00\x00\x00\x30\xff\x01\xff\x00\x00\xff\xff\xff\xff\xff\x01" +<br>
+ "\x00\x00\x00\x03\x01\x00\x00\x00\x00\x02\x00\x30\x30\x30\x00\xff" +<br>
+ "\xff\xff\xff\xff\xff\xff\x00\xff\xff\xff\xff\xff\xff\xff\xff\xff" +<br>
+ "\xff\xff\xff\xff\xff")<br>
+<br>
+ self.pipe.write(unlock)<br>
+<br>
+ ack = self.pipe.read(1)<br>
+<br>
+ if ack != chr(0x06):<br>
+ raise errors.RadioError("Expected ack but got {} ({})".format(ord(ack), type(ack)))<br>
+<br>
+ c = struct.pack(">cHH", b"W", 0, 0x38C8)<br>
+ self.pipe.write(''.join(chr(b) for b in c))<br>
+ # magic unlock sequence<br>
+ unlock = [0xFF] * 8 + [0] * 160 + [0xFF] * 32<br>
+ unlock = "".join([chr(x) for x in unlock])<br>
+ self.pipe.write(unlock)<br>
</blockquote><div><br></div><div>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.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+ # For some reason MCP-D74 skips this block. If we don't, we'll get a NACK<br>
+ # on the next one. There is also a more than 500 ms delay for the ACK.<br>
+ if 1279 in blocks:<br>
+ blocks.remove(1279)<br>
</blockquote><div><br></div><div>MCP-74 skips over many more blocks than this.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">+ if self.status_fn:<br>
+ s = chirp_common.Status()<br>
+ s.msg = "Cloning to radio"<br>
+ s.max = total<br>
+ s.cur = count<br>
+ self.status_fn(s)<br>
+<br>
+ lock = ("\x57\x00\x00\x00\x06\x02\x01\xff\x00\x00\xff")<br>
+ self.pipe.write(lock)<br></blockquote><div><br></div><div>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.</div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">+ def set_channel_name(self, number, name):<br>
+ name = name[:16] + '\x00' * 16<br>
+ if number < 999:<br>
+ self._memobj.channel_name[number].name = name[:16]<br>
+ self.add_dirty_block(self._memobj.channel_name[number])<br>
+ elif number >= 1020 and number < 1030:<br>
+ number -= 1020<br>
+ self._memobj.wx_name[number].name = name[:16]<br>
+ self.add_dirty_block(self._memobj.wx_name[number])<br>
</blockquote><div><br></div><div>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?</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">+ _mem.tone_mode = thd72.TMODES_REV[mem.tmode]<br>
+ _mem.rtone = chirp_common.TONES.index(mem.rtone)<br>
+ _mem.ctone = chirp_common.TONES.index(mem.ctone)<br>
+ _mem.dtcs = chirp_common.DTCS_CODES.index(mem.dtcs)<br>
+ _mem.cross_mode = chirp_common.CROSS_MODES.index(mem.cross_mode)<br>
+ _mem.duplex = thd72.DUPLEX_REV[mem.duplex]<br>
+ _mem.offset = mem.offset<br>
+ _mem.mode = thd72.MODES_REV[mem.mode]<br>
+<br>
+ prog_vfo = thd72.get_prog_vfo(mem.freq)<br>
+ #flag.prog_vfo = prog_vfo<br>
</blockquote><div><br></div><div>Same D72 programmable VFO issue mentioned earlier.</div><div><br></div><div>Regards,</div><div>James King VE7JWK<br></div></div></div>