Skip to content
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

Set fifo rate and mpu addr #69

Merged
merged 5 commits into from
Dec 13, 2023

Conversation

whyameye
Copy link

2 small changes to dmpInitialize():

  • before this change dmpInitialize() would set the FIFO rate at 100hz (hardcoded). This commit adds an optional parameter to set the FIFO rate and defaults to 100hz if no parameter is given. Rate is calculated by (200/(1+x)) where x is the optional parameter and defaults to 1.
  • before this change dmpInitialize() assumes the MPU6050 address is 0x68 (hardcoded). This commit adds an optional parameter to set the address and defaults to address 0x68 if no address is given.

slowing down the FIFO rate was my workaround to address issue #16. Theis commit maintains backward compatibility.

@sabas1080 sabas1080 added the Feature request New feature or request label Dec 12, 2023
@sabas1080 sabas1080 self-requested a review December 12, 2023 19:11
Copy link
Member

@sabas1080 sabas1080 left a comment

Choose a reason for hiding this comment

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

@whyameye thank you for your contribution, please check the unit tests or the github action, it seems that the structure of the functions has not been completely changed.

@whyameye
Copy link
Author

the only failures I'm seeing are all related to the example I pushed in the last PR and that's because that code is specifically written for the LGT8F328p. What am I not seeing specifically?

@whyameye
Copy link
Author

oops sorry I see the errors now. I'll look into it.

@xpeqex
Copy link
Member

xpeqex commented Dec 13, 2023

@whyameye

No worries, we will be waiting for the changes.

@whyameye
Copy link
Author

I was too hasty. Still not right even though checks passed. Don't merge yet.

@whyameye
Copy link
Author

OK I think this PR is good now. Let me know if you see any other problems.

Copy link
Member

@DeimosHall DeimosHall left a comment

Choose a reason for hiding this comment

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

LGMT

@DeimosHall DeimosHall merged commit ce4d7e2 into ElectronicCats:master Dec 13, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature request New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants