<div dir="ltr">This driver is very similar to the TH9800 driver. The memory map is slightly different, the features differ a bit, and the upload/download sequence is slightly different.<div><br></div><div>I started discussing a refactor with the TH9800 authors. They seem like a good bunch of guys. Unfortunately I don't have access to a TH7800 radio anymore and I don't have a TH9800. I didn't want to destabilize the TH9800 or break what I had for the TH7800 by refactoring much at this point.</div><div><br></div><div><span style="font-size:12.8px">> Hmm, why is this copied into the driver?</span><br style="font-size:12.8px">><br style="font-size:12.8px"><span style="font-size:12.8px">> > + add_radio_setting(basic, "apo", "Auto Power off (Hours)",</span><br style="font-size:12.8px"><span style="font-size:12.8px">> > + [("Off", 0), ("0.5", 5), ("1.0", 10), ("1.5", 15), ("2.0", 20)],</span><br style="font-size:12.8px"><span style="font-size:12.8px">> > + _settings.apo)</span><br></div><div><span style="font-size:12.8px"><br></span></div><div><span style="font-size:12.8px">I'm sorry, I don't understand your question. FWIW, APO is a feature that was broken in the TH9800 and I've fixed it in the TH7800. I planned to show the TH7800 authors how I've fixed it.</span></div><div><span style="font-size:12.8px"><br></span></div><div><span style="font-size:12.8px">There are probably a few things we could discuss about the driver. I almost forgot about the comments I added about "common code" that could potentially be moved to a different module for all to use. You're probably the right person to talk to about that.</span></div><div><span style="font-size:12.8px"><br></span></div><div><span style="font-size:12.8px">I can run the pep8 tester - hadn't done that. I'm sure I can break the lines before 80 columns.</span></div><div><span style="font-size:12.8px"><br></span></div><div><span style="font-size:12.8px">The tests pass with the driver.</span></div><div><span style="font-size:12.8px"><br></span></div><div><span style="font-size:12.8px">Regards,</span></div><div><span style="font-size:12.8px">Nathan</span></div><div><span style="font-size:12.8px"><br></span></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Jun 8, 2016 at 8:28 AM, Dan Smith via chirp_devel <span dir="ltr"><<a href="mailto:chirp_devel@intrepid.danplanet.com" target="_blank">chirp_devel@intrepid.danplanet.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Nathan,<br>
<span class=""><br>
> This is my first submission to the project. Hopefully I'm following the<br>
> process well enough. I tested this driver fairly thoroughly with the<br>
> radio when I had it, but no longer have access to it.<br>
<br>
</span>Cool, thanks!<br>
<span class=""><br>
> Wasn't sure how to include the binary file for unit tests, which I ran.<br>
> I attached it to this email.<br>
<br>
</span>Just attaching it here is okay. It would be even better to attach it to<br>
the ticket, because it doesn't get sent to all the subscribers and it's<br>
kind of archived with the request.<br>
<br>
> # HG changeset patch<br>
> # User Nathan Crapo <<a href="mailto:nathan@n4nc3o.com">nathan@n4nc3o.com</a>><br>
> # Date 1465334892 21600<br>
> # Tue Jun 07 15:28:12 2016 -0600<br>
> # Node ID 2f356864c55f674a6faf157d7e13868e5275cf72<br>
> # Parent 333a280ca0c4e856258ebf9dfdb7c547fa9ec90c<br>
> Adding support for TYT TH-7800. Fixes #3477.<br>
><br>
> diff -r 333a280ca0c4 -r 2f356864c55f chirp/drivers/th7800.py<br>
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000<br>
> +++ b/chirp/drivers/th7800.py Tue Jun 07 15:28:12 2016 -0600<br>
> @@ -0,0 +1,780 @@<br>
> +# Copyright 2014 Tom Hayward <<a href="mailto:tom@tomh.us">tom@tomh.us</a>><br>
> +# Copyright 2014 Jens Jensen <<a href="mailto:af5mi@yahoo.com">af5mi@yahoo.com</a>><br>
> +# Copyright 2014 James Lee N1DDK <<a href="mailto:jml@jmlzone.com">jml@jmlzone.com</a>><br>
> +# Copyright 2016 Nathan Crapo <<a href="mailto:nathan@n4nc3o.com">nathan@n4nc3o.com</a>> (TH-7800 only)<br>
<br>
Is this a near copy of another radio? Is it significantly different such<br>
that making them share a common base is not feasible?<br>
<br>
> +# --------------- Common Code ---------------<br>
> +<br>
> +# This section should go somewhere common like settings.py. Keep it here for<br>
> +# now until other developers review and accept or reject it.<br>
> +class RadioSettingValueMap(RadioSettingValueList):<br>
<br>
Hmm, why is this copied into the driver?<br>
<br>
> + add_radio_setting(basic, "apo", "Auto Power off (Hours)",<br>
> + [("Off", 0), ("0.5", 5), ("1.0", 10), ("1.5", 15), ("2.0", 20)],<br>
> + _settings.apo)<br>
<br>
Can you break these lines before 80 columns? There is a pep8 tester in<br>
the tree (ideally, just run run_all_tests.sh) to validate the style.<br>
<br>
I didn't apply this to run the tests, but I assume they all pass with this?<br>
<br>
Thanks!<br>
<br>
--Dan<br>
_______________________________________________<br>
chirp_devel mailing list<br>
<a href="mailto:chirp_devel@intrepid.danplanet.com">chirp_devel@intrepid.danplanet.com</a><br>
<a href="http://intrepid.danplanet.com/mailman/listinfo/chirp_devel" rel="noreferrer" target="_blank">http://intrepid.danplanet.com/mailman/listinfo/chirp_devel</a><br>
Developer docs: <a href="http://chirp.danplanet.com/projects/chirp/wiki/Developers" rel="noreferrer" target="_blank">http://chirp.danplanet.com/projects/chirp/wiki/Developers</a><br>
</blockquote></div><br></div>