phrogger
commented
9 years ago ------------------------------------ CLIP --------------------------------------
!/usr/bin/python
Demonstration of bug in Adafruit_I2C.reverseByteOrder()
from Adafruit_I2C import *
Initialize the class; address and bus number must be that of a valid I2C device
interface = Adafruit_I2C(0x18 , busnum=2)
for i in range( 0x03FD, 0x0404): y = ((i << 8 & 0xFF00) | (i >> 8 & 0X00FF)) # Reverse 16 bit byte order z = interface.reverseByteOrder(y) print 'Value = %04X, Input = %04X, Result = %04X' % (i, y, z)
---------------------------------- END CLIP -------------------------------------
Resulting demonstration
Result should equal Value.
Input is the byte reversal of Value, and is correct by inspection
root@BBB1:~# python bugdemo.py Value = 03FD, Input = FD03, Result = 03FD Value = 03FE, Input = FE03, Result = 03FE Value = 03FF, Input = FF03, Result = 03FF Value = 0400, Input = 0004, Result = 0004 # <<<<<<<<<< FAIL Value = 0401, Input = 0104, Result = 0401 Value = 0402, Input = 0204, Result = 0402 Value = 0403, Input = 0304, Result = 0403
Suggested fix: I think it is impossible to make Adafruit_I2C.reverseByteOrder() work as written. It is trying to determine the size of a value by detecting the presence of "ones" to mark the first valid byte. This fails when a leading valid byte is 0x00.
I suggest that Adafruit_I2C.reverseByteOrder() be replaced by two separate methods:
def reverseByteOrder16(self, data):
"Reverses the byte order of a 16-bit value"
val = ((data << 8 & 0xFF00) | (data >> 8 & 0X00FF))
return val
def reverseByteOrder32(self, data):
"Reverses the byte order of a 32-bit value"
val = ((data << 24 & 0xFF000000) | (data << 8 & 0xFF0000) | (data >> 8 & 0xFF00) | (data >> 24 & 0xFF))
return val
ladyada
If the low order byte is 0x00, then the method reverseByteOrder does not work.
Example: I am reading the manufacturer's code for an Adafruit MCP9808, which is 0x0400.
The sixteen bit value returned by the method readU16 is 0x0004. Swapping the bytes should result in 0x0400. reverseByteOrder(0x0004) returns 0x0004.
Since the reverseByteOrder method decides whether it is dealing with a 32 bit or 16 bit value by examining where the bits are, it decides that 0x0004 is a one byte value, and does not swap the bytes. It properly swaps 0x0104 or 0xFF03, which it correctly identifies as two byte values.
The 32 bit versus 16 bit value detector is fundamentally flawed. It would also detect the 32 bit value 0x0000FFFF as a 16 bit value, and return 0xFFFF instead of the correct 0xFFFF0000.
They only solution I can recommend is to split reverseByteOrder into two methods, one for swapping 16 bit values, and one for swapping 32 bit values.
--- Graham / KE9H