Skip to content

Fixes for SPI class #11

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
Nov 29, 2018
Merged

Fixes for SPI class #11

merged 7 commits into from
Nov 29, 2018

Conversation

caternuson
Copy link
Contributor

Fix for #10 . But also fixed to work with actual phase/polarity of sensor. Example also updated to show SPI usage.

@kattni kattni mentioned this pull request Oct 15, 2018
Open
@ladyada
Copy link
Member

ladyada commented Nov 11, 2018

@kattni please test this if hasnt yet, and then merge/release!

@ladyada ladyada requested a review from kattni November 11, 2018 17:04
@kattni
Copy link
Contributor

kattni commented Nov 11, 2018

Will do! Still needs testing. Added to my list.

kattni
kattni suggested changes Nov 20, 2018
View reviewed changes
Copy link
Contributor

@kattni kattni left a comment

Choose a reason for hiding this comment

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

Thanks for doing this!

If I update both of the following two suggestions to match the rest of the code, it runs perfectly. So please take a look and update either the two lines I suggested, or the rest of the SPI code to match.

adafruit_lsm9ds0.py Outdated
@@ -397,8 +397,8 @@ class LSM9DS0_SPI(LSM9DS0):
"""Driver for the LSM9DS0 connected over SPI."""
# pylint: disable=no-member
def __init__(self, spi, xmcs, gcs):
self._gyro_device = spi_device.I2CDevice(spi, gcs)
self._xm_device = spi_device.I2CDevice(spi, xmcs)
self._mag_device = spi_device.SPIDevice(spi, xmcs, baudrate=200000, phase=1, polarity=1)
Copy link
Contributor

Choose a reason for hiding this comment

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

The rest of the code refers to self._gyro_device, is this incorrect or is the rest of the code incorrect?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

see comment below

adafruit_lsm9ds0.py Outdated
self._gyro_device = spi_device.I2CDevice(spi, gcs)
self._xm_device = spi_device.I2CDevice(spi, xmcs)
self._mag_device = spi_device.SPIDevice(spi, xmcs, baudrate=200000, phase=1, polarity=1)
self._xg_device = spi_device.SPIDevice(spi, gcs, baudrate=200000, phase=1, polarity=1)
Copy link
Contributor

Choose a reason for hiding this comment

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

The rest of the code refers to self._xm_device. Same question as above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

see comment below

@caternuson
Copy link
Contributor Author

Thanks @kattni . Silly copy-pasta error. Didn't have hardware to test so only did like a 2 second copy paste of the fix from LSM9DS1. For some reason, that library uses different variable names for the gyro and mag sensors.

Feel stupid for not even doing a visual scan - that should've stuck out like a major thumb of soreness.

@kattni
Copy link
Contributor

kattni commented Nov 20, 2018

So the fix resulted in the code failing without swapping the two digital pins. When it runs, all of the data is incorrect. Example:

Acceleration (m/s^2): (0.154,0.154,0.154)
Magnetometer (gauss): (-0.185,-0.185,-0.185)
Gyroscope (degrees/sec): (-20.247,-20.247,-20.247)
Temperature: -39.250C

As @caternuson doesn't have hardware to test it with, he's asked that I take over this fix. I'll take a look at it when I finish my current project.

@caternuson
Copy link
Contributor Author

Thanks @kattni . Ping me if you need help. But yah, doesn't look like a simple copy-paste is all that's needed here. :(

caternuson
caternuson commented Nov 20, 2018
View reviewed changes
examples/lsm9ds0_simpletest.py
# csm.direction = Direction.OUTPUT
# csm.value = True
# sensor = adafruit_lsm9ds0.LSM9DS0_SPI(spi, csag, csm)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This example copy-pasted from LSM9DS1 is probably all wrong also. For one thing, the CS pins are taken in a different order for the LSM9DS0:

def __init__(self, spi, xmcs, gcs):

@ladyada
Copy link
Member

ladyada commented Nov 23, 2018

here's code, there were a few things wrong!

lsm9.zip

@caternuson
Copy link
Contributor Author

@ladyada thanks!

@kattni I simply added code from .zip as is and then fixed a few lint things.

kattni
kattni suggested changes Nov 29, 2018
View reviewed changes
Copy link
Contributor

@kattni kattni left a comment

Choose a reason for hiding this comment

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

Tested this on Feather M4, works great! I have a couple of suggestions below.

adafruit_lsm9ds0.py Outdated
i2c.readinto(buf, end=count)
#print("read from %02x: %s" % (address, [hex(i) for i in buf[:count]]))
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove print statement. Seems like it's left over from debugging, shouldn't be needed in the final release.

adafruit_lsm9ds0.py Outdated
@@ -391,26 +385,23 @@ def _write_u8(self, sensor_type, address, val):
self._BUFFER[0] = address & 0xFF
self._BUFFER[1] = val & 0xFF
i2c.write(self._BUFFER, end=2)
#print("write to %02x: %02x" % (address, val))
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above.

adafruit_lsm9ds0.py Outdated
spi.write(buf, end=1)
spi.readinto(buf, end=count)
#print("read from %02x: %s" % (address, [hex(i) for i in buf[:count]]))
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above.

adafruit_lsm9ds0.py Outdated
self._BUFFER[0] = (address & 0x7F) & 0xFF
self._BUFFER[1] = val & 0xFF
spi.write(self._BUFFER, end=2)
#print("write to %02x: %02x" % (address, val))
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above.

@kattni
Copy link
Contributor

kattni commented Nov 29, 2018

@ladyada Do you want the print statements left in?

@ladyada
Copy link
Member

ladyada commented Nov 29, 2018

keep them in, they're not taking up any space :)

@kattni kattni merged commit 78471b1 into adafruit:master Nov 29, 2018
@caternuson caternuson mentioned this pull request Nov 29, 2018
Closed
tannewt pushed a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Nov 30, 2018
@adafruit-adabot
Automated update by Adabot (adafruit/adabot@6086fba)
3b333ea 
Updating https://github.com/adafruit/Adafruit_CircuitPython_LSM9DS0 to 2.1.0 from 2.0.0:
  > Merge pull request adafruit/Adafruit_CircuitPython_LSM9DS0#11 from caternuson/iss10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kattni kattni kattni approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@caternuson @ladyada @kattni