Skip to content

Conversation

@JamesWhitlock
Copy link
Contributor

This PR adds support for non-default I2C masters #30. A TwoWire instance can now be optionally parsed into the contructor, e.g.

MPU6050 imu(Wire2)

Given all currently supported IMUs are I2C devices, I've put the TwoWire reference in I2CBase.

@JamesWhitlock
Copy link
Contributor Author

Hold off merging this. I just realised this doesn’t work for the hybrid imus. I will move the reference out of base.

@JamesWhitlock
Copy link
Contributor Author

Hold off merging this. I just realised this doesn’t work for the hybrid imus. I will move the reference out of base.

Done

@LiquidCGS
Copy link
Owner

LiquidCGS commented Nov 17, 2024

Hello, sorry for the wait but this does not build on IDE 1.8.19, I haven't tested it on 2.x but i don't expect it to build either

F_ICM20689.hpp: In constructor 'ICM20689::ICM20689(TwoWire&)':
F_ICM20689.hpp:87:53: error: no matching function for call to 'IMUBase::IMUBase()'
   87 |  explicit ICM20689(TwoWire& wire = Wire) : wire(wire) {};

and so on for pretty much every IMU.

@LiquidCGS
Copy link
Owner

LiquidCGS commented Nov 17, 2024

Changing
explicit ICM20689(TwoWire& wire = Wire) : wire(wire) {};
to
explicit ICM20689(TwoWire& wire = Wire) : IMUBase(wire), wire(wire) {};
in the constructor seems to do the trick for the IMU's, and changing
explicit ICM20689_AK8975(TwoWire& wire = Wire) : IMU(wire), MAG(wire) {};
to
explicit ICM20689_AK8975(TwoWire& wire = Wire) : IMUBase(wire), IMU(wire), MAG(wire) {}
in the hybrid IMU's also seems to do the trick.

However I don't have anything on hand that has multiple hardware i2c ports to test this with, I tried compiling it with an ESP32 as the board and it seems to take both "Wire" and "Wire1" in the constructor just fine but I don't have one handy to test with

@JamesWhitlock
Copy link
Contributor Author

Hi, I tested compilation of all IMUs on 2.3.3. A quick google suggests that the gcc version is the same between versions, so I'm not sure why it's failing. I'm just installing 1.8.9.

@JamesWhitlock
Copy link
Contributor Author

JamesWhitlock commented Nov 17, 2024

Right, my setup was wrong - sorry for the trouble. I'll just check the fix and push it when done.

@LiquidCGS
Copy link
Owner

No worries!

@JamesWhitlock
Copy link
Contributor Author

JamesWhitlock commented Nov 17, 2024

I pushed a fix, I removed the TwoWire reference and constructor out of the IMUBase class so that it remains as a pure interface. This makes sense for the IMU and compass combos and for non-I2C IMUs in future (e.g. SPI).

@LiquidCGS
Copy link
Owner

Looks and runs well now! I'll push an update to the arduino library manager version soon-ish after I do some cleanup

@LiquidCGS LiquidCGS merged commit 2e32299 into LiquidCGS:main Nov 17, 2024
1 check passed
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.

2 participants