Skip to content

Clock source integration #29

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 17 commits into from
Nov 7, 2022
Merged

Clock source integration #29

merged 17 commits into from
Nov 7, 2022

Conversation

SquirtleSquadLeader
Copy link
Contributor

  1. Added Clksel class to store options similar to previous author
  2. Changed Register name to match Register Map documentation
  3. Added Getter/Setter property to negotiate transactions
  4. Changed MPU6050 init() to reflect the new syntax to match the others
  5. Pre-written test script if that helps out the maintainers :)
    Third time is a charm ... ?

@SquirtleSquadLeader
Removed test script at bottome
087a70f 
... maybe this is it?
@SquirtleSquadLeader
Merge branch 'adafruit:main' into main
ca6ddaf
@SquirtleSquadLeader
Integrated Clock Source selection
dd037a9 
1) Added Clksel class to store options similar to previous author
2) Changed Register name to match Register Map documentation
3) Added Getter/Setter property to negotiate transactions
4) Changed MPU6050 __init()__ to reflect the new syntax to match the others
5) Pre-written te
Third time is a charm
@SquirtleSquadLeader
Copy link
Contributor Author

@ladyada Taking your advice and going small. I pre-wrote a little test script and enclosed it in the header. Hope that helps.

tekktrik
tekktrik requested changes Nov 5, 2022
View reviewed changes
tekktrik
tekktrik requested changes Nov 5, 2022
View reviewed changes
tekktrik
tekktrik reviewed Nov 5, 2022
View reviewed changes
@SquirtleSquadLeader
Copy link
Contributor Author

@tekktrik

I ran this through pre-commit - All checks PASSED. It looks like the error is in reference the original module syntax which has been unchanged. Maybe this is a pylint/Black error? I certainly don't mind pursuing if it is a legitimate error though. I appreciate you sticking this one out with me. First one is always the hardest.

tekktrik
tekktrik requested changes Nov 5, 2022
View reviewed changes
@SquirtleSquadLeader
Removed py:attr: decorators in some spots
3c1f0e2 
I added py:attr: to some original module Classes... maybe that was messing up linting/black
@SquirtleSquadLeader
Added :py:attr: decorators
6c1d945
tekktrik
tekktrik requested changes Nov 5, 2022
View reviewed changes
Copy link
Member

@tekktrik tekktrik left a comment

Choose a reason for hiding this comment

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

Looks like we're past the CI! 🥳

Just a few actual changes suggested, but otherwise looks good!

@SquirtleSquadLeader
Clksel class now ClockSource
c84e840 
Changed references throughout module too
@SquirtleSquadLeader
Added method description
9b9c0a6 
Hopefully these comments are suitable.
@SquirtleSquadLeader
Copy link
Contributor Author

Looks like we're past the CI! 🥳

Just a few actual changes suggested, but otherwise looks good!

Hopefully the last commit is good 🙏

@SquirtleSquadLeader
Copy link
Contributor Author

@tekktrik sorry meant to give a shout out to you when I made that last comment.

I'm not sure how long the merge process entails but I'll check back in a few days. I have a lot more to commit now that I know the appropriate format.

SquirtleSquadLeader
SquirtleSquadLeader commented Nov 6, 2022
View reviewed changes
Copy link
Contributor Author

@SquirtleSquadLeader SquirtleSquadLeader left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Complete

Edit: sorry I was trying to get rid of the reviews red x. 😅

tekktrik
tekktrik requested changes Nov 6, 2022
View reviewed changes
Copy link
Member

@tekktrik tekktrik 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 great! Just a few final tweaks and I think we're all set!

@SquirtleSquadLeader
Corrections
b53fec5 
- Fixed capitol L's
- Getter/Setter more clearly stated
tekktrik
tekktrik requested changes Nov 7, 2022
View reviewed changes
@SquirtleSquadLeader @tekktrik
Update adafruit_mpu6050.py
805cad6 
Co-authored-by: Alec Delaney <[email protected]>
tekktrik
tekktrik approved these changes Nov 7, 2022
View reviewed changes
Copy link
Member

@tekktrik tekktrik 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 great, thanks for all this!

@tekktrik tekktrik merged commit cecbdcb into adafruit:main Nov 7, 2022
@SquirtleSquadLeader
Copy link
Contributor Author

This looks great, thanks for all this!

@tekktrik - no worries. I appreciate you sticking it out 😄. This was my first ever successful PR. I have quite a bit more to contribute for this sensor. I just need to get ahold of git and how you guys need everything styled (which I feel like I do have a better understanding now). Thanks, look forward to working with you soon. 👍

adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Nov 7, 2022
@adafruit-adabot
Automated update by Adabot (adafruit/adabot@de028a7)
ffa34ce 
Updating https://github.com/adafruit/Adafruit_CircuitPython_MPU6050 to 1.2.0 from 1.1.16:
  > Merge pull request adafruit/Adafruit_CircuitPython_MPU6050#29 from SquirtleSquadLeader/Clock-Source-Integration
  > Update .pylintrc for v2.15.5
  > Fix release CI files
  > Update pylint to 2.15.5
  > Updated pylint version to 2.13.0
  > Switching to composite actions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tekktrik tekktrik tekktrik 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.
2 participants
@SquirtleSquadLeader @tekktrik