kriswiner / MPU9250

Arduino sketches for MPU9250 9DoF with AHRS sensor fusion
1.04k stars 471 forks source link

initAK8963() the parameter _Mmode is shadowing the object's variable ? #260

Open TByte007 opened 6 years ago

TByte007 commented 6 years ago

void MPU9250::initAK8963(uint8_t _Mscale, uint8_t _Mmode, float * _magCalibration) this is the original non working definition (at least for me). _Mmode is used locally for the function and never set for the object. Then at the calibration routine the object's _Mmode is zero and of course it never works.

if I change the init to:

void MPU9250::initAK8963(uint8_t _Mscale, uint8_t Mmode, float * _magCalibration)
{
  _Mmode = Mmode;

It now appears to work and of course the global (for the object) variable _Mmode is set to 6 (as per your example).

kriswiner commented 6 years ago

Which sketch?

On Sat, Apr 7, 2018 at 1:54 PM, TByte007 notifications@github.com wrote:

void MPU9250::initAK8963(uint8_t _Mscale, uint8_t _Mmode, float * _magCalibration) this is the original non working definition (at least for me). _Mmode is used locally for the function and never set for the object. Then at the calibration routine the object's _Mmode is zero and of course it never works.

if I change the init to:

void MPU9250::initAK8963(uint8_t _Mscale, uint8_t Mmode, float * _magCalibration) { _Mmode = Mmode;

It now appears to work and of course the global (for the object) variable _Mmode is set to 6 (as per your example).

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/kriswiner/MPU9250/issues/260, or mute the thread https://github.com/notifications/unsubscribe-auth/AGY1qjoMVijbtB2LTdUZnV8KlKhDyCuIks5tmSeXgaJpZM4TLP8V .

TByte007 commented 6 years ago

in MPU9250.cpp

kriswiner commented 6 years ago

Looks like none of those should be globals...

On Sat, Apr 7, 2018 at 1:58 PM, Tlera Corporation tleracorp@gmail.com wrote:

Which sketch?

On Sat, Apr 7, 2018 at 1:54 PM, TByte007 notifications@github.com wrote:

void MPU9250::initAK8963(uint8_t _Mscale, uint8_t _Mmode, float * _magCalibration) this is the original non working definition (at least for me). _Mmode is used locally for the function and never set for the object. Then at the calibration routine the object's _Mmode is zero and of course it never works.

if I change the init to:

void MPU9250::initAK8963(uint8_t _Mscale, uint8_t Mmode, float * _magCalibration) { _Mmode = Mmode;

It now appears to work and of course the global (for the object) variable _Mmode is set to 6 (as per your example).

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/kriswiner/MPU9250/issues/260, or mute the thread https://github.com/notifications/unsubscribe-auth/AGY1qjoMVijbtB2LTdUZnV8KlKhDyCuIks5tmSeXgaJpZM4TLP8V .

TByte007 commented 6 years ago

_Mmode is defined as "global" for the object MPU9250 in MPU9250.h on line 233 as uint8_t _Mmode; And it is used in magcalMPU9250() to set the delay as in "if(_Mmode == 0x02) delay(135);" and the number of samples as in "if(_Mmode == 0x02) sample_count = 128;" . And if u dont set it in initAK8963() or as MPU9250._Mmode = M_100Hz (which will be double the work and why would u do that?) the calibration routine will never work as it will just skip the calibration cycle.

kriswiner commented 6 years ago

Thanks, been a while since I've used this...

On Sat, Apr 7, 2018 at 2:09 PM, TByte007 notifications@github.com wrote:

_Mmode is defined as "global" for the object MPU9250 in MPU9250.h on line 233 as uint8_t _Mmode; And it is used in magcalMPU9250() to set the delay as in "if(_Mmode == 0x02) delay(135);" and the number of samples as in "if(_Mmode == 0x02) sample_count = 128;" . And if u dont set it in initAK8963() or as MPU9250._Mmode = M_100Hz (which will be double the work and why would u do that?) the calibration routine will never work as it will just skip the calibration cycle.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/kriswiner/MPU9250/issues/260#issuecomment-379499416, or mute the thread https://github.com/notifications/unsubscribe-auth/AGY1qlCpC6CF8Rh3KgCPw-b3jYHZP0n9ks5tmSsXgaJpZM4TLP8V .

TByte007 commented 6 years ago

Is there a better library that you use then ? :)

kriswiner commented 6 years ago

You don't like this one?

On Sat, Apr 7, 2018 at 2:16 PM, TByte007 notifications@github.com wrote:

Is there a better library that you use then ? :)

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/kriswiner/MPU9250/issues/260#issuecomment-379499811, or mute the thread https://github.com/notifications/unsubscribe-auth/AGY1qnXjXJCc4zAXRgHq4yMS4v13Qu_Sks5tmSykgaJpZM4TLP8V .

TByte007 commented 6 years ago

It's the best so far that I've found. But u said that you haven't used it for a while. That's why I'm asking :)

kriswiner commented 6 years ago

I have dozens each for a different combo of sensors and a different MCU.

None for the Nano, though...I stopped using AVRs years ago.

On Sat, Apr 7, 2018 at 2:19 PM, TByte007 notifications@github.com wrote:

It's the best so far that I've found. But u said that you haven't used it for a while. That's why I'm asking :)

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/kriswiner/MPU9250/issues/260#issuecomment-379499995, or mute the thread https://github.com/notifications/unsubscribe-auth/AGY1qsWtF42-1rh_Xu54eJspZlKoCmeSks5tmS1SgaJpZM4TLP8V .

kriswiner commented 6 years ago

There is this one, I don't use it but maybe you'll like it better...

https://github.com/brianc118/MPU9250

On Sat, Apr 7, 2018 at 2:23 PM, Tlera Corporation tleracorp@gmail.com wrote:

I have dozens each for a different combo of sensors and a different MCU.

None for the Nano, though...I stopped using AVRs years ago.

On Sat, Apr 7, 2018 at 2:19 PM, TByte007 notifications@github.com wrote:

It's the best so far that I've found. But u said that you haven't used it for a while. That's why I'm asking :)

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/kriswiner/MPU9250/issues/260#issuecomment-379499995, or mute the thread https://github.com/notifications/unsubscribe-auth/AGY1qsWtF42-1rh_Xu54eJspZlKoCmeSks5tmS1SgaJpZM4TLP8V .

TByte007 commented 6 years ago

I intend to use the sensor(s) with STM32 which will do some of the job (all of the AHRS processing and filtering may be) and send it to some bigger ARM "machine" with more RAM (like RP or some of the clones) which will do the higher level processing. But it's easier for me to test the lib on Nano as it's hooked to the PC and need only one USB (no STLink) :)

