Missing abs() in spinner examples to determine maximum value

[kevinjwalters]

Both examples/spinner.py and examples/spinner_advanced.py feature the same code which reads 32 values in total and looks for maximum absolute value from the x-axis accelerometer value. There's a missing abs() on the first read before the loop which looks like a bug?

        maxval = lis3dh.acceleration[0]  # Grab just the X acceleration value.
        for i in range(31):
            x = abs(lis3dh.acceleration[0])
            if x > maxval:
                maxval = x

[ladyada]

hmm, i think doesnt matter cause all of the 32 sampels will be larger (since they, at the least, positive)?

[kevinjwalters]

I've only looked at code, not inspected it running in debugger/etc but if the aim of the code is to find the maximum magnitude value, I think the code would fail on certain data, e.g.:

-1.0 -0.3 -0.3 -0.3 more of the same
as the first of those -0.3 would be abs()'d to 0.3 and that is greater than -1.0 so would become the maxval which would be incorrect? Very dependent on the data. Confusingly this would work most of the time.

[ladyada]

ok let me patch it, give me a moment - did you try it with abs(), did it work well then?

[kevinjwalters]

I was looking at something different (https://forums.adafruit.com/viewtopic.php?f=60&t=140745) and just noticed this. I've not seen any problems from this and with these particular non-critical examples I doubt it would or could be noticed. There's a problem if the code is cut and paste or influences other code, of course.

I wrote some tests to demo it (on 3.4.8 on Linux):

import unittest
from unittest.mock import Mock

### A section of spinner example code
def maxmagniffy(lis3dh):
    maxval = lis3dh.acceleration[0]
    for i in range(31):
        x = abs(lis3dh.acceleration[0])
        if x > maxval:
            maxval = x
    return maxval

### Note extra abs()
def maxmagnfixed(lis3dh):
    maxval = abs(lis3dh.acceleration[0])
    for i in range(31):
        x = abs(lis3dh.acceleration[0])
        if x > maxval:
            maxval = x
    return maxval

### negative to positive values
refxnumbers1 = [(x-16)/16 for x in range(16)]+[x/16 for x in range(1,17)]
### negative only values
refxnumbers2 = [(x-32)/32 for x in range(32)]
#refaccels = [(x,0.0,0.0) for x in refxnumbers]

class TestMaxMagn(unittest.TestCase):

    ### lis3dh.acceleration returns a tuple of three
    ### a crude emulation is used here
    def checkMaxMagn(self, thefn, numbers):
        lis3dh = Mock()
        ### iterate over the numbers rotated
        realxmax = max([abs(x) for x in numbers])
        for i in range(len(numbers)):
            testdata = numbers[i:] + numbers[:i]
            lis3dh.acceleration.__getitem__ = Mock(side_effect = testdata)
            maxval = thefn(lis3dh)
            self.assertEqual(maxval, realxmax)

    def testIffy(self):
        """
        """
        print("checking", refxnumbers1)
        self.checkMaxMagn(maxmagniffy, refxnumbers1)
        print("checking", refxnumbers2)
        self.checkMaxMagn(maxmagniffy, refxnumbers2)

    def testFixed(self):
        """
        """
        print("checking", refxnumbers1)
        self.checkMaxMagn(maxmagnfixed, refxnumbers1)
        print("checking", refxnumbers2)
        self.checkMaxMagn(maxmagnfixed, refxnumbers2)

verbose=2

if __name__ == '__main__':
    unittest.main(verbosity=verbose)

Output is:

testFixed (__main__.TestMaxMagn) ... checking [-1.0, -0.9375, -0.875, -0.8125, -0.75, -0.6875, -0.625, -0.5625, -0.5, -0.4375, -0.375, -0.3125, -0.25, -0.1875, -0.125, -0.0625, 0.0625, 0.125, 0.1875, 0.25, 0.3125, 0.375, 0.4375, 0.5, 0.5625, 0.625, 0.6875, 0.75, 0.8125, 0.875, 0.9375, 1.0]
checking [-1.0, -0.96875, -0.9375, -0.90625, -0.875, -0.84375, -0.8125, -0.78125, -0.75, -0.71875, -0.6875, -0.65625, -0.625, -0.59375, -0.5625, -0.53125, -0.5, -0.46875, -0.4375, -0.40625, -0.375, -0.34375, -0.3125, -0.28125, -0.25, -0.21875, -0.1875, -0.15625, -0.125, -0.09375, -0.0625, -0.03125]
ok
testIffy (__main__.TestMaxMagn) ... checking [-1.0, -0.9375, -0.875, -0.8125, -0.75, -0.6875, -0.625, -0.5625, -0.5, -0.4375, -0.375, -0.3125, -0.25, -0.1875, -0.125, -0.0625, 0.0625, 0.125, 0.1875, 0.25, 0.3125, 0.375, 0.4375, 0.5, 0.5625, 0.625, 0.6875, 0.75, 0.8125, 0.875, 0.9375, 1.0]
checking [-1.0, -0.96875, -0.9375, -0.90625, -0.875, -0.84375, -0.8125, -0.78125, -0.75, -0.71875, -0.6875, -0.65625, -0.625, -0.59375, -0.5625, -0.53125, -0.5, -0.46875, -0.4375, -0.40625, -0.375, -0.34375, -0.3125, -0.28125, -0.25, -0.21875, -0.1875, -0.15625, -0.125, -0.09375, -0.0625, -0.03125]
FAIL

======================================================================
FAIL: testIffy (__main__.TestMaxMagn)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "max-magnitude-value.py", line 54, in testIffy
    self.checkMaxMagn(maxmagniffy, refxnumbers2)
  File "max-magnitude-value.py", line 46, in checkMaxMagn
    self.assertEqual(maxval, realxmax)
AssertionError: 0.96875 != 1.0

----------------------------------------------------------------------
Ran 2 tests in 0.174s

FAILED (failures=1)

[ladyada]

ok both updated, close if its resolved! :)

[kevinjwalters]

Looks good to me, thanks.