-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Master.mag driver fixes #144
Master.mag driver fixes #144
Conversation
Many thanks for this fix! I'm curious what moving mpu6500SetClockSource(MPU6500_CLOCK_PLL_XGYRO); does? Also the commit "Check if mag data is ready before updating" might just increase i2c transactions as there will be a checking and updating a lot of the time. Isn't it better to get the same result twice or more instead and have a constant loading of the i2c bus? |
Moving the selection of the PLL clock probably doesn't do anything, but I saw it in a different location on an example driver and moved it during my testing. You make a good point about that commit. I changed it to match the scheme in sensors_task.c and didn't want to make any assumptions about the data in the magnetometer's registers when data wasn't ready, but at the same time you've been using that code prior to this without any issue. I can revert it when I get the barometer items working if you'd like. |
I will check with a i2c analyser to see how the reading behaves. Then we know if it is better to revert it. |
After checking the i2c log I can say it behaves pretty well so no need to revert it. One thought i got after looking att the sensors_task way of reading the magnetometer is to do the same in imu_cf2. That is reading x,y,z and ST2 (overflow) in one read. By the way, in the long run I think we should move the sensors_task way of reading using the MPU9250 master into imu_cf2 and make them use the same API and not have duplicated code. |
That's great to hear! I was thinking the same thing about moving a lot of the sensors task code over to the imu_cf2 module. There is a fair bit of cleanup that can be done on the slave reading and writing also. |
I've fixed the configuration of the mag so the values are updated properly now when using the sensors task and kalman estimator.
The barometer isn't working right now and I was planning on tackling that next.