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

MPU6050 i2c example expanded into library #504

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

NirajPatelRobots
Copy link

@NirajPatelRobots NirajPatelRobots commented Jul 3, 2024

I refactored and expanded the mpu6050_i2c example. This adds new functions for MPU6050 features and clarifies existing ones. It also refactors the example into an MPU6050_i2c library and main.c. There are C and C++ versions of the library. I've been using the C++ library in another project for around a year.

I realize that this is a pretty drastic change, and moves the code away from its original intent as a pico code example. I plan on separating this into a dedicated mpu6050_i2c pico library. However, some of these changes could be useful for the pico-examples repo. Some possibilities are:

  • Adding only certain features like mpu6050_power and the scale functions
  • Using only the refactor that defines constants for the register addresses
  • Removing the C++ part, or moving it to another directory
  • Refactoring out the library, so it's one .c and one .h file again

I'm happy to include any suggestions you have.

@peterharperuk
Copy link
Contributor

Thanks. I have a so far untested MPU6050, I'll give it a go.

@lurch
Copy link
Contributor

lurch commented Jul 3, 2024

Presumably this also fixes / supersedes the issues identified with the current example code? https://github.com/raspberrypi/pico-examples/issues?q=is%3Aopen+MPU6050

@NirajPatelRobots
Copy link
Author

@lurch ,
Several issues are related to the DEVICE_RESET value. The original example used 0x6B 0x00 in mpu6050_reset(). The register map says to set DEVICE_RESET =1 (0x6B 0x80) to reset the registers, so #319 implemented that. However, #352 and #494 reported that the example didn't work after #319 was merged. The discussion in 319 eventually determined that 0x80 is resetting the MPU6050, which causes the MPU6050 to enter sleep mode, which explains why the example stopped working.
Several commenters in #319 suggest writing DEVICE_RESET =1, waiting, then setting the register to 0x00 again. That works because DEVICE_RESET and SLEEP are in the same register, so sleep mode is disabled.
I added a mpu6050_power() function after mpu6050_reset() to exit sleep mode.

@NirajPatelRobots
Copy link
Author

I've pushed some changes to improve it, including adding a test that demonstrates scaling with the C++ library. Unfortunately, my pico has stopped working, so I can't test it myself.

@NirajPatelRobots
Copy link
Author

fixed my pico, then fixed the code.

@NirajPatelRobots NirajPatelRobots marked this pull request as ready for review September 20, 2024 01:07
@NirajPatelRobots
Copy link
Author

@peterharperuk thanks for waiting, I've marked this PR as ready. I'm still unsure of the best way to integrate this into the repo. Should I wrap the C++ section to check for compilers without C++?

@NirajPatelRobots
Copy link
Author

Resolves #352 , #494 , and #557

@peterharperuk
Copy link
Contributor

Can you please squash all your change into one commit? For some reason I can't seem to checkout your changes.

@NirajPatelRobots
Copy link
Author

@peterharperuk I rebased and squashed. The develop_rebased branch has the commit history if that becomes easier to checkout.

@NirajPatelRobots NirajPatelRobots marked this pull request as draft October 31, 2024 05:00
@NirajPatelRobots NirajPatelRobots marked this pull request as ready for review October 31, 2024 23:47
@NirajPatelRobots
Copy link
Author

@peterharperuk figured it out, my editor had been adding CRLF instead of LF. Sorry about that! I've rebased my master branch, that should be usable now.

@lurch
Copy link
Contributor

lurch commented Nov 4, 2024

@NirajPatelRobots This PR has ballooned back up to 12 separate commits again 😕

@NirajPatelRobots
Copy link
Author

@lurch sorry about that, I've re-squashed it.

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