diff --git a/.gitignore b/.gitignore index ba04aac..b8fd992 100644 --- a/.gitignore +++ b/.gitignore @@ -6,4 +6,6 @@ /xmodem.egg-info /.coverage /.tox +/.cache +/.idea *.py.swp diff --git a/README.rst b/README.rst index d1fd59e..964fd78 100644 --- a/README.rst +++ b/README.rst @@ -44,6 +44,16 @@ For more information, take a look at the documentation_. Changes ======= +0.4.4: + * bugfix: Large file transfers in ``send()`` were more likely to fail for + small values of ``retry``: This value should be the maximum failures per + block transfer as documented, but was improperly implemented as the number + of failures allowed for the total duration of the transfer, `PR #21 + `_. + * bugfix: ``send(retry=n)`` and ``recv(retry=n)`` would not retry ``n-1`` + times, rather than ``n`` times as documented, causing 'retry=1' to never + retry, for example. + 0.4.3: * bugfix: ``putc()`` callback was called in series, 3 times for each part of xmodem block header, data, and checksum during block transfer. Now all @@ -57,7 +67,7 @@ Changes `Issue #16 `_. 0.4.1 - * bugfix: re-transmit in send() on NAK or timeout, previously + * bugfix: re-transmit in ``send()`` on ``NAK`` or timeout, previously re-transmissions (wrongly) occurred only on garbage bytes. `PR #12 `_. diff --git a/setup.py b/setup.py index eb6411a..b92edea 100644 --- a/setup.py +++ b/setup.py @@ -7,8 +7,8 @@ setup( name='xmodem', - version='0.4.3', - author='Wijnand Modderman, Jeff Quast', + version='0.4.4', + author='Wijnand Modderman, Jeff Quast, Kris Hardy', author_email='maze@pyth0n.org', description=('XMODEM protocol implementation.'), url='https://github.com/tehmaze/xmodem', diff --git a/test/unit/test_xmodem.py b/test/unit/test_xmodem.py index 3b724d5..125bc29 100644 --- a/test/unit/test_xmodem.py +++ b/test/unit/test_xmodem.py @@ -7,10 +7,10 @@ from io import BytesIO except ImportError: # python 2 - import StringIO.StringIO as BytesIO + from StringIO import StringIO as BytesIO # local -import xmodem +from xmodem import NAK, CRC, ACK, XMODEM # 3rd-party import pytest @@ -24,24 +24,95 @@ def dummy_putc(data, timeout=1): return 0 +def test_xmodem_bad_mode(): + # given, + mode = 'XXX' + modem = XMODEM(getc=dummy_getc, putc=dummy_putc, mode=mode) + # exercise + with pytest.raises(ValueError): + status = modem.send(BytesIO(b'dummy-stream')) + + @pytest.mark.parametrize('mode', ['xmodem', 'xmodem1k']) def test_xmodem_dummy_fails_send(mode): # given, - modem = xmodem.XMODEM(getc=dummy_getc, - putc=dummy_putc, - mode=mode) + modem = XMODEM(getc=dummy_getc, putc=dummy_putc, mode=mode) # exercise status = modem.send(BytesIO(b'dummy-stream')) # verify assert not status, ("Expected value of status `False'") -def test_xmodem_bad_mode(): +@pytest.mark.parametrize('mode', ['xmodem', 'xmodem1k']) +@pytest.mark.parametrize('stream_data', [BytesIO(b'dummy-stream ' * 17), + BytesIO(b'dummy-stream ' * 1000)]) +def test_xmodem_send_exceed_maximum_number_of_resend(mode, stream_data): + """ Verify send(retry=n) after 'n' transfer failures of single block. """ + # given, - mode = 'XXX' - modem = xmodem.XMODEM(getc=dummy_getc, - putc=dummy_putc, - mode=mode) + max_resend = 3 + + def getc_generator(): + if mode == 'xmodem': + yield NAK + else: + # xmodem1k + yield CRC + + if mode == 'xmodem': + yield ACK + + for i in range(max_resend + 1): + yield None + + while True: + yield ACK + + mock = getc_generator() + + def mock_getc(size, timeout=1): + return next(mock) + + xmodem = XMODEM(getc=mock_getc, putc=dummy_putc, mode=mode) + # exercise - with pytest.raises(ValueError): - status = modem.send(BytesIO(b'dummy-stream')) + result = xmodem.send(stream=stream_data, retry=max_resend) + + # verify + assert not result + + +@pytest.mark.parametrize('mode', ['xmodem', 'xmodem1k']) +@pytest.mark.parametrize('stream_data', [BytesIO(b'dummy-stream ' * 17), + BytesIO(b'dummy-stream ' * 1000)]) +def test_xmodem_send_fails_once_each_packet(mode, stream_data): + """ Verify send(retry=n) under 'n' transfer failures of single block. """ + # given, + max_resend = 1 + + def getc_generator(): + if mode == 'xmodem': + yield NAK + else: + # xmodem1k + yield CRC + + while True: + # fail + yield None + + # succeed + yield ACK + + mock = getc_generator() + + def mock_getc(size, timeout=1): + return next(mock) + + xmodem = XMODEM(getc=mock_getc, putc=dummy_putc, mode=mode) + + # exercise + result = xmodem.send(stream=stream_data, retry=max_resend) + + # verify + assert result diff --git a/xmodem/__init__.py b/xmodem/__init__.py index 0ed7500..4822bdc 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.4.3' +__version__ = '0.4.4' import platform import logging @@ -210,7 +210,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') + >>> stream = open('/etc/issue', 'rb') >>> print(modem.send(stream)) True @@ -276,7 +276,7 @@ def callback(total_packets, success_count, error_count) 'got %r', char) error_count += 1 - if error_count >= retry: + if error_count > retry: self.log.info('send error: error_count reached %d, ' 'aborting.', retry) self.abort(timeout=timeout) @@ -308,6 +308,7 @@ def callback(total_packets, success_count, error_count) success_count += 1 if callable(callback): callback(total_packets, success_count, error_count) + error_count = 0 break self.log.error('send error: expected ACK; got %r for block %d', @@ -315,7 +316,7 @@ def callback(total_packets, success_count, error_count) error_count += 1 if callable(callback): callback(total_packets, success_count, error_count) - if error_count >= retry: + if error_count > retry: # excessive amounts of retransmissions requested, # abort transfer self.log.error('send error: NAK received %d times, ' @@ -338,7 +339,7 @@ def callback(total_packets, success_count, error_count) else: self.log.error('send error: expected ACK; got %r', char) error_count += 1 - if error_count >= retry: + if error_count > retry: self.log.warn('EOT was not ACKd, aborting transfer') self.abort(timeout=timeout) return False @@ -370,7 +371,7 @@ 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') + >>> stream = open('/etc/issue', 'wb') >>> print(modem.recv(stream)) 2342 @@ -466,7 +467,7 @@ def recv(self, stream, crc_mode=1, retry=16, timeout=60, delay=1, quiet=0): print(err_msg, file=sys.stderr) self.log.warn(err_msg) error_count += 1 - if error_count >= retry: + if error_count > retry: self.log.info('error_count reached %d, aborting.', retry) self.abort()