Skip to content

fixes #4 caused by CP's liberal unpack #5

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 11, 2019

Conversation

siddacious
Copy link
Contributor

This should fix the issue; As far as I can tell CP's unpack is filling in the missing byte which we are already shifting away, so increasing _BUFFER to 4 bytes allows CPython's unpack to not barf and the garbage extra byte at the end isn't a problem.

I tested that it works in Python 3 by pasting and example _BUFFER into a REPL and finishing the conversion by hand, but if someone can double check on an RPi or similar that would be swell.

~$ python3
Python 3.7.0 (default, Sep 25 2018, 01:24:12)
[Clang 7.0.2 (clang-700.1.81)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> raw = bytearray(b'\x01P ')
>>> from struct import unpack
>>> unpack(">i", raw)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
struct.error: unpack requires a buffer of 4 bytes
>>> unpack(">i", bytearray(b'\x01I\xa0\x00'))
(21602304,)
>>> len(bytearray(b'\x01I\xa0\x00'))
4
>>> unpack(">i", bytearray(b'\x01I\xa0\x00'))
(21602304,)
>>> unpack(">i", bytearray(b'\x01I\xa0\x00'))[0]>>8
84384
>>> (unpack(">i", bytearray(b'\x01I\xa0\x00'))[0]>>8)/4096.0
20.6015625

An example of CP's unpack behavior:

>>> unpack(">i", bytearray([1,0,0,0]))
(16777216,)
>>> unpack(">i", bytearray([1,0,0]))
(16777216,)
>>> unpack(">i", bytearray([1,0]))
(16777216,)
>>> unpack(">i", bytearray([1]))
(16777216,)

@siddacious siddacious requested a review from a team January 10, 2019 06:10
@caternuson
Copy link
Contributor

Elegant. Tested and works on RPi.

pi@raspberrypi:~ $ python3
Python 3.5.3 (default, Sep 27 2018, 17:25:39) 
[GCC 6.3.0 20170516] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import board, busio
>>> import digitalio
>>> import adafruit_max31856
>>> spi = busio.SPI(board.SCK, board.MOSI, board.MISO)
>>> cs = digitalio.DigitalInOut(board.D21)
>>> cs.direction = digitalio.Direction.OUTPUT
>>> tc = adafruit_max31856.MAX31856(spi, cs)
>>> len(tc._BUFFER)
4
>>> tc.temperature
26.1171875
>>> 

@ladyada thanks for the tip about setting PYTHONPATH for testing CP libs on RPi/blinka

@caternuson
Copy link
Contributor

caternuson commented Jan 10, 2019

@siddacious Did / can you test this on a CP board?

@siddacious
Copy link
Contributor Author

@caternuson Yes, I did.

Copy link
Contributor

@caternuson caternuson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me then. Thanks.

@ladyada ladyada merged commit e3245c3 into adafruit:master Jan 11, 2019
@ladyada
Copy link
Member

ladyada commented Jan 11, 2019

alright @caternuson @kattni bump and release :)

@kattni
Copy link
Contributor

kattni commented Jan 11, 2019

Done!

tannewt pushed a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Jan 11, 2019
Updating https://github.com/adafruit/Adafruit_CircuitPython_MAX31856 to 0.8.2 from 0.8.1:
  > Merge pull request adafruit/Adafruit_CircuitPython_MAX31856#5 from siddacious/master
  > Merge pull request adafruit/Adafruit_CircuitPython_MAX31856#3 from sommersoft/readme_fix_travis

Updating https://github.com/adafruit/Adafruit_CircuitPython_BoardTest to 1.0.0 from ac3cf2b:
  < Merge pull request adafruit/Adafruit_CircuitPython_BoardTest#4 from ShawnHymel/master
  < Merge pull request adafruit/Adafruit_CircuitPython_BoardTest#3 from ShawnHymel/master

Updating https://github.com/adafruit/Adafruit_CircuitPython_Bundle/circuitpython_library_list.md to NA from NA:
  > Added the following libraries: Adafruit_CircuitPython_ADT7410, Adafruit_CircuitPython_BoardTest
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants