On Sun, Mar 29, 2015 at 11:55 AM, Dan Smith dsmith@danplanet.com wrote:
# HG changeset patch # User Charles Stewart chuckination@gmail.com # Date 1427604940 14400 # Sun Mar 29 00:55:40 2015 -0400 # Node ID 4e970afe0a97d3d5f52165a1705c848f5e670825 # Parent ee81cedd447b16eacca70ff6e8bff3552e4a5227 [ts2000] Feature complete TS-2000 driver. Fixes #217
Debugged Tom Hayward's initial TS-2000 driver and modified kenwood_live.py to support detecting it.
Thanks!
Tom, can you take a look over this before we put it into the tree? Since it affects more than just the TS2000 driver, we should try to avoid breaking others.
I'm really struggling to read code this morning, but I'll do my best.
The use of "curj" is odd. Some code from the patch: + command_delimiters = [("\r"," "), (";","")] + for i in bauds: + curj = 0 + for j in command_delimiters: + curj += 1 + LOG.info("Trying ID at baud %i with delimiter check #%i" % (i, curj))
In Python, we usually do this with enumerate() if you really need the index, like this: command_delimiters = [("\r"," "), (";","")] for i in bauds: for curj, j in enumerate(command_delimiters): LOG.info("Trying ID at baud %i with delimiter check #%i" % (i, curj))
However, it looks like you only use the integer index during logging, so why not make the logs easier to read by logging the actual delimiter: command_delimiters = [("\r"," "), (";","")] for i in bauds: for j in command_delimiters: LOG.info("Trying ID at baud %i with delimiter %s" % (i, j))
Going one more step, you could make the code easier to read with a descriptive variable name: command_delimiters = [("\r"," "), (";","")] for i in bauds: for delimiter in command_delimiters: LOG.info("Trying ID at baud %i with delimiter %s" % (i, delimiter))
You changed a couple double line-breaks between top-level functions to single line-breaks. According to PEP8, these really should be double line-breaks. Please revert this. https://www.python.org/dev/peps/pep-0008/#blank-lines
Why change __memcache to _memcache? This appears to be only a style change and not affect the behavior of the TS-2000 driver. If this is true, I'd prefer if you chose one of these alternatives: 1) Leave __memcache the way it is, or 2) Send style changes in a separate patch.
Here's the list of errors from run_all_tests.sh: chirp/drivers/ts2000.py:19:80: E501 line too long (80 > 79 characters) chirp/drivers/ts2000.py:87:80: E501 line too long (84 > 79 characters) chirp/drivers/ts2000.py:156:80: E501 line too long (80 > 79 characters) chirp/drivers/ts2000.py:201:80: E501 line too long (93 > 79 characters) chirp/drivers/ts2000.py:235:80: E501 line too long (89 > 79 characters) chirp/drivers/ts2000.py:236:80: E501 line too long (92 > 79 characters) chirp/drivers/ts2000.py:237:80: E501 line too long (90 > 79 characters) chirp/drivers/ts2000.py:278:80: E501 line too long (89 > 79 characters) chirp/drivers/ts2000.py:279:80: E501 line too long (92 > 79 characters) chirp/drivers/ts2000.py:280:80: E501 line too long (90 > 79 characters) FAIL: Please use <80 columns in source files Checking for PEP8 regressions... ./chirp/drivers/kenwood_live.py:80:1: E302 expected 2 blank lines, found 1 ./chirp/drivers/kenwood_live.py:88:32: E231 missing whitespace after ',' ./chirp/drivers/kenwood_live.py:88:43: E231 missing whitespace after ',' ./chirp/drivers/kenwood_live.py:95:80: E501 line too long (81 > 79 characters) ./chirp/drivers/kenwood_live.py:113:1: E302 expected 2 blank lines, found 1 ./chirp/drivers/kenwood_live.py:124:1: E302 expected 2 blank lines, found 1
Thanks for working on this driver! Lots of people have asked for it.
Tom KD7LXL