kriswiner commented 6 years ago

These are what you need then:

https://www.tindie.com/products/TleraCorp/ladybug-stm32l432-development-board/

or

https://www.tindie.com/products/TleraCorp/dragonfly-stm32l47696-development-board/

On Sat, Apr 7, 2018 at 2:33 PM, TByte007 notifications@github.com wrote:

I intend to use the sensor(s) with STM32 which will do some of the job (all of the AHRS processing and filtering may be) and send it to some bigger ARM "machine" with more RAM (like RP or some of the clones) which will do the higher level processing. But it's easier for me to test the lib on Nano as it's hooked to the PC and need only one USB (no STLink) :)

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/kriswiner/MPU9250/issues/260#issuecomment-379500787, or mute the thread https://github.com/notifications/unsubscribe-auth/AGY1qm6qqaZWu9PtBXgymdlop1FSLoihks5tmTCqgaJpZM4TLP8V .

TByte007 commented 6 years ago

Yes those would be perfect for what im trying to do (although I'll try it first with F108 "bluepill" coz I have 3 :) ) . It's just that the shipping to EU doubles the costs :( (and I'm in the eastern part - read "we dont get as much as the western part of EU gets" so cost matters :) )

kriswiner commented 6 years ago

Shipping does cost what it costs, but you can ship as many items as you'd like for one shipping charge. Maybe you can get some of your friends interested to lower the per item shipping cost.

The Dragonfly L496 Vvariant has 1 MByte flash and 320 kByte SRAM. Might be all you need for your "big-boy" application.

On Sat, Apr 7, 2018 at 2:47 PM, TByte007 notifications@github.com wrote:

Yes those would be perfect for what im trying to do (although I'll try it first with F108 "bluepill" coz I have 3 :) ) . It's just that the shipping to EU doubles the costs :( (and I'm in the eastern part - read "we dont get as much as the western part of EU gets" so cost matters :) )

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/kriswiner/MPU9250/issues/260#issuecomment-379501568, or mute the thread https://github.com/notifications/unsubscribe-auth/AGY1qlukzfEUbPA9C88ccXu3Xal0vZBHks5tmTQDgaJpZM4TLP8V .

TByte007 commented 6 years ago

I will need lot more RAM coz it will work with camera images and CV (the bot have to recognize where he is in the room and reset the inertial guidance system's error) :) . That means few MB of RAM and relatively heavy libs most of which will require OS and good memory management. Btw those STM32L4s are really nice (I'm clicking around your store drooling lol) . But the price + shipping will get twice as high as OrangePi with H2+ with 256MB of RAM :) I wonder how the Chinese can manage such a low shipping rates - I get most things for free in like 10 days ...

Btw I found another problem with void MPU9250::initAK8963(uint8_t _Mscale, uint8_t _Mmode, float * _magCalibration) now it shadows the objects own "_magCalibration" (same names). I guess those are artifacts from switching to OOP. I quick fixed it by changing the declaration to void MPU9250::initAK8963(uint8_t _Mscale, uint8_t _Mmode, float * magCalibration) and then

  _magCalibration[0] =  (float)(rawData[0] - 128)/256.0f + 1.0f;   // Return x-axis sensitivity adjustment values, etc.
  _magCalibration[1] =  (float)(rawData[1] - 128)/256.0f + 1.0f;  
  _magCalibration[2] =  (float)(rawData[2] - 128)/256.0f + 1.0f; 

to

  magCalibration[0] = _magCalibration[0] =  (float)(rawData[0] - 128)/256.0f + 1.0f;   // Return x-axis sensitivity adjustment values, etc.
  magCalibration[1] = _magCalibration[1] =  (float)(rawData[1] - 128)/256.0f + 1.0f;  
  magCalibration[2] = _magCalibration[2] =  (float)(rawData[2] - 128)/256.0f + 1.0f; 

Otherwise the biases wont get calculated as _magCalibration[] will be all zeroes.

kriswiner commented 6 years ago

Yes, this is odd since I am sure this sketch used to work...

On Sat, Apr 7, 2018 at 3:14 PM, TByte007 notifications@github.com wrote:

I will need lot more RAM coz it will work with camera images and CV (the bot have to recognize where he is in the room and reset the inertial guidance system's error) :) . That means few MB of RAM and relatively heavy libs most of which will require OS and good memory management. Btw those STM32L4s are really nice (I'm clicking around your store drooling lol) . But the price + shipping will get twice as high as OrangePi with H2+ with 256MB of RAM :) I wonder how the Chinese can manage such a low shipping rates - I get most things for free in like 10 days ...

Btw I found another problem with void MPU9250::initAK8963(uint8_t _Mscale, uint8_t _Mmode, float _magCalibration) now it shadows the objects own "_magCalibration" (same names). I guess those are artifacts from switching to OOP. I quick fixed it by changing the declaration to void MPU9250::initAK8963(uint8_t _Mscale, uint8_t _Mmode, float magCalibration) and then _magCalibration[0] = (float)(rawData[0] - 128)/256.0f + 1.0f; // Return x-axis sensitivity adjustment values, etc. _magCalibration[1] = (float)(rawData[1] - 128)/256.0f + 1.0f; _magCalibration[2] = (float)(rawData[2] - 128)/256.0f + 1.0f;

to

magCalibration[0] = _magCalibration[0] = (float)(rawData[0] - 128)/256.0f + 1.0f; // Return x-axis sensitivity adjustment values, etc. magCalibration[1] = _magCalibration[1] = (float)(rawData[1] - 128)/256.0f + 1.0f; magCalibration[2] = _magCalibration[2] = (float)(rawData[2] - 128)/256.0f + 1.0f;

Otherwise the biases wont get calculated as _magCalibration[] will be all zeroes.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/kriswiner/MPU9250/issues/260#issuecomment-379502914, or mute the thread https://github.com/notifications/unsubscribe-auth/AGY1qk2eWIUuBhystMkboBMPux5gvDNaks5tmTpDgaJpZM4TLP8V .

TByte007 commented 6 years ago

I ended up finding out why it worked for you :) . If u preset the values of magBias and magScale (as u did) you end up never using the magcalMPU9250() function so it actually works in that case. The code needs cleaning coz some parts it seems are left inside the objects others are pulled outside and it became kind of a ... you know what :) . Btw using the interrupt and sleep/wake up never worked stable for me. I could see the magnetometer not working fro seconds then just waking up giving few different values then getting to bed again so I removed all the wake up/sleep parts for my case.