[chirp_devel] [PATCH 0/6] improve cli download/upload
For the most part, this series incrementally improves the CLI, making the image upload/download process more robust and clean. This has all been tested with a Baofeng UV-5R.
The first patch fixes a couple of minor issues with the cpep8 scripts.
Zach Welch (5): Improve cpep8 scripts (#2355) Expose console verbosity (#2347) Make download/upload progress optional (#2343) Improve CLI download/upload (#2343) Fix status reporting in UV-5R (#2343)
Zachary T Welch (1): Add newline at end of console status (#2343)
chirp/chirp_common.py | 12 +++++++++--- chirp/drivers/uv5r.py | 2 ++ chirp/logger.py | 6 ++++++ chirpc | 40 ++++++++++++++++++++++++++++++---------- tools/cpep8.py | 2 ++ tools/cpep8.sh | 2 +- 6 files changed, 50 insertions(+), 14 deletions(-)
From: Zach Welch zach@mandolincreekfarm.com
# HG changeset patch # User Zach Welch zach@mandolincreekfarm.com # Fake Node ID eeb698d50d058dd6d30ede9507c0809dff6ecfe1
Improve cpep8 scripts (#2355)
This patch adds prints to cpep8.py's scanning function. It's useful to know what files the script is scanning during an update, as the in-tree virtualenv (or other files) may be included by accident.
This patch also fixes cpep8.sh to use the correct path to cpep8.py, allowing it to be run from anywhere in the source tree.
diff --git a/tools/cpep8.py b/tools/cpep8.py index fb1ff48..2bf55d0 100755 --- a/tools/cpep8.py +++ b/tools/cpep8.py @@ -90,12 +90,14 @@ def get_exceptions(f): return ignore
if args.update: + print "Starting update of %d files" % len(manifest) bad = [] for f in manifest: checker = pep8.StyleGuide(quiet=True, ignore=get_exceptions(f)) results = checker.check_files([f]) if results.total_errors: bad.append(f) + print "%s: %s" % (results.total_errors and "FAIL" or "PASS", f)
with file(blacklist_filename, "w") as fh: print >>fh, """\ diff --git a/tools/cpep8.sh b/tools/cpep8.sh index 88a7af9..89fd9ac 100755 --- a/tools/cpep8.sh +++ b/tools/cpep8.sh @@ -17,5 +17,5 @@ fi
source ${VENV}/bin/activate pip install pep8==${PEP8_VERSION} >${VENV}/pep8.log 2>&1 -./tools/cpep8.py "$@" +${TOOLS_DIR}/cpep8.py "$@" deactivate
From: Zach Welch zach@mandolincreekfarm.com
# HG changeset patch # User Zach Welch zach@mandolincreekfarm.com # Fake Node ID f9766c707308a213b993ea42f01872100a054c87
Expose console verbosity (#2347)
This patch adds a method to query the console verbosity level. This is useful for code that needs to produce non-logging output, conditional on whether the --quiet or --verbose options were given.
diff --git a/chirp/logger.py b/chirp/logger.py index eb29e6f..6e841bc 100644 --- a/chirp/logger.py +++ b/chirp/logger.py @@ -97,6 +97,7 @@ class Logger(object): self.early_level = logging.DEBUG
self.console = logging.StreamHandler(console_stream) + self.console_level = self.early_level self.console.setLevel(self.early_level) self.console.setFormatter(logging.Formatter(console_format)) self.logger.addHandler(self.console) @@ -132,6 +133,7 @@ class Logger(object): self.LOG.debug("verbosity=%d", level) if level > logging.CRITICAL: level = logging.CRITICAL + self.console_level = level self.console.setLevel(level)
def set_log_level(self, level): @@ -148,6 +150,11 @@ class Logger(object): Logger.instance = Logger()
+def is_visible(level): + """Returns True if a message at level will be shown on the console""" + return level >= Logger.instance.console_level + + def add_arguments(parser): parser.add_argument("-q", "--quiet", action="count", default=0, help="Decrease verbosity")
# HG changeset patch # User Zachary T Welch zach@mandolincreekfarm.com # Fake Node ID 7b488413bec51647efcc47a17d11502226442fec
Add newline at end of console status (#2343)
When downloading/uploading on the console, a pretty status bar is printed on a single line. At the end of the transfer, a newline needs to be added, or subsequent output will appear on the same line as the status bar.
diff --git a/chirp/chirp_common.py b/chirp/chirp_common.py index 16cbf40..0a34adc 100644 --- a/chirp/chirp_common.py +++ b/chirp/chirp_common.py @@ -1218,7 +1218,10 @@ class Status: pct = 0.0 ticks = "?" * 10
- return "|%-10s| %2.1f%% %s" % (ticks, pct, self.msg) + s = "|%-10s| %2.1f%% %s" % (ticks, pct, self.msg) + if self.cur == self.max: + s += "\n" + return s
def is_fractional_step(freq):
Add newline at end of console status (#2343)
When downloading/uploading on the console, a pretty status bar is printed on a single line. At the end of the transfer, a newline needs to be added, or subsequent output will appear on the same line as the status bar.
diff --git a/chirp/chirp_common.py b/chirp/chirp_common.py index 16cbf40..0a34adc 100644 --- a/chirp/chirp_common.py +++ b/chirp/chirp_common.py @@ -1218,7 +1218,10 @@ class Status: pct = 0.0 ticks = "?" * 10
return "|%-10s| %2.1f%% %s" % (ticks, pct, self.msg)
s = "|%-10s| %2.1f%% %s" % (ticks, pct, self.msg)if self.cur == self.max:s += "\n"return s
This seems like a layering violation. If I stringify a thing, I expect it to be mostly consistent and not imply details of how I'm using the string. Further, your '\n' at the end isn't valid on all platforms and won't have the desired effect there. You'll end up with a \n\r at the end of the last loop instead of a \r\n, which is what you'd want on Windows.
Why wouldn't we just print the newline once we're done spewing these lines from the caller and not make this inner thing do something based on how it thinks it's being called?
--Dan
From: Zach Welch zach@mandolincreekfarm.com
# HG changeset patch # User Zach Welch zach@mandolincreekfarm.com # Fake Node ID 404b50668252b9130530e9049ab132165b0c7206
Make download/upload progress optional (#2343)
This patch prevents the progress bar from appearing on the console if the user gives the --quiet option. It also changes its destination to stdout rather than stderr.
diff --git a/chirp/chirp_common.py b/chirp/chirp_common.py index 0a34adc..61cd7a0 100644 --- a/chirp/chirp_common.py +++ b/chirp/chirp_common.py @@ -654,9 +654,12 @@ class MTOBankModel(BankModel):
def console_status(status): """Write a status object to the console""" + import logging + from chirp import logger + if not logger.is_visible(logging.WARN): + return import sys - - sys.stderr.write("\r%s" % status) + sys.stdout.write("\r%s" % status)
class RadioPrompts:
From: Zach Welch zach@mandolincreekfarm.com
# HG changeset patch # User Zach Welch zach@mandolincreekfarm.com # Fake Node ID f1690dc7a7dcea8aec8801a87dcaf7f2acad1969
Improve CLI download/upload (#2343)
This patch adds much-needed checks in the CLI, allowing an unwitting user to stumble their way toward a working set of options that permits downloading/uploading an image from/to a radio.
diff --git a/chirpc b/chirpc index db3b13f..8ea0bed 100755 --- a/chirpc +++ b/chirpc @@ -17,6 +17,7 @@
import serial +import os import sys import argparse import logging @@ -185,7 +186,7 @@ if __name__ == "__main__": if options.mmap: rclass = directory.get_radio_by_image(options.mmap).__class__ else: - print "Must specify a radio model" + print "You must specify a radio model. See --list-radios." sys.exit(1) else: rclass = directory.get_radio(options.radio) @@ -195,6 +196,9 @@ if __name__ == "__main__": s = options.mmap else: s = options.radio + ".img" + if not os.path.exists(s): + LOG.error("Image file '%s' does not exist" % s) + sys.exit(1) else: LOG.info("opening %s at %i" % (options.serial, rclass.BAUD_RATE)) s = serial.Serial(port=options.serial, @@ -293,17 +297,33 @@ if __name__ == "__main__": print mem
if options.download_mmap: - # isinstance(radio, chirp_common.IcomMmapRadio) or fail_unsupported() - radio.sync_in() - radio.save_mmap(options.mmap) + if not issubclass(rclass, chirp_common.CloneModeRadio): + LOG.error("%s is not a clone mode radio" % options.radio) + sys.exit(1) + if not options.mmap: + LOG.error("You must specify the destnation file name with --mmap") + sys.exit(1) + try: + radio.sync_in() + radio.save_mmap(options.mmap) + except Exception, e: + LOG.error(e) + sys.exit(1)
if options.upload_mmap: - # isinstance(radio, chirp_common.IcomMmapRadio) or fail_unsupported() - radio.load_mmap(options.mmap) - if radio.sync_out(): - print "Clone successful" - else: - LOG.error("Clone failed") + if not issubclass(rclass, chirp_common.CloneModeRadio): + LOG.error("%s is not a clone mode radio" % options.radio) + sys.exit(1) + if not options.mmap: + LOG.error("You must specify the destnation file name with --mmap") + sys.exit(1) + try: + radio.load_mmap(options.mmap) + radio.sync_out() + print "Upload successful" + except Exception, e: + LOG.error(e) + sys.exit(1)
if options.mmap and isinstance(radio, chirp_common.CloneModeRadio): radio.save_mmap(options.mmap)
@@ -293,17 +297,33 @@ if __name__ == "__main__": print mem
if options.download_mmap:
# isinstance(radio, chirp_common.IcomMmapRadio) or fail_unsupported()radio.sync_in()radio.save_mmap(options.mmap)
if not issubclass(rclass, chirp_common.CloneModeRadio):LOG.error("%s is not a clone mode radio" % options.radio)sys.exit(1)if not options.mmap:LOG.error("You must specify the destnation file name with --mmap")
Typo in 'destination'.
sys.exit(1)try:radio.sync_in()radio.save_mmap(options.mmap)except Exception, e:LOG.error(e)
This is going to make it really hard to debug what's going on if/when someone reports it. LOG.exception() will dump the exception, but at ERROR level, which is probably not what you want here. Maybe check your global --debug flag and only do it in that case?
sys.exit(1)if options.upload_mmap:
# isinstance(radio, chirp_common.IcomMmapRadio) or fail_unsupported()radio.load_mmap(options.mmap)if radio.sync_out():print "Clone successful"else:LOG.error("Clone failed")
if not issubclass(rclass, chirp_common.CloneModeRadio):LOG.error("%s is not a clone mode radio" % options.radio)sys.exit(1)if not options.mmap:LOG.error("You must specify the destnation file name with --mmap")
Same typo here.
--Dan
On 03/05/2015 03:22 PM, Dan Smith wrote: ...
LOG.error("You must specify the destnation file name with --mmap")Typo in 'destination'.
Good catch.
sys.exit(1)try:radio.sync_in()radio.save_mmap(options.mmap)except Exception, e:LOG.error(e)This is going to make it really hard to debug what's going on if/when someone reports it. LOG.exception() will dump the exception, but at ERROR level, which is probably not what you want here. Maybe check your global --debug flag and only do it in that case?
For now, I think we need to use LOG.exception unconditionally. We can't use the new is_visible for log messages. That routine only checks the console's verbosity level. As such, it is only intended to control normal program output, since that does not go through the logging infrastructure.
For logging, we need to rely on the handlers to set their desired logging level (via --verbose/--quiet). Our code unconditionally makes the proper call, and the handlers sort out whether to display it. Concretely, the console and log file may have different logging levels (by default, WARN and DEBUG respectively), so a message may show up in one and not the other. In other words, log output is already conditional on the logging level; it would be redundant (at best) and confusing (at worst) to try to add another condition to it.
Down the road, I think we should plan to switch it to LOG.error, because other logging information (generated at the point of exception) should provide sufficient context for us to debug things. More generally, users should never see a stack trace (in an ideal world). Of course, we are nowhere near that goal as yet, so LOG.exception is the right thing to do.
sys.exit(1) if options.upload_mmap:
# isinstance(radio, chirp_common.IcomMmapRadio) or fail_unsupported()radio.load_mmap(options.mmap)if radio.sync_out():print "Clone successful"else:LOG.error("Clone failed")
if not issubclass(rclass, chirp_common.CloneModeRadio):LOG.error("%s is not a clone mode radio" % options.radio)sys.exit(1)if not options.mmap:LOG.error("You must specify the destnation file name with --mmap")Same typo here.
Actually, it's even worse; it should read "source", not "destination".
Cheers,
From: Zach Welch zach@mandolincreekfarm.com
# HG changeset patch # User Zach Welch zach@mandolincreekfarm.com # Fake Node ID 65278104b45c86cf57ba3b92cec73cd37a475204
Fix status reporting in UV-5R (#2343)
This patch ensures the UV-5R driver reports 100% completion after finishing an upload or download.
diff --git a/chirp/drivers/uv5r.py b/chirp/drivers/uv5r.py index beb1392..915431f 100644 --- a/chirp/drivers/uv5r.py +++ b/chirp/drivers/uv5r.py @@ -519,6 +519,7 @@ def _do_download(radio): for i in range(0, 0x1800, 0x40): data += _read_block(radio, i, 0x40) _do_status(radio, i) + _do_status(radio, radio.get_memsize()) LOG.debug("done.") LOG.debug("downloading aux block...") # Auxiliary block starts at 0x1ECO (?) @@ -568,6 +569,7 @@ def _do_upload(radio): for i in range(0x08, 0x1808, 0x10): _send_block(radio, i - 0x08, radio.get_mmap()[i:i + 0x10]) _do_status(radio, i) + _do_status(radio, radio.get_memsize())
if len(radio.get_mmap().get_packed()) == 0x1808: LOG.info("Old image, not writing aux block")
participants (3)
-
Dan Smith -
Zach Welch -
Zachary T Welch