search

copterust / mpu9250

no_std driver for the MPU9250 & onboard AK8963 (accelerometer + gyroscope + magnetometer IMU)
Apache License 2.0
28 stars 13 forks source link

Support for more MPU devices #4

Open fmckeogh opened 5 years ago

fmckeogh commented 5 years ago

I've been working on a fork of this repo that adds support for the MPU6050 and MPU6000. This means I have had to add an Interface trait implemented by I2C and SPI so that the library can work with both. However, I am unsure of how clear this is, especially since modules are either one interface or the other. As an example, let mpu6050 = Mpu9250::imu_default(i2c, delay).unwrap(); just doesn't seem very nice to me, maybe let mpu6050 = MpuX::imu_default(i2c, delay).unwrap();? The problem is that internally, there is lots of shared behaviour which would be nice not to duplicate.

What are your thoughts on this? Is it something you would like to see merged, or should I develop it as its own crate?

little-arhat commented 5 years ago

@chocol4te thanks for your interest in this crate, we would definitely love to see fixes and improvements.

Adding i2c would mean major version bump, so changing top level name is ok.

As for sharing functionality, I guess it make sense to create Transport struct/trait that would take care of writing/reading data and then use that object in MpuX.

mciantyre commented 5 years ago

Semi-related: I’ve been adding I2C support for the current family of MPU sensors in my fork. I generalized the existing implementation on a Device trait, which is similar to the Interface trait described above. Then, I added functions to construct an I2C-based sensor that are behind an ”i2c” Cargo feature-flag. The end result is an internal implementation that supports both SPI and I2C with an unchanged public API, so we don’t necessarily need a major version bump right away. Users could opt-in to I2C support as desired.

My use-case is MPU support on the BeagleBone Blue, which has an MPU9250 on I2C2. I’d love to collaborate on the shared SPI/I2C approaches!

little-arhat commented 5 years ago

@mciantyre that sounds awesome!

indeed, we can publish minor version bump with feature flag for i2c, and then remove it, as it doesn't seem too hard — we just need to provide ergonomic constructors.

Feel free to send pull request, would love to merge your changes and build on top!