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

Implement QMI8658 gyro/accelerometer support #1161

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

DvdGiessen
Copy link
Contributor

@DvdGiessen DvdGiessen commented Sep 29, 2024

This implements basic support for the QMI8658 IMU, which is for example used on a number of off-the-shelf Waveshare RP2040 boards (I'm using this one, see my branch with the board config here).

This is a very basic implementation, making use of the MIT-licensed code from Waveshare to implement communications with the IMU. It uses the GP2040 I2C abstraction, and I've added a simple configuration for it that for now only allows enabling/disabling it.

Sharing it here as a draft PR; in case I don't find the time to finish polishing this up with extra settings, calibration, etc it might hopefully still be useful to others. :)

@mikepparks
Copy link
Contributor

Glad to see someone taking advantage of the new Aux structures. Given that we're in the middle of rolling a release, this will likely need to move off to the next version to give us time to get hardware to check it out.

  • Good catch on the Relative sensor change. I added Relative fairly late in the dev work and didn't update those.
  • I would suggest fixing the indentation issue you have for optional QMI8658Options qmi8658Options = 28; in config.proto to match the others.
  • It's preferred that we avoid the old naming for I2C terms, preferring "target" or "device".
  • Debug output functions such as printf should either be removed or excluded via a conditional define for their usage, as they can potentially impact performance.

@DvdGiessen
Copy link
Contributor Author

Thanks for the feedback!

  • I would suggest fixing the indentation issue you have for optional QMI8658Options qmi8658Options = 28; in config.proto to match the others.

Fixed! I didn't notice my editor used .editorconfig to switch to tabs here; already had to fight it a bit for example due to the trim trailing whitespace option resulting in a lot of unrelated whitespace changes I had to exclude from my commits. :) Might make for another small but useful PR similar to #436.

  • It's preferred that we avoid the old naming for I2C terms, preferring "target" or "device".
  • Debug output functions such as printf should either be removed or excluded via a conditional define for their usage, as they can potentially impact performance.

I copied the QMI8658 library directly from Waveshare, so I didn't look into cleaning these up yet. I've addressed these two points for now; there's more things in these files that could be cleaned up but at least for now it is somewhat functional.

@arntsonl
Copy link
Contributor

This is a cool add-on, @DvdGiessen what do you think it would take to get this one done? I like the functionality!

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