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

calibrate call offsets internal integration #56

Closed
mycosd opened this issue Jul 25, 2024 · 10 comments · Fixed by #59
Closed

calibrate call offsets internal integration #56

mycosd opened this issue Jul 25, 2024 · 10 comments · Fixed by #59
Assignees
Labels
bug Something isn't working

Comments

@mycosd
Copy link

mycosd commented Jul 25, 2024

Hi,

I've tested the GY521_pitch_roll_yaw example and noticed a 30° offset on the roll axis right from the start. Playing around I also noticed that increasing the calibrate times argument, just makes it worse.

There is an integration with class member variables going on in read, and since calibrate calls it, this integration is offset by a constant factor of times.

I think at the end of calibrate the variables _gax, _gay and _gaz should be reset to zero. And also they should be set to zero in the constructor to begin with. I only skimmed through the code, so there is a good chance that this suggestion could be wrong.

Maik

@RobTillaart RobTillaart self-assigned this Jul 25, 2024
@RobTillaart
Copy link
Owner

RobTillaart commented Jul 25, 2024

Thanks for reporting the issue.

There is a drift in the pitch roll yaw math too, and it might be related. (See issue #47).

I need to find a large timeslot to dive into the datasheet and the math.
So it is on my list however it might take till the long winter evenings.

@RobTillaart
Copy link
Owner

I think at the end of calibrate the variables _gax, _gay and _gaz should be reset to zero. And also they should be set to zero in the constructor to begin with. I only skimmed through the code, so there is a good chance that this suggestion could be wrong.

There is a good chance that your suggestion is right!
I label this one as bug and check if these 3 variables are initialized.
This can be verified relative fast so should not be postponed to December :)

@RobTillaart RobTillaart added the bug Something isn't working label Jul 25, 2024
@RobTillaart
Copy link
Owner

Printing the _gax _gay _gaz in the begin() function shows the UNO compiler sets them to zero.
However other compilers might behave differently - especially with optimizations.
In short ==> explicit set them to zero in constructor and in reset().

@RobTillaart
Copy link
Owner

Created develop branch with the improved initialization.

to be continued

@RobTillaart
Copy link
Owner

There is an integration with class member variables going on in read, and since calibrate calls it, this integration is offset by a constant factor of times.

idea is to split read() in the actual reading of the sensor and doing the math ==> readRaw().
Then calibrate() can call readRaw().

@RobTillaart
Copy link
Owner

RobTillaart commented Jul 25, 2024

@mycosd

There is an integration with class member variables going on in read, and since calibrate calls it, this integration is offset by a constant factor of times.

Split off readRaw() from read() as private function.
This is now used by calibrate() instead of read(), so the integration code of read() is not executed (x times).
So it should behave better.

If you have time, can you verify if the develop branch works better than the 0.6.0 release?
(no HW nearby to test for real).

Thanks

@RobTillaart
Copy link
Owner

I think at the end of calibrate the variables _gax, _gay and _gaz should be reset to zero.

Need to think about this, makes sense as it accumulated its value pre-calibration.

@mycosd
Copy link
Author

mycosd commented Jul 25, 2024

I tested the develop branch, but unfortunately something in there is completely broken. On master I see an offset of 30° on roll, but on develop the offset is:

pitch -2300°
roll +71000°
yaw -1100°

Also the values are moving at very high rates, something like 250° per second even though the board is not moving at all.
I use normalize=false to see the absolute offset.

@RobTillaart
Copy link
Owner

Ok, thanks for testing,
Food for thought, as said before need big timeslot ...

@mycosd
Copy link
Author

mycosd commented Jul 25, 2024

I did some digging, the reason for the issue now is:
The error variable (axe,gxe,...) is now unscaled, but in read it is treated as scaled. Simplest fix I could come up with, is changing the scaling in calibrate like that:

  float factor = 1.0 / times;
  axe = _axe * factor * _raw2g;
  aye = _aye * factor * _raw2g;
  aze = _aze * factor * _raw2g;
  gxe = _gxe * factor * _raw2dps;
  gye = _gye * factor * _raw2dps;
  gze = _gze * factor * _raw2dps;

alternatively, you can change the order in read, so it adds the error before scaling... that might be even better, as you don't add any computation. But this will break backward compatibility for those who have hardcoded error values.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants