Skip to content

Adding support for the LSM6DSO32 #20

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 5 commits into from
Aug 10, 2020
Merged

Adding support for the LSM6DSO32 #20

merged 5 commits into from
Aug 10, 2020

Conversation

siddacious
Copy link
Contributor

No description provided.

@siddacious siddacious requested a review from a team July 17, 2020 01:53
tannewt
tannewt requested changes Jul 17, 2020
View reviewed changes
Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

One restructuring request.

README.rst Outdated
@@ -6,15 +6,19 @@ Introduction
:alt: Documentation Status

.. image:: https://img.shields.io/discord/327254708534116352.svg
:target: https://adafru.it/discord
:target: https://discord.gg/nBQh6qu
Copy link
Member

Choose a reason for hiding this comment

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

This is the old broken link I think.

adafruit_lsm6ds.py Outdated
@@ -386,6 +405,33 @@ def __init__(self, i2c_bus, address=_LSM6DS_DEFAULT_ADDRESS):
self._i3c_disable = True


class LSM6DSO32(LSM6DS): # pylint: disable=too-many-instance-attributes
Copy link
Member

Choose a reason for hiding this comment

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

How about splitting the subclasses into separate files? That way they all aren't loaded into memory when one is needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tannewt ok, please take a look

tannewt
tannewt requested changes Jul 22, 2020
View reviewed changes
Copy link
Member

@tannewt tannewt 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! Just a couple comments.

adafruit_lsm6ds/__init__.py Outdated
if value is GyroRange.RANGE_125_DPS:
self._gyro_range_125dps = True
self._gyro_range_4000dps = False
elif value is GyroRange.RANGE_4000_DPS:
elif value == 4000:
Copy link
Member

Choose a reason for hiding this comment

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

Why undo this? Maybe have a top-level GyroRange and mask the gyro_range property in subclass that don't support 4000 so they error? Something like:

@property
def gyro_range(self, value):
    if value == GyroRange.RANGE_4000_DPS:
        raise ValueError()
    super().gyro_range = value

I'm not positive that will work since it's a property. You may need a private internal method instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tannewt please take a look at my changes when you have a chance. I'm now relying on the non-existance of the 4k range in the non-ISM classes to handle itself, and moved the 4k bit management into the subclass where it belongs.

@siddacious siddacious mentioned this pull request Jul 30, 2020
Closed
@evaherrada evaherrada mentioned this pull request Aug 6, 2020
Closed
@evaherrada evaherrada linked an issue Aug 6, 2020 that may be closed by this pull request
AttributeError: 'LSM6DS33' object has no attribute 'pedometer_steps' #21
Closed
@siddacious siddacious requested a review from tannewt August 10, 2020 17:46
tannewt
tannewt approved these changes Aug 10, 2020
View reviewed changes
Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

This looks good to me! Thank you!

@tannewt tannewt merged commit 305c658 into master Aug 10, 2020
adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Aug 21, 2020
@adafruit-adabot
Automated update by Adabot (adafruit/adabot@fddc0fa)
0bf8323 
Updating https://github.com/adafruit/Adafruit_CircuitPython_DHT to 3.5.0 from 3.4.2:
  > Merge pull request adafruit/Adafruit_CircuitPython_DHT#46 from adafruit/pulseio-fix

Updating https://github.com/adafruit/Adafruit_CircuitPython_HCSR04 to 0.4.3 from 0.4.2:
  > Merge pull request adafruit/Adafruit_CircuitPython_HCSR04#21 from FoamyGuy/dannystaple-readme-patch

Updating https://github.com/adafruit/Adafruit_CircuitPython_INA219 to 3.4.4 from 3.4.3:
  > Merge pull request adafruit/Adafruit_CircuitPython_INA219#24 from nvtkaszpir/patch-1
  > Merge pull request adafruit/Adafruit_CircuitPython_INA219#23 from nvtkaszpir/patch-2
  > Merge pull request adafruit/Adafruit_CircuitPython_INA219#22 from nvtkaszpir/patch-3

Updating https://github.com/adafruit/Adafruit_CircuitPython_LSM6DS to 4.1.0 from 4.0.1:
  > Merge pull request adafruit/Adafruit_CircuitPython_LSM6DS#20 from adafruit/lsm6dso32

Updating https://github.com/adafruit/Adafruit_CircuitPython_MS8607 to 1.0.1 from 1.0.0:
  > updated PID

Updating https://github.com/adafruit/Adafruit_CircuitPython_PCT2075 to 1.1.4 from 1.1.3:
  > Merge pull request adafruit/Adafruit_CircuitPython_PCT2075#12 from garrettheath4/readme-fix

Updating https://github.com/adafruit/Adafruit_CircuitPython_PM25 to 1.0.3 from 1.0.2:
  > Merge pull request adafruit/Adafruit_CircuitPython_PM25#6 from dglaude/patch-2

Updating https://github.com/adafruit/Adafruit_CircuitPython_PN532 to 2.2.0 from 2.1.2:
  > Merge pull request adafruit/Adafruit_CircuitPython_PN532#37 from dunkmann00/listen-for-passive-target

Updating https://github.com/adafruit/Adafruit_CircuitPython_TLC59711 to 1.2.3 from 1.2.2:
  > Merge pull request adafruit/Adafruit_CircuitPython_TLC59711#14 from FoamyGuy/remove_hardcoded_baudrate

Updating https://github.com/adafruit/Adafruit_CircuitPython_Bitmap_Font to 1.2.0 from 1.1.3:
  > Merge pull request adafruit/Adafruit_CircuitPython_Bitmap_Font#27 from FoamyGuy/adding_more_examples
  > Merge pull request adafruit/Adafruit_CircuitPython_Bitmap_Font#24 from FoamyGuy/fix_docs_link_in_readme

Updating https://github.com/adafruit/Adafruit_CircuitPython_BLE_Adafruit to 1.2.0 from 1.1.1:
  > Merge pull request adafruit/Adafruit_CircuitPython_BLE_Adafruit#6 from dhalbert/fix-packet-sizes

Updating https://github.com/adafruit/Adafruit_CircuitPython_Display_Text to 2.8.2 from 2.8.1:
  > Merge pull request adafruit/Adafruit_CircuitPython_Display_Text#88 from FoamyGuy/fix_load_glyphs_for_builtin

Updating https://github.com/adafruit/Adafruit_CircuitPython_LED_Animation to 2.4.2 from 2.4.1:
  > Merge pull request adafruit/Adafruit_CircuitPython_LED_Animation#64 from rhooper/adjust-copyright

Updating https://github.com/adafruit/Adafruit_CircuitPython_Motor to 3.2.1 from 3.1.2:
  > Merge pull request adafruit/Adafruit_CircuitPython_Motor#46 from caternuson/example_rename
  > Merge pull request adafruit/Adafruit_CircuitPython_Motor#44 from caternuson/digi_step

Updating https://github.com/adafruit/Adafruit_CircuitPython_ProgressBar to 1.3.2 from 1.3.1:
  > Merge pull request adafruit/Adafruit_CircuitPython_ProgressBar#14 from FoamyGuy/simpletest_improvement

Updating https://github.com/adafruit/Adafruit_CircuitPython_PyBadger to 3.1.0 from 3.0.1:
  > Merge pull request adafruit/Adafruit_CircuitPython_PyBadger#35 from FoamyGuy/adding_cpb_gizmo
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tannewt tannewt tannewt 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.

AttributeError: 'LSM6DS33' object has no attribute 'pedometer_steps'
2 participants
@siddacious @tannewt