From a6c24ab5e990b9dc48fb0fb8097e46e3e2545fc0 Mon Sep 17 00:00:00 2001 From: Jeff Quast Date: Sun, 28 Dec 2014 19:06:19 -0800 Subject: [PATCH 01/13] Use functional tests (tox + py.test + lrzsz package) This implements travis-ci integration, yet untested. I will have to clone the repository to allow travis-ci access for testing. Once I added tox support, I added python3 testing -- which immediately failed. I spent some time adding python3 support. Some of it may incur some small overhead. We can address these if we decide on a new API for YMODEM (multi-file, file-like iface) support. Additionally, debug logging is less verbose in the case of not logging each successful checksum -- and more verbose in the case of indicating each and every step through the transfer. These were decorated as I went with the python3 porting process. --- .gitignore | 3 + .travis.yml | 24 ++-- requirements-testing.txt | 3 + test/functional/__init__.py | 0 test/functional/test-recv.py | 42 ------- test/functional/test-send.py | 48 ------- test/functional/test_xmodem.py | 223 +++++++++++++++++++++++++++++++++ test/unit/test_xmodem.py | 13 +- tox.ini | 13 ++ xmodem/__init__.py | 218 ++++++++++++++++++++------------ 10 files changed, 409 insertions(+), 178 deletions(-) create mode 100644 requirements-testing.txt create mode 100644 test/functional/__init__.py delete mode 100644 test/functional/test-recv.py delete mode 100644 test/functional/test-send.py create mode 100644 test/functional/test_xmodem.py create mode 100644 tox.ini diff --git a/.gitignore b/.gitignore index 46fb7fb..affae6e 100644 --- a/.gitignore +++ b/.gitignore @@ -3,3 +3,6 @@ *.pyc /dist /xmodem.egg-info +/.coverage +/.tox +*.py.swp diff --git a/.travis.yml b/.travis.yml index d0a2c03..e97cf29 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,10 +1,20 @@ language: python -python: - - "2.6" - - "2.7" -# install required packages + +env: + - TOXENV=py26 + - TOXENV=py27 + - TOXENV=py33 + - TOXENV=pypy + - TOXENV=py34 + install: - - sudo apt-get install lrzsz -# command to run tests + - pip install -q tox + - sudo apt-get install lrzsz + script: - - make tests + - tox -e $TOXENV + +notifications: + email: + - contact@jeffquast.com + - maze@pyth0n.org diff --git a/requirements-testing.txt b/requirements-testing.txt new file mode 100644 index 0000000..93cf5f7 --- /dev/null +++ b/requirements-testing.txt @@ -0,0 +1,3 @@ +pytest-cov +pytest +tox diff --git a/test/functional/__init__.py b/test/functional/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/test/functional/test-recv.py b/test/functional/test-recv.py deleted file mode 100644 index 1313156..0000000 --- a/test/functional/test-recv.py +++ /dev/null @@ -1,42 +0,0 @@ -import select -import subprocess -import sys -import StringIO -from xmodem import XMODEM - -if __name__ == '__main__': - pipe = subprocess.Popen(['sx', '--xmodem', __file__], - stdin=subprocess.PIPE, stdout=subprocess.PIPE) - si, so = (pipe.stdin, pipe.stdout) - - def getc(size, timeout=3): - w,t,f = select.select([so], [], [], timeout) - if w: - data = so.read(size) - else: - data = None - - print 'getc(', repr(data), ')' - return data - - def putc(data, timeout=3): - w,t,f = select.select([], [si], [], timeout) - if t: - si.write(data) - si.flush() - size = len(data) - else: - size = None - - print 'putc(', repr(data), repr(size), ')' - return size - - stream = StringIO.StringIO() - xmodem = XMODEM(getc, putc) - nbytes = xmodem.recv(stream, retry=8) - - print >> sys.stderr, 'received', nbytes, 'bytes' - print >> sys.stderr, stream.getvalue() - - sys.exit(int(nbytes == 0)) - diff --git a/test/functional/test-send.py b/test/functional/test-send.py deleted file mode 100644 index cd42f80..0000000 --- a/test/functional/test-send.py +++ /dev/null @@ -1,48 +0,0 @@ -import os -import select -import subprocess -import sys -import StringIO -import tempfile -from xmodem import XMODEM - -if __name__ == '__main__': - fd, fn = tempfile.mkstemp() - pipe = subprocess.Popen(['rx', '--xmodem', fn], - stdin=subprocess.PIPE, stdout=subprocess.PIPE) - si, so = (pipe.stdin, pipe.stdout) - - def getc(size, timeout=3): - w,t,f = select.select([so], [], [], timeout) - if w: - data = so.read(size) - else: - data = None - - print 'getc(', repr(data), ')' - return data - - def putc(data, timeout=3): - w,t,f = select.select([], [si], [], timeout) - if t: - si.write(data) - si.flush() - size = len(data) - else: - size = None - - print 'putc(', repr(data), repr(size), ')' - return size - - stream = open(__file__, 'rb') - xmodem = XMODEM(getc, putc) - status = xmodem.send(stream, retry=8) - stream.close() - - print >> sys.stderr, 'sent', status - print >> sys.stderr, file(fn).read() - - os.unlink(fn) - - sys.exit(int(not status)) - diff --git a/test/functional/test_xmodem.py b/test/functional/test_xmodem.py new file mode 100644 index 0000000..ee1b9c7 --- /dev/null +++ b/test/functional/test_xmodem.py @@ -0,0 +1,223 @@ +import os +import errno +import select +import logging +import platform +import tempfile +import functools +import subprocess +try: + # python 3 + from io import BytesIO +except ImportError: + # python 2 + import StringIO.StringIO as BytesIO +from xmodem import XMODEM, XMODEM1k + +logging.basicConfig(format='%(levelname)-5s %(message)s', + level=logging.DEBUG) + + +def _multi_which(prog_names): + for prog_name in prog_names: + proc = subprocess.Popen(('which', prog_name), stdout=subprocess.PIPE) + stdout, stderr = proc.communicate() + if proc.returncode == 0: + return stdout.strip() + return None + + +def _get_recv_program(): + bin_path = _multi_which(('rb', 'lrb')) + assert bin_path is not None, ( + "program required: {0!r}. " + "Try installing lrzsz package.".format(bin_path)) + return bin_path + + +def _get_send_program(): + bin_path = _multi_which(('sb', 'lsb')) + assert bin_path is not None, ( + "program required: {0!r}. " + "Try installing lrzsz package.".format(bin_path)) + return bin_path + +recv_prog = _get_recv_program() +send_prog = _get_send_program() + + +def _fill_binary_data(stream): + chunksize = 521 + for byte in range(0x00, 0xff + 1): + stream.write(bytearray([byte] * chunksize)) + stream.seek(0) + return stream + + +def _verify_binary_data(stream, padding=b'\xff'): + stream.seek(0) + chunksize = 521 + for byte in range(0x00, 0xff + 1): + assert stream.read(chunksize) == bytearray([byte] * chunksize) + while True: + try: + # BSD-style EOF + data = stream.read(1) + assert data in (b'', padding) + if data == b'': + # BSD-style EOF + break + except OSError as err: + # Linux-style EOF + assert err.errno == errno.EIO + + +def _proc_getc(size, timeout=1, proc=None): + # our getc function simply pipes to the standard out of the `rb' + # or `lrb' program -- any data written by such program is returned + # by our getc() callback. + assert proc.returncode is None, ("{0} has exited: (returncode={1})" + .format(proc, proc.returncode)) + logging.debug(('get', size)) + ready_read, _, _ = select.select([proc.stdout], [], [], timeout) + if not ready_read: + assert False, ("Timeout on stdout of {0}.".format(proc)) + data = proc.stdout.read(size) + logging.debug(('got', len(data), data)) + return data + + +def _proc_putc(data, timeout=1, proc=None): + # similarly, our putc function simply writes to the standard of + # our `rb' or `lrb' program -- any data written by our XMODEM + # protocol via putc() callback is written to the stdin of such + # program. + assert proc.returncode is None, ("{0} has exited: (returncode={1})" + .format(proc, proc.returncode)) + _, ready_write, _ = select.select([], [proc.stdin], [], timeout) + if not ready_write: + assert False, ("Timeout on stdin of {0}.".format(proc)) + logging.debug(('put', len(data), data)) + proc.stdin.write(data) + proc.stdin.flush() + return len(data) + + +def test_xmodem_send(): + """ Using external program for receive, verify XMODEM.send(). """ + # Given, + _, recv_filename = tempfile.mkstemp() + try: + proc = subprocess.Popen( + (recv_prog, '--xmodem', '--verbose', recv_filename), + stdin=subprocess.PIPE, stdout=subprocess.PIPE, bufsize=0) + + getc = functools.partial(_proc_getc, proc=proc) + putc = functools.partial(_proc_putc, proc=proc) + + xmodem = XMODEM(getc, putc, pad=b'\xbb') + stream = _fill_binary_data(BytesIO()) + + # Exercise, + status = xmodem.send(stream, timeout=5) + + # Verify, + assert status is True + _verify_binary_data(stream) + _verify_binary_data(open(recv_filename, 'rb'), padding=b'\xbb') + proc.wait() + assert proc.returncode == 0 + + finally: + if os.path.isfile(recv_filename): + os.unlink(recv_filename) + + +def test_xmodem_recv(): + """ Using external program for send, verify XMODEM.recv(). """ + # Given, + _, send_filename = tempfile.mkstemp() + try: + with open(send_filename, 'wb') as stream: + _fill_binary_data(stream) + proc = subprocess.Popen( + (send_prog, '--xmodem', '--verbose', send_filename), + stdin=subprocess.PIPE, stdout=subprocess.PIPE, bufsize=0) + + getc = functools.partial(_proc_getc, proc=proc) + putc = functools.partial(_proc_putc, proc=proc) + + xmodem = XMODEM(getc, putc, pad=b'\xbb') + recv_stream = BytesIO() + + # Exercise, + status = xmodem.recv(recv_stream, timeout=5) + + # Verify, + assert status == recv_stream.tell() + _verify_binary_data(recv_stream, padding=b'\xbb') + proc.wait() + assert proc.returncode == 0 + + finally: + os.unlink(send_filename) + + +def test_xmodem1k_send(): + """ Using external program for receive, verify XMODEM1k.send(). """ + # Given, + _, recv_filename = tempfile.mkstemp() + try: + proc = subprocess.Popen( + (recv_prog, '--xmodem', '--verbose', recv_filename), + stdin=subprocess.PIPE, stdout=subprocess.PIPE, bufsize=0) + + getc = functools.partial(_proc_getc, proc=proc) + putc = functools.partial(_proc_putc, proc=proc) + + xmodem = XMODEM1k(getc, putc, pad=b'\xbb') + stream = _fill_binary_data(BytesIO()) + + # Exercise, + status = xmodem.send(stream, timeout=5) + + # Verify, + assert status is True + _verify_binary_data(stream) + _verify_binary_data(open(recv_filename, 'rb'), padding=b'\xbb') + proc.wait() + assert proc.returncode == 0 + + finally: + os.unlink(recv_filename) + + +def test_xmodem1k_recv(): + """ Using external program for send, verify XMODEM1k.recv(). """ + # Given, + _, send_filename = tempfile.mkstemp() + try: + with open(send_filename, 'wb') as stream: + _fill_binary_data(stream) + proc = subprocess.Popen( + (send_prog, '--xmodem', '--verbose', '--1k', send_filename), + stdin=subprocess.PIPE, stdout=subprocess.PIPE, bufsize=0) + + getc = functools.partial(_proc_getc, proc=proc) + putc = functools.partial(_proc_putc, proc=proc) + + xmodem = XMODEM1k(getc, putc, pad=b'\xbb') + recv_stream = BytesIO() + + # Exercise, + status = xmodem.recv(recv_stream, timeout=5) + + # Verify, + assert status == recv_stream.tell() + _verify_binary_data(recv_stream, padding=b'\xbb') + proc.wait() + assert proc.returncode == 0 + + finally: + if os.path.isfile(send_filename): + os.unlink(send_filename) diff --git a/test/unit/test_xmodem.py b/test/unit/test_xmodem.py index cce6111..3b724d5 100644 --- a/test/unit/test_xmodem.py +++ b/test/unit/test_xmodem.py @@ -1,8 +1,13 @@ """ Unit tests for XMODEM protocol. """ -# std -from StringIO import StringIO +# std imports +try: + # python 3 + from io import BytesIO +except ImportError: + # python 2 + import StringIO.StringIO as BytesIO # local import xmodem @@ -26,7 +31,7 @@ def test_xmodem_dummy_fails_send(mode): putc=dummy_putc, mode=mode) # exercise - status = modem.send(StringIO(b'dummy-stream')) + status = modem.send(BytesIO(b'dummy-stream')) # verify assert not status, ("Expected value of status `False'") @@ -39,4 +44,4 @@ def test_xmodem_bad_mode(): mode=mode) # exercise with pytest.raises(ValueError): - status = modem.send(StringIO(b'dummy-stream')) + status = modem.send(BytesIO(b'dummy-stream')) diff --git a/tox.ini b/tox.ini new file mode 100644 index 0000000..37ca48f --- /dev/null +++ b/tox.ini @@ -0,0 +1,13 @@ +[tox] +envlist = py26, + py27, + py33, + py34, + pypy + +skip_missing_interpreters = true + +[testenv] +usedevelop=true +deps=-rrequirements-testing.txt +commands = py.test --verbose --verbose --cov xmodem {posargs} diff --git a/xmodem/__init__.py b/xmodem/__init__.py index 1ec04ac..fe2a849 100644 --- a/xmodem/__init__.py +++ b/xmodem/__init__.py @@ -106,6 +106,7 @@ ''' +from __future__ import division, print_function __author__ = 'Wijnand Modderman ' __copyright__ = ['Copyright (c) 2010 Wijnand Modderman', @@ -113,20 +114,21 @@ __license__ = 'MIT' __version__ = '0.3.3' +import platform import logging import time import sys from functools import partial # Protocol bytes -SOH = chr(0x01) -STX = chr(0x02) -EOT = chr(0x04) -ACK = chr(0x06) -DLE = chr(0x10) -NAK = chr(0x15) -CAN = chr(0x18) -CRC = chr(0x43) # C +SOH = b'\x01' +STX = b'\x02' +EOT = b'\x04' +ACK = b'\x06' +DLE = b'\x10' +NAK = b'\x15' +CAN = b'\x18' +CRC = b'C' class XMODEM(object): @@ -190,7 +192,7 @@ class XMODEM(object): 0x6e17, 0x7e36, 0x4e55, 0x5e74, 0x2e93, 0x3eb2, 0x0ed1, 0x1ef0, ] - def __init__(self, getc, putc, mode='xmodem', pad='\x1a'): + def __init__(self, getc, putc, mode='xmodem', pad=b'\x1a'): self.getc = getc self.putc = putc self.mode = mode @@ -201,7 +203,7 @@ def abort(self, count=2, timeout=60): ''' Send an abort sequence using CAN bytes. ''' - for counter in xrange(0, count): + for _ in range(count): self.putc(CAN, timeout) def send(self, stream, retry=16, timeout=60, quiet=False, callback=None): @@ -209,7 +211,7 @@ def send(self, stream, retry=16, timeout=60, quiet=False, callback=None): Send a stream via the XMODEM protocol. >>> stream = file('/etc/issue', 'rb') - >>> print modem.send(stream) + >>> print(modem.send(stream)) True Returns ``True`` upon successful transmission or ``False`` in case of @@ -244,6 +246,7 @@ def callback(total_packets, success_count, error_count) raise ValueError("Invalid mode specified: {self.mode!r}" .format(self=self)) + self.log.debug('Begin start sequence, packet_size=%d', packet_size) error_count = 0 crc_mode = 0 cancel = 0 @@ -251,27 +254,31 @@ def callback(total_packets, success_count, error_count) char = self.getc(1) if char: if char == NAK: + self.log.debug('standard checksum requested (NAK).') crc_mode = 0 break elif char == CRC: + self.log.debug('16-bit CRC requested (CRC).') crc_mode = 1 break elif char == CAN: if not quiet: - print >> sys.stderr, 'received CAN' + print('received CAN', file=sys.stderr) if cancel: - self.log.info('Transmission canceled: ' - 'Received CAN twice.') + self.log.info('Transmission canceled: received 2xCAN ' + 'at start-sequence') return False else: + self.log.debug('cancellation at start sequence.') cancel = 1 else: - self.log.error('send ERROR expected NAK/CRC, ' - 'got %s', ord(char)) + self.log.error('send error: expected NAK, CRC, or CAN; ' + 'got %r', char) error_count += 1 if error_count >= retry: - self.log.info('error_count reached %s, aborting.', retry) + self.log.info('send error: error_count reached %d, ' + 'aborting.', retry) self.abort(timeout=timeout) return False @@ -283,31 +290,21 @@ def callback(total_packets, success_count, error_count) while True: data = stream.read(packet_size) if not data: - self.log.info('sending EOT') # end of stream + self.log.debug('send: at EOF') break total_packets += 1 + + header = self._make_send_header(packet_size, sequence) data = data.ljust(packet_size, self.pad) - if crc_mode: - crc = self.calc_crc(data) - else: - crc = self.calc_checksum(data) + checksum = self._make_send_checksum(crc_mode, data) # emit packet while True: - if packet_size == 128: - self.putc(SOH) - else: # packet_size == 1024 - self.putc(STX) - self.putc(chr(sequence)) - self.putc(chr(0xff - sequence)) + self.log.debug('send: block %d', sequence) + self.putc(header) self.putc(data) - if crc_mode: - self.putc(chr(crc >> 8)) - self.putc(chr(crc & 0xff)) - else: - self.putc(chr(crc)) - + self.putc(checksum) char = self.getc(1, timeout) if char == ACK: success_count += 1 @@ -315,28 +312,33 @@ def callback(total_packets, success_count, error_count) callback(total_packets, success_count, error_count) break if char == NAK: + self.log.warn('send error: NAK received ' + 'for block %d', sequence) error_count += 1 if callable(callback): callback(total_packets, success_count, error_count) if error_count >= retry: # excessive amounts of retransmissions requested, # abort transfer + self.log.error('send error: NAK received %d times, ' + 'aborting.', error_count) self.abort(timeout=timeout) - self.log.warning('excessive NAKs, transfer aborted') return False # return to loop and resend continue # protocol error + self.log.error('send error: expected ACK, NAK; got %r, ' + 'aborting.', char) self.abort(timeout=timeout) - self.log.error('protocol error (getc returned %r)', char) return False # keep track of sequence sequence = (sequence + 1) % 0x100 while True: + self.log.debug('sending EOT, awaiting ACK') # end of transmission self.putc(EOT) @@ -345,21 +347,42 @@ def callback(total_packets, success_count, error_count) if char == ACK: break else: + self.log.error('send error: expected ACK; got %r', char) error_count += 1 if error_count >= retry: + self.log.warn('EOT was not ACKd, aborting transfer') self.abort(timeout=timeout) - self.log.warning('EOT was not ACKd, transfer aborted') return False - self.log.info('Transmission successful.') + self.log.info('Transmission successful (ACK received).') return True + def _make_send_header(self, packet_size, sequence): + assert packet_size in (128, 1024), packet_size + _bytes = [] + if packet_size == 128: + _bytes.append(ord(SOH)) + elif packet_size == 1024: + _bytes.append(ord(STX)) + _bytes.extend([sequence, 0xff - sequence]) + return bytearray(_bytes) + + def _make_send_checksum(self, crc_mode, data): + _bytes = [] + if crc_mode: + crc = self.calc_crc(data) + _bytes.extend([crc >> 8, crc & 0xff]) + else: + crc = self.calc_checksum(data) + _bytes.append(crc) + return bytearray(_bytes) + def recv(self, stream, crc_mode=1, retry=16, timeout=60, delay=1, quiet=0): ''' Receive a stream via the XMODEM protocol. >>> stream = file('/etc/issue', 'wb') - >>> print modem.recv(stream) + >>> print(modem.recv(stream)) 2342 Returns the number of bytes received on success or ``None`` in case of @@ -377,30 +400,38 @@ def recv(self, stream, crc_mode=1, retry=16, timeout=60, delay=1, quiet=0): self.log.info('error_count reached %d, aborting.', retry) self.abort(timeout=timeout) return None - elif crc_mode and error_count < (retry / 2): + elif crc_mode and error_count < (retry // 2): if not self.putc(CRC): + self.log.debug('recv error: putc failed, ' + 'sleeping for %d', delay) time.sleep(delay) error_count += 1 else: crc_mode = 0 if not self.putc(NAK): + self.log.debug('recv error: putc failed, ' + 'sleeping for %d', delay) time.sleep(delay) error_count += 1 char = self.getc(1, timeout) - if not char: + if char is None: + self.log.warn('recv error: getc timeout in start sequence') error_count += 1 continue elif char == SOH: - #crc_mode = 0 + self.log.debug('recv: SOH') break elif char == STX: + self.log.debug('recv: STX') break elif char == CAN: if cancel: - self.log.info('Transmission canceled: Received CAN twice.') + self.log.info('Transmission canceled: received 2xCAN ' + 'at start-sequence') return None else: + self.log.debug('cancellation at start sequence.') cancel = 1 else: error_count += 1 @@ -414,10 +445,14 @@ def recv(self, stream, crc_mode=1, retry=16, timeout=60, delay=1, quiet=0): while True: while True: if char == SOH: - packet_size = 128 + if packet_size != 128: + self.log.debug('recv: SOH, using 128b packet_size') + packet_size = 128 break elif char == STX: - packet_size = 1024 + if packet_size != 1024: + self.log.debug('recv: SOH, using 1k packet_size') + packet_size = 1024 break elif char == EOT: # We received an EOT, so send an ACK and return the @@ -429,15 +464,18 @@ def recv(self, stream, crc_mode=1, retry=16, timeout=60, delay=1, quiet=0): elif char == CAN: # cancel at two consecutive cancels if cancel: - self.log.info('Transmission canceled: ' - 'Received CAN twice.') + self.log.info('Transmission canceled: received 2xCAN ' + 'at block %d', sequence) return None else: + self.log.debug('cancellation at block %d', sequence) cancel = 1 else: + err_msg = ('recv error: expected SOH, EOT; ' + 'got {0!r}'.format(char)) if not quiet: - print >> sys.stderr, \ - 'recv ERROR expected SOH/EOT, got', ord(char) + print(err_msg, file=sys.stderr) + self.log.warn(err_msg) error_count += 1 if error_count >= retry: self.log.info('error_count reached %d, aborting.', @@ -448,34 +486,33 @@ def recv(self, stream, crc_mode=1, retry=16, timeout=60, delay=1, quiet=0): # read sequence error_count = 0 cancel = 0 + self.log.debug('recv: block sequence') seq1 = self.getc(1, timeout) if seq1 is None: - self.log.warning('getc failed to get first sequence byte') + self.log.warn('getc failed to get first sequence byte') seq2 = None else: seq1 = ord(seq1) seq2 = self.getc(1, timeout) if seq2 is None: - self.log.warning('getc failed to get second sequence byte') + self.log.warn('getc failed to get second sequence byte') else: seq2 = 0xff - ord(seq2) - if seq1 == sequence and seq2 == sequence: + if not (seq1 == seq2 == sequence): + # consume data anyway ... even though we will discard it, + # it is not the sequence we expected! + self.log.error('expected sequence %d, ' + 'got (seq1=%r, seq2=%r), ', + 'receiving next block, will NAK.', + sequence, seq1, seq2) + self.getc(packet_size + 1 + crc_mode) + else: # sequence is ok, read packet # packet_size + checksum + self.log.debug('reading data block %d', sequence) data = self.getc(packet_size + 1 + crc_mode, timeout) - if crc_mode: - csum = (ord(data[-2]) << 8) + ord(data[-1]) - data = data[:-2] - self.log.debug('CRC (%04x <> %04x)', - csum, self.calc_crc(data)) - valid = csum == self.calc_crc(data) - else: - csum = data[-1] - data = data[:-1] - self.log.debug('checksum (checksum(%02x <> %02x)', - ord(csum), self.calc_checksum(data)) - valid = ord(csum) == self.calc_checksum(data) + valid, data = self._verify_recv_checksum(crc_mode, data) # valid data, append chunk if valid: @@ -483,17 +520,40 @@ def recv(self, stream, crc_mode=1, retry=16, timeout=60, delay=1, quiet=0): stream.write(data) self.putc(ACK) sequence = (sequence + 1) % 0x100 + # get next byte char = self.getc(1, timeout) continue - else: - # consume data - self.getc(packet_size + 1 + crc_mode) - self.log.debug('expecting sequence %d, got %s/%s', - sequence, seq1, seq2) # something went wrong, request retransmission - self.log.warning('error in recv(), requesting retransmission') + self.log.warn('recv error: requesting retransmission (NAK)') self.putc(NAK) + continue + + def _verify_recv_checksum(self, crc_mode, data): + if crc_mode: + _checksum = bytearray(data[-2:]) + their_sum = (_checksum[0] << 8) + _checksum[1] + data = data[:-2] + + our_sum = self.calc_crc(data) + valid = bool(their_sum == our_sum) + if not valid: + self.log.warn('recv error: checksum fail ' + '(theirs=%04x, ours=%04x), ' + 'data=%r', + their_sum, our_sum) + else: + _checksum = bytearray(data[-1]) + their_sum = _checksum[0] + data = data[:-1] + + our_sum = self.calc_checksum(data) + valid = their_sum == our_sum + if not valid: + self.log.warn('recv error: checksum fail ' + '(theirs=%02x, ours=%02x)', + their_sum, our_sum) + return valid, data def calc_checksum(self, data, checksum=0): ''' @@ -506,7 +566,10 @@ def calc_checksum(self, data, checksum=0): '0x3c' ''' - return (sum(map(ord, data)) + checksum) % 256 + if platform.python_version_tuple() >= ('3', '0', '0'): + return (sum(data) + checksum) % 256 + else: + return (sum(map(ord, data)) + checksum) % 256 def calc_crc(self, data, crc=0): ''' @@ -519,8 +582,9 @@ def calc_crc(self, data, crc=0): '0xd5e3' ''' - for char in data: - crc = ((crc << 8) ^ self.crctable[((crc >> 8) ^ ord(char)) & 0xff]) & 0xffff + for char in bytearray(data): + crctbl_idx = ((crc >> 8) ^ char) & 0xff + crc = ((crc << 8) ^ self.crctable[crctbl_idx]) & 0xffff return crc & 0xffff @@ -548,8 +612,8 @@ def run(): def _func(so, si): import select - print 'si', si - print 'so', so + print(('si', si)) + print(('so', so)) def getc(size, timeout=3): read_ready, _, _ = select.select([so], [], [], timeout) @@ -558,7 +622,7 @@ def getc(size, timeout=3): else: data = None - print 'getc(', repr(data), ')' + print(('getc(', repr(data), ')')) return data def putc(data, timeout=3): @@ -570,7 +634,7 @@ def putc(data, timeout=3): else: size = None - print 'putc(', repr(data), repr(size), ')' + print(('putc(', repr(data), repr(size), ')')) return size return getc, putc From 999cc3e6bc0718fc0156d989084fc0baa29626f7 Mon Sep 17 00:00:00 2001 From: Jeff Quast Date: Sun, 28 Dec 2014 22:30:18 -0800 Subject: [PATCH 02/13] wire-in tox to Makefile 'tests' target --- Makefile | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Makefile b/Makefile index fa25e10..84700ea 100644 --- a/Makefile +++ b/Makefile @@ -1,7 +1,5 @@ tests: - PYTHONPATH=. python test/test.py test/test.py - PYTHONPATH=. python test/test-recv.py - PYTHONPATH=. python test/test-send.py + tox upload: python setup.py sdist upload From a4061e0ed20752475eeddb46e3b36a95a701dab5 Mon Sep 17 00:00:00 2001 From: Jeff Quast Date: Sun, 28 Dec 2014 22:36:12 -0800 Subject: [PATCH 03/13] FIX -- Too many arguments for logging format string --- xmodem/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xmodem/__init__.py b/xmodem/__init__.py index fe2a849..a75ae70 100644 --- a/xmodem/__init__.py +++ b/xmodem/__init__.py @@ -503,7 +503,7 @@ def recv(self, stream, crc_mode=1, retry=16, timeout=60, delay=1, quiet=0): # consume data anyway ... even though we will discard it, # it is not the sequence we expected! self.log.error('expected sequence %d, ' - 'got (seq1=%r, seq2=%r), ', + 'got (seq1=%r, seq2=%r), ' 'receiving next block, will NAK.', sequence, seq1, seq2) self.getc(packet_size + 1 + crc_mode) From 0483a00644065aba00faa3dda580d21f943cb0db Mon Sep 17 00:00:00 2001 From: Jeff Quast Date: Sun, 28 Dec 2014 22:36:38 -0800 Subject: [PATCH 04/13] Not enough arguments for logging format string --- xmodem/__init__.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/xmodem/__init__.py b/xmodem/__init__.py index a75ae70..adcd6c7 100644 --- a/xmodem/__init__.py +++ b/xmodem/__init__.py @@ -539,8 +539,7 @@ def _verify_recv_checksum(self, crc_mode, data): valid = bool(their_sum == our_sum) if not valid: self.log.warn('recv error: checksum fail ' - '(theirs=%04x, ours=%04x), ' - 'data=%r', + '(theirs=%04x, ours=%04x), ', their_sum, our_sum) else: _checksum = bytearray(data[-1]) From 3228e70d657649a53a9b6ec634ca6be383d82221 Mon Sep 17 00:00:00 2001 From: Jeff Quast Date: Sun, 28 Dec 2014 23:21:10 -0800 Subject: [PATCH 05/13] bugfix old-style checksum and add functional tests --- test/functional/accessories.py | 29 ++++++++ test/functional/test_xmodem.py | 117 +++++++++++++++++++++++---------- xmodem/__init__.py | 2 +- 3 files changed, 114 insertions(+), 34 deletions(-) create mode 100644 test/functional/accessories.py diff --git a/test/functional/accessories.py b/test/functional/accessories.py new file mode 100644 index 0000000..f271c52 --- /dev/null +++ b/test/functional/accessories.py @@ -0,0 +1,29 @@ +import subprocess + + +def _multi_which(prog_names): + for prog_name in prog_names: + proc = subprocess.Popen(('which', prog_name), stdout=subprocess.PIPE) + stdout, stderr = proc.communicate() + if proc.returncode == 0: + return stdout.strip() + return None + + +def _get_recv_program(): + bin_path = _multi_which(('rb', 'lrb')) + assert bin_path is not None, ( + "program required: {0!r}. " + "Try installing lrzsz package.".format(bin_path)) + return bin_path + + +def _get_send_program(): + bin_path = _multi_which(('sb', 'lsb')) + assert bin_path is not None, ( + "program required: {0!r}. " + "Try installing lrzsz package.".format(bin_path)) + return bin_path + +recv_prog = _get_recv_program() +send_prog = _get_send_program() diff --git a/test/functional/test_xmodem.py b/test/functional/test_xmodem.py index ee1b9c7..45a446d 100644 --- a/test/functional/test_xmodem.py +++ b/test/functional/test_xmodem.py @@ -1,8 +1,10 @@ +# std imports +from __future__ import print_function import os +import sys import errno import select import logging -import platform import tempfile import functools import subprocess @@ -12,53 +14,29 @@ except ImportError: # python 2 import StringIO.StringIO as BytesIO + +# local from xmodem import XMODEM, XMODEM1k +from .accessories import recv_prog, send_prog logging.basicConfig(format='%(levelname)-5s %(message)s', level=logging.DEBUG) -def _multi_which(prog_names): - for prog_name in prog_names: - proc = subprocess.Popen(('which', prog_name), stdout=subprocess.PIPE) - stdout, stderr = proc.communicate() - if proc.returncode == 0: - return stdout.strip() - return None - - -def _get_recv_program(): - bin_path = _multi_which(('rb', 'lrb')) - assert bin_path is not None, ( - "program required: {0!r}. " - "Try installing lrzsz package.".format(bin_path)) - return bin_path - - -def _get_send_program(): - bin_path = _multi_which(('sb', 'lsb')) - assert bin_path is not None, ( - "program required: {0!r}. " - "Try installing lrzsz package.".format(bin_path)) - return bin_path - -recv_prog = _get_recv_program() -send_prog = _get_send_program() +CHUNKSIZE = 521 def _fill_binary_data(stream): - chunksize = 521 for byte in range(0x00, 0xff + 1): - stream.write(bytearray([byte] * chunksize)) + stream.write(bytearray([byte] * CHUNKSIZE)) stream.seek(0) return stream def _verify_binary_data(stream, padding=b'\xff'): stream.seek(0) - chunksize = 521 for byte in range(0x00, 0xff + 1): - assert stream.read(chunksize) == bytearray([byte] * chunksize) + assert stream.read(CHUNKSIZE) == bytearray([byte] * CHUNKSIZE) while True: try: # BSD-style EOF @@ -103,6 +81,15 @@ def _proc_putc(data, timeout=1, proc=None): return len(data) +def _send_callback(total_packets, success_count, error_count): + # this simple callback simply asserts that no errors have occurred, and + # prints the given status to stderr. This is captured but displayed in + # py.test output only on error. + assert error_count == 0 + assert success_count == total_packets + print('{0}'.format(total_packets), file=sys.stderr) + + def test_xmodem_send(): """ Using external program for receive, verify XMODEM.send(). """ # Given, @@ -119,7 +106,7 @@ def test_xmodem_send(): stream = _fill_binary_data(BytesIO()) # Exercise, - status = xmodem.send(stream, timeout=5) + status = xmodem.send(stream, timeout=5, callback=_send_callback) # Verify, assert status is True @@ -179,7 +166,7 @@ def test_xmodem1k_send(): stream = _fill_binary_data(BytesIO()) # Exercise, - status = xmodem.send(stream, timeout=5) + status = xmodem.send(stream, timeout=5, callback=_send_callback) # Verify, assert status is True @@ -221,3 +208,67 @@ def test_xmodem1k_recv(): finally: if os.path.isfile(send_filename): os.unlink(send_filename) + + +def test_xmodem_send_16bit_crc(): + """ + Using external program for receive, verify XMODEM.send() with 16-bit CRC. + """ + # Given, + _, recv_filename = tempfile.mkstemp() + try: + proc = subprocess.Popen( + (recv_prog, '--xmodem', '--verbose', '--with-crc', recv_filename), + stdin=subprocess.PIPE, stdout=subprocess.PIPE, bufsize=0) + + getc = functools.partial(_proc_getc, proc=proc) + putc = functools.partial(_proc_putc, proc=proc) + + xmodem = XMODEM(getc, putc, pad=b'\xbb') + stream = _fill_binary_data(BytesIO()) + + # Exercise, + status = xmodem.send(stream, timeout=5, callback=_send_callback) + + # Verify, + assert status is True + _verify_binary_data(stream) + _verify_binary_data(open(recv_filename, 'rb'), padding=b'\xbb') + proc.wait() + assert proc.returncode == 0 + + finally: + if os.path.isfile(recv_filename): + os.unlink(recv_filename) + + +def test_xmodem_recv_oldstyle_checksum(): + """ + Using external program for send, verify XMODEM.recv() with crc_mode 0. + """ + # Given, + _, send_filename = tempfile.mkstemp() + try: + with open(send_filename, 'wb') as stream: + _fill_binary_data(stream) + proc = subprocess.Popen( + (send_prog, '--xmodem', '--verbose', send_filename), + stdin=subprocess.PIPE, stdout=subprocess.PIPE, bufsize=0) + + getc = functools.partial(_proc_getc, proc=proc) + putc = functools.partial(_proc_putc, proc=proc) + + xmodem = XMODEM(getc, putc, pad=b'\xbb') + recv_stream = BytesIO() + + # Exercise, + status = xmodem.recv(recv_stream, timeout=5, crc_mode=0) + + # Verify, + assert status == recv_stream.tell() + _verify_binary_data(recv_stream, padding=b'\xbb') + proc.wait() + assert proc.returncode == 0 + + finally: + os.unlink(send_filename) diff --git a/xmodem/__init__.py b/xmodem/__init__.py index adcd6c7..591289a 100644 --- a/xmodem/__init__.py +++ b/xmodem/__init__.py @@ -542,7 +542,7 @@ def _verify_recv_checksum(self, crc_mode, data): '(theirs=%04x, ours=%04x), ', their_sum, our_sum) else: - _checksum = bytearray(data[-1]) + _checksum = bytearray([data[-1]]) their_sum = _checksum[0] data = data[:-1] From 492043f06e110112748c78bd2a40afbf68d8a1ba Mon Sep 17 00:00:00 2001 From: Jeff Quast Date: Mon, 29 Dec 2014 00:00:16 -0800 Subject: [PATCH 06/13] make failing test cases for CRC issue #7 --- test/functional/accessories.py | 27 +++++++++ test/functional/test_failures.py | 101 +++++++++++++++++++++++++++++++ test/functional/test_xmodem.py | 71 ++++++++-------------- 3 files changed, 152 insertions(+), 47 deletions(-) create mode 100644 test/functional/test_failures.py diff --git a/test/functional/accessories.py b/test/functional/accessories.py index f271c52..0079469 100644 --- a/test/functional/accessories.py +++ b/test/functional/accessories.py @@ -1,4 +1,5 @@ import subprocess +import errno def _multi_which(prog_names): @@ -27,3 +28,29 @@ def _get_send_program(): recv_prog = _get_recv_program() send_prog = _get_send_program() + +CHUNKSIZE = 521 + + +def fill_binary_data(stream): + for byte in range(0x00, 0xff + 1, 10): + stream.write(bytearray([byte] * CHUNKSIZE)) + stream.seek(0) + return stream + + +def verify_binary_data(stream, padding): + stream.seek(0) + for byte in range(0x00, 0xff + 1, 10): + assert stream.read(CHUNKSIZE) == bytearray([byte] * CHUNKSIZE) + while True: + try: + # BSD-style EOF + data = stream.read(1) + assert data in (b'', padding) + if data == b'': + # BSD-style EOF + break + except OSError as err: + # Linux-style EOF + assert err.errno == errno.EIO diff --git a/test/functional/test_failures.py b/test/functional/test_failures.py new file mode 100644 index 0000000..15420ca --- /dev/null +++ b/test/functional/test_failures.py @@ -0,0 +1,101 @@ +import subprocess +import functools +import tempfile +import select +import logging +import os + +try: + # python 3 + from io import BytesIO +except ImportError: + # python 2 + import StringIO.StringIO as BytesIO + +from .accessories import ( + # recv_prog, + send_prog, + fill_binary_data, + verify_binary_data, +) +from xmodem import XMODEM, XMODEM1k + +logging.basicConfig(format='%(levelname)-5s %(message)s', + level=logging.DEBUG) + + +CHECKFAIL_SEQ = 0 + + +def _proc_getc_fail_16bit_checksum(size, timeout=1, proc=None, num_failures=0): + # our getc function simply pipes to the standard out of the `rb' + # or `lrb' program -- any data written by such program is returned + # by our getc() callback. + global CHECKFAIL_SEQ + assert proc.returncode is None, ("{0} has exited: (returncode={1})" + .format(proc, proc.returncode)) + logging.debug(('get', size)) + ready_read, _, _ = select.select([proc.stdout], [], [], timeout) + if not ready_read: + assert False, ("Timeout on stdout of {0}.".format(proc)) + data = proc.stdout.read(size) + if len(data) == 130 and CHECKFAIL_SEQ < num_failures: + CHECKFAIL_SEQ += 1 + _checksum = bytearray(data[-2:]) + data = data[:-2] + _checksum[0] |= 0xb0 + _checksum[1] |= 0x0b + data += chr(_checksum[0]) + chr(_checksum[1]) + logging.debug(('got', len(data), data)) + return data + + +def _proc_putc(data, timeout=1, proc=None): + # similarly, our putc function simply writes to the standard of + # our `rb' or `lrb' program -- any data written by our XMODEM + # protocol via putc() callback is written to the stdin of such + # program. + assert proc.returncode is None, ("{0} has exited: (returncode={1})" + .format(proc, proc.returncode)) + _, ready_write, _ = select.select([], [proc.stdin], [], timeout) + if not ready_write: + assert False, ("Timeout on stdin of {0}.".format(proc)) + logging.debug(('put', len(data), data)) + proc.stdin.write(data) + proc.stdin.flush() + return len(data) + + +def test_xmodem_recv_bad_checksum(): + """ + Using external program for send, verify checksum fail in XMODEM.recv(). + """ + # Given, + _, send_filename = tempfile.mkstemp() + try: + with open(send_filename, 'wb') as stream: + fill_binary_data(stream) + proc = subprocess.Popen( + (send_prog, '--xmodem', '--verbose', send_filename), + stdin=subprocess.PIPE, stdout=subprocess.PIPE, bufsize=0) + + getc = functools.partial(_proc_getc_fail_16bit_checksum, + proc=proc, num_failures=5) + putc = functools.partial(_proc_putc, proc=proc) + + xmodem = XMODEM(getc, putc) + recv_stream = BytesIO() + + # Exercise, + status = xmodem.recv(recv_stream, timeout=5, crc_mode=1) + + # Verify, + assert status == recv_stream.tell() + verify_binary_data(recv_stream, padding=b'\x1a') + proc.wait() + assert proc.returncode == 0 + + finally: + os.unlink(send_filename) + +# assert False diff --git a/test/functional/test_xmodem.py b/test/functional/test_xmodem.py index 45a446d..9b1d6fc 100644 --- a/test/functional/test_xmodem.py +++ b/test/functional/test_xmodem.py @@ -2,7 +2,6 @@ from __future__ import print_function import os import sys -import errno import select import logging import tempfile @@ -17,39 +16,17 @@ # local from xmodem import XMODEM, XMODEM1k -from .accessories import recv_prog, send_prog +from .accessories import ( + recv_prog, + send_prog, + fill_binary_data, + verify_binary_data, +) logging.basicConfig(format='%(levelname)-5s %(message)s', level=logging.DEBUG) -CHUNKSIZE = 521 - - -def _fill_binary_data(stream): - for byte in range(0x00, 0xff + 1): - stream.write(bytearray([byte] * CHUNKSIZE)) - stream.seek(0) - return stream - - -def _verify_binary_data(stream, padding=b'\xff'): - stream.seek(0) - for byte in range(0x00, 0xff + 1): - assert stream.read(CHUNKSIZE) == bytearray([byte] * CHUNKSIZE) - while True: - try: - # BSD-style EOF - data = stream.read(1) - assert data in (b'', padding) - if data == b'': - # BSD-style EOF - break - except OSError as err: - # Linux-style EOF - assert err.errno == errno.EIO - - def _proc_getc(size, timeout=1, proc=None): # our getc function simply pipes to the standard out of the `rb' # or `lrb' program -- any data written by such program is returned @@ -103,15 +80,15 @@ def test_xmodem_send(): putc = functools.partial(_proc_putc, proc=proc) xmodem = XMODEM(getc, putc, pad=b'\xbb') - stream = _fill_binary_data(BytesIO()) + stream = fill_binary_data(BytesIO()) # Exercise, status = xmodem.send(stream, timeout=5, callback=_send_callback) # Verify, assert status is True - _verify_binary_data(stream) - _verify_binary_data(open(recv_filename, 'rb'), padding=b'\xbb') + verify_binary_data(stream, padding=b'\xbb') + verify_binary_data(open(recv_filename, 'rb'), padding=b'\xbb') proc.wait() assert proc.returncode == 0 @@ -126,7 +103,7 @@ def test_xmodem_recv(): _, send_filename = tempfile.mkstemp() try: with open(send_filename, 'wb') as stream: - _fill_binary_data(stream) + fill_binary_data(stream) proc = subprocess.Popen( (send_prog, '--xmodem', '--verbose', send_filename), stdin=subprocess.PIPE, stdout=subprocess.PIPE, bufsize=0) @@ -134,7 +111,7 @@ def test_xmodem_recv(): getc = functools.partial(_proc_getc, proc=proc) putc = functools.partial(_proc_putc, proc=proc) - xmodem = XMODEM(getc, putc, pad=b'\xbb') + xmodem = XMODEM(getc, putc) recv_stream = BytesIO() # Exercise, @@ -142,7 +119,7 @@ def test_xmodem_recv(): # Verify, assert status == recv_stream.tell() - _verify_binary_data(recv_stream, padding=b'\xbb') + verify_binary_data(recv_stream, padding=b'\x1a') proc.wait() assert proc.returncode == 0 @@ -163,15 +140,15 @@ def test_xmodem1k_send(): putc = functools.partial(_proc_putc, proc=proc) xmodem = XMODEM1k(getc, putc, pad=b'\xbb') - stream = _fill_binary_data(BytesIO()) + stream = fill_binary_data(BytesIO()) # Exercise, status = xmodem.send(stream, timeout=5, callback=_send_callback) # Verify, assert status is True - _verify_binary_data(stream) - _verify_binary_data(open(recv_filename, 'rb'), padding=b'\xbb') + verify_binary_data(stream, padding=b'\xbb') + verify_binary_data(open(recv_filename, 'rb'), padding=b'\xbb') proc.wait() assert proc.returncode == 0 @@ -185,7 +162,7 @@ def test_xmodem1k_recv(): _, send_filename = tempfile.mkstemp() try: with open(send_filename, 'wb') as stream: - _fill_binary_data(stream) + fill_binary_data(stream) proc = subprocess.Popen( (send_prog, '--xmodem', '--verbose', '--1k', send_filename), stdin=subprocess.PIPE, stdout=subprocess.PIPE, bufsize=0) @@ -193,7 +170,7 @@ def test_xmodem1k_recv(): getc = functools.partial(_proc_getc, proc=proc) putc = functools.partial(_proc_putc, proc=proc) - xmodem = XMODEM1k(getc, putc, pad=b'\xbb') + xmodem = XMODEM1k(getc, putc) recv_stream = BytesIO() # Exercise, @@ -201,7 +178,7 @@ def test_xmodem1k_recv(): # Verify, assert status == recv_stream.tell() - _verify_binary_data(recv_stream, padding=b'\xbb') + verify_binary_data(recv_stream, padding=b'\x1a') proc.wait() assert proc.returncode == 0 @@ -225,15 +202,15 @@ def test_xmodem_send_16bit_crc(): putc = functools.partial(_proc_putc, proc=proc) xmodem = XMODEM(getc, putc, pad=b'\xbb') - stream = _fill_binary_data(BytesIO()) + stream = fill_binary_data(BytesIO()) # Exercise, status = xmodem.send(stream, timeout=5, callback=_send_callback) # Verify, assert status is True - _verify_binary_data(stream) - _verify_binary_data(open(recv_filename, 'rb'), padding=b'\xbb') + verify_binary_data(stream, padding=b'\xbb') + verify_binary_data(open(recv_filename, 'rb'), padding=b'\xbb') proc.wait() assert proc.returncode == 0 @@ -250,7 +227,7 @@ def test_xmodem_recv_oldstyle_checksum(): _, send_filename = tempfile.mkstemp() try: with open(send_filename, 'wb') as stream: - _fill_binary_data(stream) + fill_binary_data(stream) proc = subprocess.Popen( (send_prog, '--xmodem', '--verbose', send_filename), stdin=subprocess.PIPE, stdout=subprocess.PIPE, bufsize=0) @@ -258,7 +235,7 @@ def test_xmodem_recv_oldstyle_checksum(): getc = functools.partial(_proc_getc, proc=proc) putc = functools.partial(_proc_putc, proc=proc) - xmodem = XMODEM(getc, putc, pad=b'\xbb') + xmodem = XMODEM(getc, putc) recv_stream = BytesIO() # Exercise, @@ -266,7 +243,7 @@ def test_xmodem_recv_oldstyle_checksum(): # Verify, assert status == recv_stream.tell() - _verify_binary_data(recv_stream, padding=b'\xbb') + verify_binary_data(recv_stream, padding=b'\x1a') proc.wait() assert proc.returncode == 0 From 71c9def866fb84836e824c305db4878aac721ad0 Mon Sep 17 00:00:00 2001 From: Jeff Quast Date: Mon, 29 Dec 2014 00:00:47 -0800 Subject: [PATCH 07/13] ignore /build --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index affae6e..ba04aac 100644 --- a/.gitignore +++ b/.gitignore @@ -2,6 +2,7 @@ /.pydevproject *.pyc /dist +/build /xmodem.egg-info /.coverage /.tox From b6b173d25d521f990fe0994efd5b9883cf3a8e3b Mon Sep 17 00:00:00 2001 From: Jeff Quast Date: Mon, 29 Dec 2014 00:01:45 -0800 Subject: [PATCH 08/13] Fix missing document links in https://pythonhosted.org/xmodem/xmodem.html URLS at the top for the TXT documents fail, because the docs/ are not provided with a source distribution --- setup.py | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/setup.py b/setup.py index 24ca1a8..aa51269 100644 --- a/setup.py +++ b/setup.py @@ -1,11 +1,11 @@ from setuptools import setup, find_packages setup( - name = 'xmodem', - version = '0.3.3', - author = 'Wijnand Modderman', - author_email = 'maze@pyth0n.org', - description = ('XMODEM protocol implementation.'), + name='xmodem', + version='0.3.3', + author='Wijnand Modderman', + author_email='maze@pyth0n.org', + description=('XMODEM protocol implementation.'), long_description = ''' ================================ XMODEM protocol implementation @@ -45,9 +45,16 @@ .. _documentation: http://packages.python.org/xmodem/xmodem.html ''', - license = 'MIT', - keywords = 'xmodem protocol', - packages = ['xmodem'], - package_data = {'': ['doc/*.TXT']}, + license='MIT', + keywords='xmodem protocol', + packages=['xmodem'], + package_data={'': ['doc/*.TXT', 'doc/*.txt']}, + include_package_data=True, + data_files=[ + ('doc', ('doc/XMODEM.TXT', + 'doc/XMODEM1K.TXT', + 'doc/XMODMCRC.TXT', + 'doc/ymodem.txt')) + ], ) From a14caadf5a0ad054e9f8754888568f5a63745dc2 Mon Sep 17 00:00:00 2001 From: Jeff Quast Date: Mon, 29 Dec 2014 00:04:47 -0800 Subject: [PATCH 09/13] coveralls support --- .travis.yml | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index e97cf29..7520ce9 100644 --- a/.travis.yml +++ b/.travis.yml @@ -8,12 +8,15 @@ env: - TOXENV=py34 install: - - pip install -q tox - - sudo apt-get install lrzsz + - pip install -q tox coveralls + - sudo apt-get install -qq -y lrzsz script: - tox -e $TOXENV +after_success: + - coveralls + notifications: email: - contact@jeffquast.com From 9b03fc201a78302e0c128009a4f8347fe56eb892 Mon Sep 17 00:00:00 2001 From: Jeff Quast Date: Mon, 29 Dec 2014 00:44:28 -0800 Subject: [PATCH 10/13] Bugfix issue #7 on crc checksum failure When a CRC checksum failure occurs and NAK is sent, 1. we should purge any awaiting stdin as suggested by ymodem.txt and documented therein, i would suppose this may only be for ymodem, which has a kind of readahead buffer, though. For xmodem local testing piped with lrzsz, no data is purged. 2. we should read the next start-of-header block, as is usually done when the checksum is valid -- this causes our 'seq1' to be SOH or STX, and seq2 fails 1's complement and subsequent failures about unexpected sequence occur, otherwise. --- test/functional/test_failures.py | 51 ++++++++++++++++++++++++-------- test/functional/test_xmodem.py | 4 +-- xmodem/__init__.py | 23 +++++++++++--- 3 files changed, 60 insertions(+), 18 deletions(-) diff --git a/test/functional/test_failures.py b/test/functional/test_failures.py index 15420ca..7f9d0ec 100644 --- a/test/functional/test_failures.py +++ b/test/functional/test_failures.py @@ -1,8 +1,8 @@ import subprocess import functools import tempfile -import select import logging +import select import os try: @@ -27,25 +27,23 @@ CHECKFAIL_SEQ = 0 -def _proc_getc_fail_16bit_checksum(size, timeout=1, proc=None, num_failures=0): - # our getc function simply pipes to the standard out of the `rb' - # or `lrb' program -- any data written by such program is returned - # by our getc() callback. +def _proc_getc_fail_16bit_checksum(size, timeout=1, proc=None, num_failures=1): + # this getc function adjusts the final checksum of the first block to be + # failed, causing a purge and retransmission of the first block. global CHECKFAIL_SEQ assert proc.returncode is None, ("{0} has exited: (returncode={1})" .format(proc, proc.returncode)) logging.debug(('get', size)) ready_read, _, _ = select.select([proc.stdout], [], [], timeout) - if not ready_read: - assert False, ("Timeout on stdout of {0}.".format(proc)) + if proc.stdout not in ready_read: + return None data = proc.stdout.read(size) if len(data) == 130 and CHECKFAIL_SEQ < num_failures: CHECKFAIL_SEQ += 1 _checksum = bytearray(data[-2:]) - data = data[:-2] _checksum[0] |= 0xb0 _checksum[1] |= 0x0b - data += chr(_checksum[0]) + chr(_checksum[1]) + data = bytearray(data[:-2]) + _checksum logging.debug(('got', len(data), data)) return data @@ -79,8 +77,7 @@ def test_xmodem_recv_bad_checksum(): (send_prog, '--xmodem', '--verbose', send_filename), stdin=subprocess.PIPE, stdout=subprocess.PIPE, bufsize=0) - getc = functools.partial(_proc_getc_fail_16bit_checksum, - proc=proc, num_failures=5) + getc = functools.partial(_proc_getc_fail_16bit_checksum, proc=proc) putc = functools.partial(_proc_putc, proc=proc) xmodem = XMODEM(getc, putc) @@ -98,4 +95,34 @@ def test_xmodem_recv_bad_checksum(): finally: os.unlink(send_filename) -# assert False + +def test_xmodem1k_recv_bad_checksum(): + """ + Using external program for send, verify checksum fail in XMODEM.recv(). + """ + # Given, + _, send_filename = tempfile.mkstemp() + try: + with open(send_filename, 'wb') as stream: + fill_binary_data(stream) + proc = subprocess.Popen( + (send_prog, '--xmodem', '--verbose', send_filename), + stdin=subprocess.PIPE, stdout=subprocess.PIPE, bufsize=0) + + getc = functools.partial(_proc_getc_fail_16bit_checksum, proc=proc) + putc = functools.partial(_proc_putc, proc=proc) + + xmodem = XMODEM1k(getc, putc) + recv_stream = BytesIO() + + # Exercise, + status = xmodem.recv(recv_stream, timeout=5, crc_mode=1) + + # Verify, + assert status == recv_stream.tell() + verify_binary_data(recv_stream, padding=b'\x1a') + proc.wait() + assert proc.returncode == 0 + + finally: + os.unlink(send_filename) diff --git a/test/functional/test_xmodem.py b/test/functional/test_xmodem.py index 9b1d6fc..7bd8de6 100644 --- a/test/functional/test_xmodem.py +++ b/test/functional/test_xmodem.py @@ -35,7 +35,7 @@ def _proc_getc(size, timeout=1, proc=None): .format(proc, proc.returncode)) logging.debug(('get', size)) ready_read, _, _ = select.select([proc.stdout], [], [], timeout) - if not ready_read: + if proc.stdout not in ready_read: assert False, ("Timeout on stdout of {0}.".format(proc)) data = proc.stdout.read(size) logging.debug(('got', len(data), data)) @@ -50,7 +50,7 @@ def _proc_putc(data, timeout=1, proc=None): assert proc.returncode is None, ("{0} has exited: (returncode={1})" .format(proc, proc.returncode)) _, ready_write, _ = select.select([], [proc.stdin], [], timeout) - if not ready_write: + if proc.stdin not in ready_write: assert False, ("Timeout on stdin of {0}.".format(proc)) logging.debug(('put', len(data), data)) proc.stdin.write(data) diff --git a/xmodem/__init__.py b/xmodem/__init__.py index 591289a..d9ae5bb 100644 --- a/xmodem/__init__.py +++ b/xmodem/__init__.py @@ -486,7 +486,7 @@ def recv(self, stream, crc_mode=1, retry=16, timeout=60, delay=1, quiet=0): # read sequence error_count = 0 cancel = 0 - self.log.debug('recv: block sequence') + self.log.debug('recv: data block %d', sequence) seq1 = self.getc(1, timeout) if seq1 is None: self.log.warn('getc failed to get first sequence byte') @@ -497,6 +497,7 @@ def recv(self, stream, crc_mode=1, retry=16, timeout=60, delay=1, quiet=0): if seq2 is None: self.log.warn('getc failed to get second sequence byte') else: + # second byte is the same as first as 1's complement seq2 = 0xff - ord(seq2) if not (seq1 == seq2 == sequence): @@ -510,7 +511,6 @@ def recv(self, stream, crc_mode=1, retry=16, timeout=60, delay=1, quiet=0): else: # sequence is ok, read packet # packet_size + checksum - self.log.debug('reading data block %d', sequence) data = self.getc(packet_size + 1 + crc_mode, timeout) valid, data = self._verify_recv_checksum(crc_mode, data) @@ -520,13 +520,28 @@ def recv(self, stream, crc_mode=1, retry=16, timeout=60, delay=1, quiet=0): stream.write(data) self.putc(ACK) sequence = (sequence + 1) % 0x100 - # get next byte + # get next start-of-header byte char = self.getc(1, timeout) continue # something went wrong, request retransmission - self.log.warn('recv error: requesting retransmission (NAK)') + self.log.warn('recv error: purge, requesting retransmission (NAK)') + while True: + # When the receiver wishes to , it should call a "PURGE" + # subroutine, to wait for the line to clear. Recall the sender + # tosses any characters in its UART buffer immediately upon + # completing sending a block, to ensure no glitches were mis- + # interpreted. The most common technique is for "PURGE" to + # call the character receive subroutine, specifying a 1-second + # timeout, and looping back to PURGE until a timeout occurs. + # The is then sent, ensuring the other end will see it. + data = self.getc(1, timeout=1) + if data is None: + break + assert False, data self.putc(NAK) + # get next start-of-header byte + char = self.getc(1, timeout) continue def _verify_recv_checksum(self, crc_mode, data): From 75af6ba9b59f39e9c12013c04f767b4f15644ea7 Mon Sep 17 00:00:00 2001 From: Jeff Quast Date: Mon, 29 Dec 2014 01:06:18 -0800 Subject: [PATCH 11/13] bump version, add changes section, add self as author :) --- README.rst | 17 ++++++++++---- doc/source/conf.py | 4 ++-- setup.py | 58 +++++++++------------------------------------- 3 files changed, 26 insertions(+), 53 deletions(-) diff --git a/README.rst b/README.rst index e430b10..27bbc19 100644 --- a/README.rst +++ b/README.rst @@ -1,3 +1,9 @@ +.. image:: https://travis-ci.org/tehmaze/xmodem.png?branch=master + :target: https://travis-ci.org/tehmaze/xmodem + +.. image:: https://coveralls.io/repos/tehmaze/xmodem/badge.png + :target: https://coveralls.io/r/tehmaze/xmodem + ================================ XMODEM protocol implementation ================================ @@ -35,8 +41,11 @@ For more information, take a look at the documentation_. .. _documentation: http://packages.python.org/xmodem/xmodem.html -.. image:: https://travis-ci.org/tehmaze/xmodem.png?branch=master - :target: https://travis-ci.org/tehmaze/xmodem +Changes +======= -.. image:: https://coveralls.io/repos/tehmaze/xmodem/badge.png - :target: https://coveralls.io/r/tehmaze/xmodem +0.4.0 + * enhancement: support for python 3 + `PR #8 `_. + * bugfix: CRC failures in XMODEM.recv() were not renegotiated correctly + `PR #11 `_. diff --git a/doc/source/conf.py b/doc/source/conf.py index 250a7ba..2b656c9 100644 --- a/doc/source/conf.py +++ b/doc/source/conf.py @@ -49,9 +49,9 @@ # built documents. # # The short X.Y version. -version = '0.3' +version = '0.4' # The full version, including alpha/beta/rc tags. -release = '0.3.3' +release = '0.4.0' # The language for content autogenerated by Sphinx. Refer to documentation # for a list of supported languages. diff --git a/setup.py b/setup.py index aa51269..45460cd 100644 --- a/setup.py +++ b/setup.py @@ -1,60 +1,24 @@ -from setuptools import setup, find_packages +import os +from setuptools import setup + +HERE = os.path.dirname(__file__) setup( name='xmodem', - version='0.3.3', - author='Wijnand Modderman', + version='0.4.0', + author='Wijnand Modderman, Jeff Quast', author_email='maze@pyth0n.org', description=('XMODEM protocol implementation.'), - long_description = ''' -================================ - XMODEM protocol implementation -================================ - -Documentation available at http://packages.python.org/xmodem/ - -The source code repository is at https://github.com/tehmaze/xmodem - -Usage -===== - -Create a function to get and put character data (to a serial line for -example):: - - >>> from xmodem import XMODEM - >>> def getc(size, timeout=1): - ... return data or None - ... - >>> def putc(data, timeout=1): - ... return size or None - ... - >>> modem = XMODEM(getc, putc) - -Now, to upload a file, use the ``send`` method:: - - >>> stream = open('/etc/fstab', 'rb') - >>> modem.send(stream) - -To download a file, use the ``recv`` method:: - - >>> stream = open('output', 'wb') - >>> modem.recv(stream) - -For more information, take a look at the documentation_. - -.. _documentation: http://packages.python.org/xmodem/xmodem.html - -''', + long_description = open(os.path.join(HERE, 'README.rst'), 'rb').read(), license='MIT', keywords='xmodem protocol', packages=['xmodem'], - package_data={'': ['doc/*.TXT', 'doc/*.txt']}, + package_data={'': ['doc/*.TXT', 'doc/*.txt', 'README.rst']}, include_package_data=True, data_files=[ ('doc', ('doc/XMODEM.TXT', - 'doc/XMODEM1K.TXT', - 'doc/XMODMCRC.TXT', - 'doc/ymodem.txt')) + 'doc/XMODEM1K.TXT', + 'doc/XMODMCRC.TXT', + 'doc/ymodem.txt')), ], ) - From 471e4dfdb8984f0ebb0d7fc871ecc10a7070547f Mon Sep 17 00:00:00 2001 From: Jeff Quast Date: Mon, 29 Dec 2014 01:12:08 -0800 Subject: [PATCH 12/13] use codecs.open for bytes/str py2/3 read() issue --- setup.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 45460cd..50eecb7 100644 --- a/setup.py +++ b/setup.py @@ -1,7 +1,9 @@ import os +import codecs from setuptools import setup HERE = os.path.dirname(__file__) +README_RST = os.path.join(HERE, 'README.rst') setup( name='xmodem', @@ -9,7 +11,7 @@ author='Wijnand Modderman, Jeff Quast', author_email='maze@pyth0n.org', description=('XMODEM protocol implementation.'), - long_description = open(os.path.join(HERE, 'README.rst'), 'rb').read(), + long_description = codecs.open(README_RST, 'rb', 'utf8').read(), license='MIT', keywords='xmodem protocol', packages=['xmodem'], From 42b026cb9f34a6df9d89ccb76cdeb2a444ca6785 Mon Sep 17 00:00:00 2001 From: Jeff Quast Date: Mon, 29 Dec 2014 09:52:12 -0800 Subject: [PATCH 13/13] another version --- xmodem/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xmodem/__init__.py b/xmodem/__init__.py index d9ae5bb..48ee70e 100644 --- a/xmodem/__init__.py +++ b/xmodem/__init__.py @@ -112,7 +112,7 @@ __copyright__ = ['Copyright (c) 2010 Wijnand Modderman', 'Copyright (c) 1981 Chuck Forsberg'] __license__ = 'MIT' -__version__ = '0.3.3' +__version__ = '0.4.0' import platform import logging