Skip to content

adding rotation modifier for gestures #26

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 4 commits into from
Jun 17, 2020

Conversation

FoamyGuy
Copy link

@FoamyGuy FoamyGuy commented May 4, 2020

This adds a rotation offset that can be used to adjust the values returned by gesture() in case your sensor is mounted in a different orientation. Solves #25

def rotation(self, new_rotation):
if new_rotation in [0, 90, 180, 270]:
self._rotation = new_rotation

Choose a reason for hiding this comment

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

I think this should throw a ValueError if the input is not in [0,90,180,270] - it will just ignore it silently as written

Choose a reason for hiding this comment

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

Thanks

Copy link

@jerryneedell jerryneedell left a comment

Choose a reason for hiding this comment

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

Nice additions! This works well - tested with an apds9960_breakout on a grandcentral_m4.
I think it should be made clear that only values of 0,90,180,270 are permitted and throw an error (ValueError) if something else is input. A note should also be added to the Doc String that only these 4 values are accepted.

@property
def rotation(self):
"""Gesture rotation offset"""
return self._rotation

Choose a reason for hiding this comment

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

Indicate that only [0,90,180,270] are acceptable

@FoamyGuy
Copy link
Author

Agree'd thank you for catching that @jerryneedell. I will make those changes later on tonight.

Copy link

@jerryneedell jerryneedell 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 - tested with breakout on grandcentral. Entered 100 -- received ValueError

@evaherrada evaherrada merged commit 4242772 into adafruit:master Jun 17, 2020
@evaherrada evaherrada mentioned this pull request Jun 17, 2020
adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Jun 19, 2020
Updating https://github.com/adafruit/Adafruit_CircuitPython_APDS9960 to 2.2.0 from 2.1.1:
  > Merge pull request adafruit/Adafruit_CircuitPython_APDS9960#26 from FoamyGuy/rotation_modifier

Updating https://github.com/adafruit/Adafruit_CircuitPython_LIS3MDL to 1.1.3 from 1.1.2:
  > Merge pull request adafruit/Adafruit_CircuitPython_LIS3MDL#10 from kattni/combo-example

Updating https://github.com/adafruit/Adafruit_CircuitPython_BLE to 7.0.0 from 6.1.5:
  > Merge pull request adafruit/Adafruit_CircuitPython_BLE#89 from dhalbert/easier-prefixes

Updating https://github.com/adafruit/Adafruit_CircuitPython_BLE_BroadcastNet to 0.10.0 from 0.9.0:
  > Merge pull request adafruit/Adafruit_CircuitPython_BLE_BroadcastNet#10 from dhalbert/match_prefixes

Updating https://github.com/adafruit/Adafruit_CircuitPython_BLE_Radio to 0.3.0 from 0.2.1:
  > Merge pull request adafruit/Adafruit_CircuitPython_BLE_Radio#12 from dhalbert/match_prefixes
  > Merge pull request adafruit/Adafruit_CircuitPython_BLE_Radio#10 from adafruit/add-build-yml

Updating https://github.com/adafruit/Adafruit_CircuitPython_BLE_Adafruit to 1.1.0 from 1.0.0:
  > Merge pull request adafruit/Adafruit_CircuitPython_BLE_Adafruit#4 from dhalbert/match-prefix

Updating https://github.com/adafruit/Adafruit_CircuitPython_BLE_Eddystone to 0.10.0 from 0.9.0:
  > Merge pull request adafruit/Adafruit_CircuitPython_BLE_Eddystone#8 from dhalbert/match_prefixes
  > build.yml: add black formatting check

Updating https://github.com/adafruit/Adafruit_CircuitPython_Display_Text to 2.7.1 from 2.7.0:
  > Merge pull request adafruit/Adafruit_CircuitPython_Display_Text#51 from caternuson/iss50
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.

3 participants