[chirp_devel] Adding TYT TH-7800 Support. Fixes #3477
This is my first submission to the project. Hopefully I'm following the process well enough. I tested this driver fairly thoroughly with the radio when I had it, but no longer have access to it.
Wasn't sure how to include the binary file for unit tests, which I ran. I attached it to this email.
Please let me know if I can provide anything else.
Regards, Nathan
Hi Nathan,
This is my first submission to the project. Hopefully I'm following the process well enough. I tested this driver fairly thoroughly with the radio when I had it, but no longer have access to it.
Cool, thanks!
Wasn't sure how to include the binary file for unit tests, which I ran. I attached it to this email.
Just attaching it here is okay. It would be even better to attach it to the ticket, because it doesn't get sent to all the subscribers and it's kind of archived with the request.
# HG changeset patch # User Nathan Crapo nathan@n4nc3o.com # Date 1465334892 21600 # Tue Jun 07 15:28:12 2016 -0600 # Node ID 2f356864c55f674a6faf157d7e13868e5275cf72 # Parent 333a280ca0c4e856258ebf9dfdb7c547fa9ec90c Adding support for TYT TH-7800. Fixes #3477.
diff -r 333a280ca0c4 -r 2f356864c55f chirp/drivers/th7800.py --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/chirp/drivers/th7800.py Tue Jun 07 15:28:12 2016 -0600 @@ -0,0 +1,780 @@ +# Copyright 2014 Tom Hayward tom@tomh.us +# Copyright 2014 Jens Jensen af5mi@yahoo.com +# Copyright 2014 James Lee N1DDK jml@jmlzone.com +# Copyright 2016 Nathan Crapo nathan@n4nc3o.com (TH-7800 only)
Is this a near copy of another radio? Is it significantly different such that making them share a common base is not feasible?
+# --------------- Common Code ---------------
+# This section should go somewhere common like settings.py. Keep it here for +# now until other developers review and accept or reject it. +class RadioSettingValueMap(RadioSettingValueList):
Hmm, why is this copied into the driver?
add_radio_setting(basic, "apo", "Auto Power off (Hours)",[("Off", 0), ("0.5", 5), ("1.0", 10), ("1.5", 15), ("2.0", 20)],_settings.apo)
Can you break these lines before 80 columns? There is a pep8 tester in the tree (ideally, just run run_all_tests.sh) to validate the style.
I didn't apply this to run the tests, but I assume they all pass with this?
Thanks!
--Dan
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.
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.
Hmm, why is this copied into the driver?
add_radio_setting(basic, "apo", "Auto Power off (Hours)",[("Off", 0), ("0.5", 5), ("1.0", 10),
("1.5", 15), ("2.0", 20)],
_settings.apo)
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.
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.
I can run the pep8 tester - hadn't done that. I'm sure I can break the lines before 80 columns.
The tests pass with the driver.
Regards, Nathan
On Wed, Jun 8, 2016 at 8:28 AM, Dan Smith via chirp_devel < chirp_devel@intrepid.danplanet.com> wrote:
Hi Nathan,
This is my first submission to the project. Hopefully I'm following the process well enough. I tested this driver fairly thoroughly with the radio when I had it, but no longer have access to it.
Cool, thanks!
Wasn't sure how to include the binary file for unit tests, which I ran. I attached it to this email.
Just attaching it here is okay. It would be even better to attach it to the ticket, because it doesn't get sent to all the subscribers and it's kind of archived with the request.
# HG changeset patch # User Nathan Crapo nathan@n4nc3o.com # Date 1465334892 21600 # Tue Jun 07 15:28:12 2016 -0600 # Node ID 2f356864c55f674a6faf157d7e13868e5275cf72 # Parent 333a280ca0c4e856258ebf9dfdb7c547fa9ec90c Adding support for TYT TH-7800. Fixes #3477.
diff -r 333a280ca0c4 -r 2f356864c55f chirp/drivers/th7800.py --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/chirp/drivers/th7800.py Tue Jun 07 15:28:12 2016 -0600 @@ -0,0 +1,780 @@ +# Copyright 2014 Tom Hayward tom@tomh.us +# Copyright 2014 Jens Jensen af5mi@yahoo.com +# Copyright 2014 James Lee N1DDK jml@jmlzone.com +# Copyright 2016 Nathan Crapo nathan@n4nc3o.com (TH-7800 only)
Is this a near copy of another radio? Is it significantly different such that making them share a common base is not feasible?
+# --------------- Common Code ---------------
+# This section should go somewhere common like settings.py. Keep it
here for
+# now until other developers review and accept or reject it. +class RadioSettingValueMap(RadioSettingValueList):
Hmm, why is this copied into the driver?
add_radio_setting(basic, "apo", "Auto Power off (Hours)",[("Off", 0), ("0.5", 5), ("1.0", 10), ("1.5",15), ("2.0", 20)],
_settings.apo)Can you break these lines before 80 columns? There is a pep8 tester in the tree (ideally, just run run_all_tests.sh) to validate the style.
I didn't apply this to run the tests, but I assume they all pass with this?
Thanks!
--Dan _______________________________________________ chirp_devel mailing list chirp_devel@intrepid.danplanet.com http://intrepid.danplanet.com/mailman/listinfo/chirp_devel Developer docs: http://chirp.danplanet.com/projects/chirp/wiki/Developers
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.
Okay, generally we would make those a single driver base class with subclasses for the variants. If you don't have the radio to test with them we'll just have to keep them separate for now I guess.
Hmm, why is this copied into the driver?
add_radio_setting(basic, "apo", "Auto Power off (Hours)",[("Off", 0), ("0.5", 5), ("1.0", 10), ("1.5", 15), ("2.0", 20)],_settings.apo)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.
This referred to the bit above, which is the common code.
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.
Yep, please just put those pieces in settings.py next to the others.
I can run the pep8 tester - hadn't done that. I'm sure I can break the lines before 80 columns.
Cool, thanks. I'll probably complain about a few other bits.
The tests pass with the driver.
Excellent. If you can fix up the style bits and move the common stuff over to the right place we should be good to go.
Thanks!
--Dan
participants (2)
-
Dan Smith -
Nathan Crapo