Skip to content

Possible fix for #11 #15

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 2 commits into from
Oct 9, 2018
Merged

Possible fix for #11 #15

merged 2 commits into from
Oct 9, 2018

Conversation

davidskeck
Copy link
Contributor

Would like someone to review, didn't setup hardware tests yet. Will do so when I have more time. Thanks for the Hacktoberfest tag!

Would like someone to review, didn't setup hardware tests yet. Will do so when I have more time. Thanks for the Hacktoberfest tag!
@kattni kattni requested a review from a team October 9, 2018 01:13
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. The changes requested are mainly cosmetic.

accel = (calibration_data >> 2) & 0x03
mag = calibration_data & 0x03
return sys, gyro, accel, mag

Copy link
Contributor

Choose a reason for hiding this comment

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

Being able to get the individual status's might be useful. Change this to a property called calibration_status, still returning the same tuple.

if sys < 3 or gyro < 3 or accel < 3 or mag < 3:
return False
return True

Copy link
Contributor

Choose a reason for hiding this comment

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

Change name to simply calibrated. And a python-y way to do the check and return is:

return sys == gyro == accel == mag == 0x03

Also - the doc strings for properties should read like a variable, not a function. So instead of describing what it does ("Returns..."), describe what it is ("The status of...").

@davidskeck
Copy link
Contributor Author

Thank you for the review. I've made the requested changes. This is helping me understand Python decorators better too. 👍

@caternuson caternuson merged commit 9617e19 into adafruit:master Oct 9, 2018
@caternuson
Copy link
Contributor

Thanks for the update!

@davidskeck davidskeck deleted the patch-1 branch October 9, 2018 21:58
tannewt pushed a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Nov 11, 2018
Updating https://github.com/adafruit/Adafruit_CircuitPython_BNO055 to 3.0.4 from 3.0.3:
  > Merge pull request adafruit/Adafruit_CircuitPython_BNO055#15 from davidskeck/patch-1
  > ignore the board module imports in .pylintrc
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.

2 participants