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

QST QMC5883L cleanup/rewrite #14384

Merged
merged 1 commit into from Sep 2, 2020
Merged

QST QMC5883L cleanup/rewrite #14384

merged 1 commit into from Sep 2, 2020

Conversation

dagar
Copy link
Member

@dagar dagar commented Mar 13, 2020

Similar to #14383, this PR is a cleanup/rewrite of the QST QMC5883L driver.

  • simple state machine to reset, configure, etc (all sleeps removed)
  • checked register mechanism
  • if you disconnect and reconnect the sensor it will detect the bad settings and reconfigure itself
  • old driver had a SPI interface (that doesn't physically exist)

TODO:

  • review ODR selection
  • review range
  • review rotation (test on mRo GPS, etc)

Screen Shot 2020-03-13 at 12 56 47 AM

					// Sensor orientation
					//  Forward X := +X
					//  Right   Y := -Y
					//  Down    Z := -Z

@dagar dagar added this to the Release v1.12.0 milestone Mar 18, 2020
@dagar dagar changed the title WIP: QST QML5883L cleanup/rewrite QST QML5883L cleanup/rewrite May 26, 2020
@dagar dagar marked this pull request as ready for review May 26, 2020 12:37
@dagar dagar force-pushed the pr-QMC5883L_new branch 3 times, most recently from 47f0c27 to 228e0d2 Compare June 22, 2020 16:08
@dagar dagar changed the title QST QML5883L cleanup/rewrite QST QMC5883L cleanup/rewrite Aug 25, 2020
@dagar
Copy link
Member Author

dagar commented Aug 25, 2020

@rolandash
Copy link
Contributor

@rolandash can you comment on the usage of the qmc5883l on the mindpx2?

You mean in the context of this PR?

@dagar
Copy link
Member Author

dagar commented Aug 28, 2020

@rolandash can you comment on the usage of the qmc5883l on the mindpx2?

You mean in the context of this PR?

Yes, is it actually used (internally) and how is it mounted?

@rolandash
Copy link
Contributor

rolandash commented Aug 29, 2020

@rolandash can you comment on the usage of the qmc5883l on the mindpx2?

You mean in the context of this PR?

Yes, is it actually used (internally) and how is it mounted?

Yes the new shipments have changed to QMC5883.
It is a drop-in replacement to HMC5883, no changes to PCB. The mounting is exactly the same.

It works fine on v1.10.x. However I did not test against this PR yet.

@dagar
Copy link
Member Author

dagar commented Aug 30, 2020

Bench tested with an mRo GPS unit.

With the orientation from the datasheet preserved (X = X) the mRo direction of flight is rotation 10 (ROTATION_ROLL_180_YAW_90), which mag cal auto rotation correctly found and set.

INFO  [commander] [cal] External Mag: 1 (396809), no rotation change: 0
INFO  [commander] [cal] External Mag: 2 (527649), determined rotation: 10
INFO  [sensor_calibration] MAG 396825 EN: 1, offset: [-0.035 -0.010 -0.157], scale: [ 0.989  1.000  1.023], Internal
INFO  [sensor_calibration] MAG 396809 EN: 1, offset: [ 0.055  0.034 -0.104], scale: [ 0.984  0.998  1.007], External ROT: 0
INFO  [sensor_calibration] MAG 527649 EN: 1, offset: [-0.417 -0.055 -0.008], scale: [ 0.976  1.024  1.005], External ROT: 10
INFO  [vehicle_magnetometer] selected magnetometer: 396809 (1)
INFO  [data_validator] validator: best: 1, prev best: 1, failsafe: NO (0 events)
INFO  [data_validator] sensor #0, prio: 50, state: OK
INFO  [data_validator]  val:   0.1754, lp:   0.1763 mean dev:  -0.0005 RMS:   0.0023 conf:   1.0000
INFO  [data_validator]  val:  -0.1402, lp:  -0.1385 mean dev:  -0.0000 RMS:   0.0014 conf:   1.0000
INFO  [data_validator]  val:   0.3784, lp:   0.3733 mean dev:  -0.0001 RMS:   0.0049 conf:   1.0000
INFO  [data_validator] sensor #1, prio: 75, state: OK
INFO  [data_validator]  val:   0.1694, lp:   0.1681 mean dev:  -0.0003 RMS:   0.0019 conf:   1.0000
INFO  [data_validator]  val:  -0.1267, lp:  -0.1276 mean dev:  -0.0001 RMS:   0.0014 conf:   1.0000
INFO  [data_validator]  val:   0.3789, lp:   0.3782 mean dev:  -0.0011 RMS:   0.0044 conf:   1.0000
INFO  [data_validator] sensor #2, prio: 75, state: OK
INFO  [data_validator]  val:   0.2096, lp:   0.2092 mean dev:   0.0280 RMS:   0.0569 conf:   1.0000
INFO  [data_validator]  val:  -0.1396, lp:  -0.1386 mean dev:  -0.0403 RMS:   0.0823 conf:   1.0000
INFO  [data_validator]  val:   0.3942, lp:   0.3971 mean dev:  -0.0252 RMS:   0.0512 conf:   1.0000
INFO  [sensor_calibration] MAG 396825 EN: 1, offset: [-0.035 -0.010 -0.157], scale: [ 0.989  1.000  1.023], Internal
INFO  [sensor_calibration] MAG 396809 EN: 1, offset: [ 0.055  0.034 -0.104], scale: [ 0.984  0.998  1.007], External ROT: 0
INFO  [sensor_calibration] MAG 527649 EN: 1, offset: [-0.417 -0.055 -0.008], scale: [ 0.976  1.024  1.005], External ROT: 10

@dagar
Copy link
Member Author

dagar commented Aug 30, 2020

@rolandash if you could quickly verify this on a mindracer-v2 we should be good to merge.

@dagar dagar merged commit 24125b3 into master Sep 2, 2020
@dagar dagar deleted the pr-QMC5883L_new branch September 2, 2020 17:16
@ryanjAA
Copy link
Contributor

ryanjAA commented Sep 2, 2020

Will this make it to 1.11? I know it says 1.12 but this is a pretty common HMC5883 replacement, we can physically flight test it this week if need be.

Have ~10x 1.11 rc3 flight tests with this compass (external) completed already this week so can compare against that if need be. If bench test is only needed, we can do as well.

@dagar
Copy link
Member Author

dagar commented Sep 2, 2020

Will this make it to 1.11? I know it says 1.12 but this is a pretty common HMC5883 replacement, we can physically flight test it this week if need be.

No, but we're going to be doing releases much more frequently going forward. Smaller (safer) incremental updates so upgrading isn't such a monumental task.

@ryanjAA
Copy link
Contributor

ryanjAA commented Sep 3, 2020

I’m a big fan of incremental changes. Will definitely make upgrading easier. Ok. Sounds good.

@rolandash
Copy link
Contributor

@rolandash if you could quickly verify this on a mindracer-v2 we should be good to merge.

Sry @dagar , I just do not have a qmc5883 test unit right now.
I will have a new qmc5883 test unit next week.

@rolandash
Copy link
Contributor

@rolandash if you could quickly verify this on a mindracer-v2 we should be good to merge.

@dagar a quick test shows the qmc5883 driver on latest master gives wrong direction which is 180 degree reverted of true north.

I am using a mind racer-v2 with qmc5883.

@dagar
Copy link
Member Author

dagar commented Sep 13, 2020

@rolandash if you could quickly verify this on a mindracer-v2 we should be good to merge.

@dagar a quick test shows the qmc5883 driver on latest master gives wrong direction which is 180 degree reverted of true north.

I am using a mind racer-v2 with qmc5883.

Sorry about that, quickly fixed in master. 1984663

@rolandash
Copy link
Contributor

thank you @dagar , that fixed it.

@MikeCharikov
Copy link

@dagar
Hi, Daniel!

We have met strange behaviour of this driver while migrating from 1.11 to 1.12\1.13. Often Driver regulary sticks for ~400ms. While diving into the code we have discovered that configuration registers values have been changed.

Here it is what is in 1.12 - 1.13 - Master code right now:

`// CNTL1
enum CNTL1_BIT : uint8_t {
// OSR[7:6]
OSR_512 = Bit7 | Bit6, // 00

// RNG[5:4]
RNG_2G          = Bit5 | Bit4, // 00

// ODR[3:2]
ODR_50HZ        = Bit2,        // 01

// MODE[1:0]
Mode_Continuous = Bit0,        // 01

};`

Looks like this two lines of code is incorrect

OSR_512 = Bit7 | Bit6, // 00
RNG_2G = Bit5 | Bit4, // 00

This values actually should be equal to 00 and 00, according to datasheet https://datasheet.lcsc.com/szlcsc/QST-QMC5883L-TR_C192585.pdf

But "Bit7 | Bit6" means OSR_64 and "Bit5 | Bit4" means "Reserved"

@dagar
Copy link
Member Author

dagar commented Jun 6, 2023

@MikeCharikov it's maybe a bit unclear, but OSR_512 and RNG_2G are cleared bits. I'll make it more explicit.

// Register | Set bits, Clear bits
{ Register::CNTL1, CNTL1_BIT::ODR_50HZ | CNTL1_BIT::Mode_Continuous, CNTL1_BIT::OSR_512 | CNTL1_BIT::RNG_2G},
{ Register::CNTL2, CNTL2_BIT::ROL_PNT, CNTL2_BIT::SOFT_RST},
{ Register::SET_RESET_PERIOD, SET_RESET_PERIOD_BIT::FBR, 0 },

I've added some clarification in #21693, can you fix that a try and we'll continue debugging from there?

@MikeCharikov
Copy link

Thanks, will try to test it ASAP

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants