hamishcunningham/pi-tronics — pull request #3 by stoduk (2014-05-23 18:01:45+00:00)
Title: Fix bug in retry handling: if all retries fail then error is swallowed

Moved to using a decorator to add the retry - should be clearer what is happening.

I had to make some changes to the retry decorator, which I've pushed upstream to https://github.com/saltycrane/retry-decorator

Some notes:
- I tested this fully with my UT (which isn't publically committed) - logs below
- I tested the changes I need to "retry" decorator separately, see the pull request at https://github.com/saltycrane/retry-decorator/pull/2
- it requires adding a new file - I've copied what I could see for other files (basically add the file, and the symlink in debian/). Is there any more to it than this?

Testing with the UT stub that I added, this is what is causing all the DataPacket() output, which can be ignored (it comes from the test code).. See comments inline
```
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ ./mopicli -sv
DataPacket(op='r', register=9, data=None, block=False) 773
DataPacket(op='r', register=0, data=None, block=False) 7
Verbose status:
Battery #1 active
Battery #2 active
Battery full (blue led)
NiMH battery profile
Battery #1 low/not present
Battery #2 low/not present
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ ./mopicli -ron
DataPacket(op='r', register=9, data=None, block=False) 773
DataPacket(op='r', register=3, data=None, block=False) 65535
DataPacket(op='r', register=3, data=None, block=False) 65535
DataPacket(op='r', register=3, data=None, block=False) 65535
DataPacket(op='r', register=3, data=None, block=False) 0
Power on delay: 0 seconds
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ # read of single value - retry then suceed
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ ./mopicli -won 1
DataPacket(op='r', register=9, data=None, block=False) 773
DataPacket(op='w', register=3, data=1, block=False) 0
DataPacket(op='r', register=3, data=None, block=False) 65535
DataPacket(op='r', register=3, data=None, block=False) 65535
DataPacket(op='r', register=3, data=None, block=False) 65535
DataPacket(op='r', register=3, data=None, block=False) 0
DataPacket(op='w', register=3, data=1, block=False) 0
DataPacket(op='r', register=3, data=None, block=False) 0
DataPacket(op='w', register=3, data=1, block=False) 0
DataPacket(op='r', register=3, data=None, block=False) 0
DataPacket(op='w', register=3, data=1, block=False) None
DataPacket(op='r', register=3, data=None, block=False) 1
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ # write of single value - retry then suceed
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ ./mopicli -ron
DataPacket(op='r', register=9, data=None, block=False) 773
DataPacket(op='r', register=3, data=None, block=False) 65535
DataPacket(op='r', register=3, data=None, block=False) 65535
DataPacket(op='r', register=3, data=None, block=False) 65535
DataPacket(op='r', register=3, data=None, block=False) 1
Power on delay: 1 seconds
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ # and reading back agrees it was set
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ ./mopicli -rc
DataPacket(op='r', register=9, data=None, block=False) 773
DataPacket(op='r', register=2, data=None, block=True) [0, 0, 0, 0, 0]
Combined configuration:
Maximum voltage: 0 mV
Good voltage: 0 mV
Low voltage: 0 mV
Critical voltage: 0 mV
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ ./mopicli -wc 1 100 200 300 400
DataPacket(op='r', register=9, data=None, block=False) 773
DataPacket(op='w', register=2, data=[1, 1, 2, 3, 4], block=True) 0
DataPacket(op='r', register=2, data=None, block=True) [0, 0, 0, 0, 0]
DataPacket(op='w', register=2, data=[1, 1, 2, 3, 4], block=True) 0
DataPacket(op='r', register=2, data=None, block=True) [0, 0, 0, 0, 0]
DataPacket(op='w', register=2, data=[1, 1, 2, 3, 4], block=True) 0
DataPacket(op='r', register=2, data=None, block=True) [0, 0, 0, 0, 0]
DataPacket(op='w', register=2, data=[1, 1, 2, 3, 4], block=True) None
DataPacket(op='r', register=2, data=None, block=True) [1, 1, 2, 3, 4]
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ # reading in bulk can't fail, so reads whatever it gets
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ # writing can fail, if reading back doesn't match
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ # retry appears to work
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ ./mopicli -rc
DataPacket(op='r', register=9, data=None, block=False) 773
DataPacket(op='r', register=2, data=None, block=True) [1, 1, 2, 3, 4]
Combined configuration:
Source type: batteries
Maximum voltage: 100 mV
Good voltage: 200 mV
Low voltage: 300 mV
Critical voltage: 400 mV
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ # and reading values back looks good
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ # now make all ops retry for more than 4 times
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ ./mopicli -ron
DataPacket(op='r', register=9, data=None, block=False) 773
DataPacket(op='r', register=3, data=None, block=False) 65535
DataPacket(op='r', register=3, data=None, block=False) 65535
DataPacket(op='r', register=3, data=None, block=False) 65535
DataPacket(op='r', register=3, data=None, block=False) 65535
mopicli. Input/output error
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ # failed as it should
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ ./mopicli -won 12
DataPacket(op='r', register=9, data=None, block=False) 773
DataPacket(op='w', register=3, data=12, block=False) 0
DataPacket(op='r', register=3, data=None, block=False) 65535
DataPacket(op='r', register=3, data=None, block=False) 65535
DataPacket(op='r', register=3, data=None, block=False) 65535
DataPacket(op='r', register=3, data=None, block=False) 65535
mopicli. Input/output error
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ # failed as it should
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ ./mopicli -ron
DataPacket(op='r', register=9, data=None, block=False) 773
DataPacket(op='r', register=3, data=None, block=False) 65535
DataPacket(op='r', register=3, data=None, block=False) 65535
DataPacket(op='r', register=3, data=None, block=False) 65535
DataPacket(op='r', register=3, data=None, block=False) 65535
mopicli. Input/output error
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ # read back also fails
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ ./mopicli -rc
DataPacket(op='r', register=9, data=None, block=False) 773
DataPacket(op='r', register=2, data=None, block=True) [0, 0, 0, 0, 0]
Combined configuration:
Maximum voltage: 0 mV
Good voltage: 0 mV
Low voltage: 0 mV
Critical voltage: 0 mV
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ # that can't fail
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ ./mopicli -wc 1 100 200 300 400
DataPacket(op='r', register=9, data=None, block=False) 773
DataPacket(op='w', register=2, data=[1, 1, 2, 3, 4], block=True) 0
DataPacket(op='r', register=2, data=None, block=True) [0, 0, 0, 0, 0]
DataPacket(op='w', register=2, data=[1, 1, 2, 3, 4], block=True) 0
DataPacket(op='r', register=2, data=None, block=True) [0, 0, 0, 0, 0]
DataPacket(op='w', register=2, data=[1, 1, 2, 3, 4], block=True) 0
DataPacket(op='r', register=2, data=None, block=True) [0, 0, 0, 0, 0]
DataPacket(op='w', register=2, data=[1, 1, 2, 3, 4], block=True) 0
DataPacket(op='r', register=2, data=None, block=True) [0, 0, 0, 0, 0]
mopicli. Communication error on send
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ # but writes can fail
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ ./mopicli -rc
DataPacket(op='r', register=9, data=None, block=False) 773
DataPacket(op='r', register=2, data=None, block=True) [0, 0, 0, 0, 0]
Combined configuration:
Maximum voltage: 0 mV
Good voltage: 0 mV
Low voltage: 0 mV
Critical voltage: 0 mV
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ # and indeed had no effect
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ # all looks good
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ # check that not failing anything works ok
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ ./mopicli -ron
DataPacket(op='r', register=9, data=None, block=False) 773
DataPacket(op='r', register=3, data=None, block=False) 0
Power on delay: 0 seconds
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ ./mopicli -won 123
DataPacket(op='r', register=9, data=None, block=False) 773
DataPacket(op='w', register=3, data=123, block=False) None
DataPacket(op='r', register=3, data=None, block=False) 123
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ ./mopicli -ron
DataPacket(op='r', register=9, data=None, block=False) 773
DataPacket(op='r', register=3, data=None, block=False) 123
Power on delay: 123 seconds
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ # all good for single value read/write
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ ./mopicli -rc
DataPacket(op='r', register=9, data=None, block=False) 773
DataPacket(op='r', register=2, data=None, block=True) [0, 0, 0, 0, 0]
Combined configuration:
Maximum voltage: 0 mV
Good voltage: 0 mV
Low voltage: 0 mV
Critical voltage: 0 mV
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ ./mopicli -wc 1 200 300 400 500
DataPacket(op='r', register=9, data=None, block=False) 773
DataPacket(op='w', register=2, data=[1, 2, 3, 4, 5], block=True) None
DataPacket(op='r', register=2, data=None, block=True) [1, 2, 3, 4, 5]
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ ./mopicli -rc
DataPacket(op='r', register=9, data=None, block=False) 773
DataPacket(op='r', register=2, data=None, block=True) [1, 2, 3, 4, 5]
Combined configuration:
Source type: batteries
Maximum voltage: 200 mV
Good voltage: 300 mV
Low voltage: 400 mV
Critical voltage: 500 mV
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ # all good
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $
```

With my diffs removed you can see the problem I'm fixing - if writes fail enough times the error is swallowed accidentally.
```
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ patch -R < ~/mopi/diff.retryfix.3
patching file mopiapi.py
patching file mopicli
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ ./mopicli -ron
DataPacket(op='r', register=9, data=None, block=False) 773
DataPacket(op='r', register=3, data=None, block=False) 65535
DataPacket(op='r', register=3, data=None, block=False) 65535
DataPacket(op='r', register=3, data=None, block=False) 65535
mopicli. Communication error. Check bus? Check connection?
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ ./mopicli -ron
DataPacket(op='r', register=9, data=None, block=False) 773
DataPacket(op='r', register=3, data=None, block=False) 65535
DataPacket(op='r', register=3, data=None, block=False) 65535
DataPacket(op='r', register=3, data=None, block=False) 65535
mopicli. Communication error. Check bus? Check connection?
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ ./mopicli -won 1
DataPacket(op='r', register=9, data=None, block=False) 773
DataPacket(op='r', register=3, data=None, block=False) 65535
DataPacket(op='r', register=3, data=None, block=False) 65535
DataPacket(op='r', register=3, data=None, block=False) 65535
mopicli. Communication error. Check bus? Check connection?
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ ./mopicli -ron
DataPacket(op='r', register=9, data=None, block=False) 773
DataPacket(op='r', register=3, data=None, block=False) 65535
DataPacket(op='r', register=3, data=None, block=False) 65535
DataPacket(op='r', register=3, data=None, block=False) 65535
mopicli. Communication error. Check bus? Check connection?
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ ./mopicli -ron
DataPacket(op='r', register=9, data=None, block=False) 773
DataPacket(op='r', register=3, data=None, block=False) 65535
DataPacket(op='r', register=3, data=None, block=False) 65535
DataPacket(op='r', register=3, data=None, block=False) 0
Power on delay: 0 seconds
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ ./mopicli -won 1
DataPacket(op='r', register=9, data=None, block=False) 773
DataPacket(op='r', register=3, data=None, block=False) 65535
DataPacket(op='r', register=3, data=None, block=False) 65535
DataPacket(op='r', register=3, data=None, block=False) 0
DataPacket(op='w', register=3, data=1, block=False) 0
DataPacket(op='r', register=3, data=None, block=False) 0
DataPacket(op='w', register=3, data=1, block=False) 0
DataPacket(op='r', register=3, data=None, block=False) 0
DataPacket(op='w', register=3, data=1, block=False) None
DataPacket(op='r', register=3, data=None, block=False) 1
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ ./mopicli -ron
DataPacket(op='r', register=9, data=None, block=False) 773
DataPacket(op='r', register=3, data=None, block=False) 65535
DataPacket(op='r', register=3, data=None, block=False) 65535
DataPacket(op='r', register=3, data=None, block=False) 1
Power on delay: 1 seconds
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ ./mopicli -rc
DataPacket(op='r', register=9, data=None, block=False) 773
DataPacket(op='r', register=2, data=None, block=True) [0, 0, 0, 0, 0]
Combined configuration:
Maximum voltage: 0 mV
Good voltage: 0 mV
Low voltage: 0 mV
Critical voltage: 0 mV
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ ./mopicli -wc 1 100 200 300 400
DataPacket(op='r', register=9, data=None, block=False) 773
DataPacket(op='r', register=2, data=None, block=True) [0, 0, 0, 0, 0]
DataPacket(op='w', register=2, data=[1, 1, 2, 3, 4], block=True) 0
DataPacket(op='r', register=2, data=None, block=True) [0, 0, 0, 0, 0]
DataPacket(op='w', register=2, data=[1, 1, 2, 3, 4], block=True) 0
DataPacket(op='r', register=2, data=None, block=True) [0, 0, 0, 0, 0]
DataPacket(op='w', register=2, data=[1, 1, 2, 3, 4], block=True) None
DataPacket(op='r', register=2, data=None, block=True) [1, 1, 2, 3, 4]
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ ./mopicli -rc
DataPacket(op='r', register=9, data=None, block=False) 773
DataPacket(op='r', register=2, data=None, block=True) [1, 1, 2, 3, 4]
Combined configuration:
Source type: batteries
Maximum voltage: 100 mV
Good voltage: 200 mV
Low voltage: 300 mV
Critical voltage: 400 mV
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ ./mopicli -ron
DataPacket(op='r', register=9, data=None, block=False) 773
DataPacket(op='r', register=3, data=None, block=False) 65535
DataPacket(op='r', register=3, data=None, block=False) 65535
DataPacket(op='r', register=3, data=None, block=False) 65535
mopicli. Communication error. Check bus? Check connection?
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ ./mopicli -won 1
DataPacket(op='r', register=9, data=None, block=False) 773
DataPacket(op='r', register=3, data=None, block=False) 65535
DataPacket(op='r', register=3, data=None, block=False) 65535
DataPacket(op='r', register=3, data=None, block=False) 65535
mopicli. Communication error. Check bus? Check connection?
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ ./mopicli -ron
DataPacket(op='r', register=9, data=None, block=False) 773
DataPacket(op='r', register=3, data=None, block=False) 65535
DataPacket(op='r', register=3, data=None, block=False) 65535
DataPacket(op='r', register=3, data=None, block=False) 65535
mopicli. Communication error. Check bus? Check connection?
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ ./mopicli -rc
DataPacket(op='r', register=9, data=None, block=False) 773
DataPacket(op='r', register=2, data=None, block=True) [0, 0, 0, 0, 0]
Combined configuration:
Maximum voltage: 0 mV
Good voltage: 0 mV
Low voltage: 0 mV
Critical voltage: 0 mV
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ ./mopicli -wc 1 200 300 400 500
DataPacket(op='r', register=9, data=None, block=False) 773
DataPacket(op='r', register=2, data=None, block=True) [0, 0, 0, 0, 0]
DataPacket(op='w', register=2, data=[1, 2, 3, 4, 5], block=True) 0
DataPacket(op='r', register=2, data=None, block=True) [0, 0, 0, 0, 0]
DataPacket(op='w', register=2, data=[1, 2, 3, 4, 5], block=True) 0
DataPacket(op='r', register=2, data=None, block=True) [0, 0, 0, 0, 0]
DataPacket(op='w', register=2, data=[1, 2, 3, 4, 5], block=True) 0
DataPacket(op='r', register=2, data=None, block=True) [0, 0, 0, 0, 0]
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ ./mopicli -rc
DataPacket(op='r', register=9, data=None, block=False) 773
DataPacket(op='r', register=2, data=None, block=True) [0, 0, 0, 0, 0]
Combined configuration:
Maximum voltage: 0 mV
Good voltage: 0 mV
Low voltage: 0 mV
Critical voltage: 0 mV
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ # array write fails but doesn't error
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ # single writes do error, but because read also fails
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ # so make reads work but writes fail
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ ./mopicli -ron
DataPacket(op='r', register=9, data=None, block=False) 773
DataPacket(op='r', register=3, data=None, block=False) 65535
DataPacket(op='r', register=3, data=None, block=False) 65535
DataPacket(op='r', register=3, data=None, block=False) 0
Power on delay: 0 seconds
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ ./mopicli -ron
DataPacket(op='r', register=9, data=None, block=False) 773
DataPacket(op='r', register=3, data=None, block=False) 65535
DataPacket(op='r', register=3, data=None, block=False) 65535
DataPacket(op='r', register=3, data=None, block=False) 0
Power on delay: 0 seconds
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ ./mopicli -won 2
DataPacket(op='r', register=9, data=None, block=False) 773
DataPacket(op='r', register=3, data=None, block=False) 65535
DataPacket(op='r', register=3, data=None, block=False) 65535
DataPacket(op='r', register=3, data=None, block=False) 0
DataPacket(op='w', register=3, data=2, block=False) 0
DataPacket(op='r', register=3, data=None, block=False) 0
DataPacket(op='w', register=3, data=2, block=False) 0
DataPacket(op='r', register=3, data=None, block=False) 0
DataPacket(op='w', register=3, data=2, block=False) 0
DataPacket(op='r', register=3, data=None, block=False) 0
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ # seemed to fail but no error
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ ./mopicli -ron
DataPacket(op='r', register=9, data=None, block=False) 773
DataPacket(op='r', register=3, data=None, block=False) 65535
DataPacket(op='r', register=3, data=None, block=False) 65535
DataPacket(op='r', register=3, data=None, block=False) 0
Power on delay: 0 seconds
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $ # definitely failed
pi@RPi-Printer ~/mopi/pi-tronics/simbamon $
```