Mpu6500 support

[JohannesWilde]

I tried to use a MPU-6500 and stumbled upon your great library. The only problems I saw were, that

• the init-routine always failed,
• I only received 0-values for the magnetometer.

Once I understood that this is due to the MPU-6500 having a different identification value [WHO_AM_I] and no magnetometer, I came up with this sketch. From a conceptual point of view I would have liked to design this the other way round, i.e. MPU5250 should have inherited from MPU6500 - but I did not want to redesign everything without having asked your views regarding this. Do you have some experience with the MPU-6500? Mine is not too deep and the proposed changes come from a quick comparison of the register map descriptions of both devices. They seem - apart from some small naming differences - to be identical.

[wollewald]

Hi @JohannesWilde, I don't have experience with the MPU6500. In fact I was not aware of it at all. A very short look into the data sheet also gave me the impression that it just the MPU9250 minus magnetometer and with different WHO_AM_I. I would not care who inherits from whom. The only thing I want to keep is full backwards compatibility. That means that people who already use my lib shall not experience any issue with an update. How do you want to proceed? I can test your code once I get the MPU6500.

[JohannesWilde]

Hi wollewald, thanks for the fast reply. What I would do: once I find the time - probably next weekend? - I would create a redesign. Then I would like to ask for your review. If you will receive an MPU6500 in the near future, I even would wait for you to actually test my changes. What do you think?

[wollewald]

Sounds like plan - thanks for investing the time!

[JohannesWilde]

Hi @wollewald , I managed to find some time this weekend and implemented my proposed changes. I tried to keep everything backwards-compatible, which is why:

• The enums in the MPU6500 files still contain the MPU9250-parts in their names.
• All of the #defines in the original MPU9250 header still exist - allthough they now simply relay to the static constexpr T variables of the respective classes. I do like those to be correctly typed...

I used your _MPU9250_blank_allsettings.ino script, somewhat adapted to my MPU6500, to test that I won't increase program space by too much. Turns out, for the MPU9250 on an Arduino Uno I got

• from: 9940 Bytes program storage and 739 Bytes of data
• to: 9600 Bytes program storage and 714 Bytes of data.

• Because of these considerations I would have two remarks:

• Using local variables instead of member variables in a member function reduced both program storage and data. I think this is because the Arduino then does not have to do so many ld [load] and st [store] operations when assigning that variable and instead can maybe even keep those values in registers.
• Contrary the introduction of inheritance leads to more program space usage and probably also to more function calls [but mostly only for constructors?!?] - I don't think that should be relevant for runtime behaviour.

So if you find the time to test and maybe even look at my changes, I would be glad about feedback. If there is anything I messed up, please let me know - I will try to fix it.

Thanks.

[JohannesWilde]

@wollewald I don't know whether the following is specific to my MPU6500 - but I wanted to ask anyway:

First I noticed the line accOffsetVal.z -= 16384.0;. I suppose this rotates the z-Axis?

When I played around with my MPU6500, I noticed the following:

• Compared to the x,y,z-coordinates [assuming z points up] printed on my device the ones returned by your library for the acceleration values [gValue = getGValues();] after calling autoOffsets() were inverted [allthough accordingly still right-handed].
• When comparing those values to the ones returned by gyr = getGyrValues(), I observed that when turning the device counter-clockwise around the z-axis [assuming z pointing upwards, i.e. the library showing an acceleration of gValue.z = 1], the gyr.z was greater than 0. This matches with my expectations of the right-hand-rule for an upwards-pointing z-axis. This is however not consistent with gValue.z > 0 pointing downwards.

Is this a misconception on my part? Did I do something wrong?

Does this problem only occur with the MPU6500 and not with the MPU9250?

Should I move this to a separate issue?

I would be glad about any feedback.

[wollewald]

Hi @JohannesWilde ,

first of all thanks for your hard work! I will try to test your changes latest next weekend. I don't have the MPU6500 yet, therefore I can only test if the MPU9250 still works.

accOffsetVal.z -= 16384.0; is part of the procedure to "calibrate" the MPU9250. You position your device flat in the x,y plane. The acceleration values of x and y should be zero in this position. 1 g should act on z. During the the calibration procedure I switch to the 2 g range. So 1g should result in a raw value of +16384 (= 2^15). The difference to that number is the offset. That means it does not change any axis or so.

Let me check when I receive my MPU6500 modules if I notice any differences.

Thanks again! Regards, Wolfgang

[wollewald]

Hi @JohannesWilde, I have just received the MPU6500 modules I have ordered. I have not yet tried your extention, I wanted first to check what happens with my original lib. I also see the Init procedure and the magnetometer does not work. So exactly what you experienced.

Then I had look into the question you raised regarding the axes and directions, but I am not sure I have fully understood the issue. What I can say is that my MPU6500 modules and the MPU9250 behave the same and the autoOffset function does not change anything with regards to directions.

When the module is positioned flat, the x-axis and y-axis are horizontal and z-axis is vertical (positive z-axis showing upwards). As expected acceleration values for the x- and y-axis are zero (more or less) and the z-axis is roughly 1g. If I turn the y-axis upwards, so that it's vertical I get 1g as expected and 0 for the z and x. Same behavior for the x-axis. So, everything is fine with the acceleration values, would you agree?

Now to the gyroscope values. Whenever I turn the module clockwise around an axis (looking into the positive direction) I get positive gyroscope values. For counter-clockwise movements they are negative. And this is independent from the autoOffset. So when the module is positioned flat (x and z horizontal, z showing upwards) and I turn the module clockwise looking from above (which means counter-clockwise when looking into the positive z-axis) the gyroscope delivers negative value.

Can you tell me again what is changing when you apply the autoOffset function?

As I said I have not tested your changes yet. Ihope I get this done next weekend.

[wollewald]

Hi @JohannesWilde , I tried your updated version with the MPU6500 and with the MPU9250 and it works well. Great Job!

It looks a bit strange for the users to use enums which contain "MPU9250" in their names for their MPU6500 module. It works, but it's not very nice. I have tried this and it works:

typedef enum MPU9250_ACC_RANGE {
    MPU9250_ACC_RANGE_2G = 0, MPU9250_ACC_RANGE_4G = 1, MPU9250_ACC_RANGE_8G = 2, MPU9250_ACC_RANGE_16G = 3, 
    MPU6500_ACC_RANGE_2G = 0, MPU6500_ACC_RANGE_4G = 1, MPU6500_ACC_RANGE_8G = 2, MPU6500_ACC_RANGE_16G = 3
} MPU9250_accRange;

At least from the user side it would look better. Internally it would then even look better like this:

typedef enum MPU9250_6500_ACC_RANGE {
    MPU9250_ACC_RANGE_2G = 0, MPU9250_ACC_RANGE_4G = 1, MPU9250_ACC_RANGE_8G = 2, MPU9250_ACC_RANGE_16G = 3, 
    MPU6500_ACC_RANGE_2G = 0, MPU6500_ACC_RANGE_4G = 1, MPU6500_ACC_RANGE_8G = 2, MPU6500_ACC_RANGE_16G = 3
} MPU9250_6500_accRange;

The latter would mean of course more changes to do. This is something I could do after the merge.

The pull request is still draft. I would be happy to merge it.

And have you had the chance to look into my last comments regarding the axes?

[JohannesWilde]

Hi @wollewald , nice to hear my changes work. Regarding the coordinates-orientation-issue:

• You wrote:

  When the module is positioned flat, the x-axis and y-axis are horizontal and z-axis is vertical (positive z-axis showing upwards). As expected acceleration values for the x- and y-axis are zero (more or less) and the z-axis is roughly 1g.

  which is exactly what I meant. Assuming the z-Axis points upwards, I would have expected "-1g", i.e. a negative value, as the gravitation is pointing downwards [into the opposite direction of the z-axis].

Right now I don't think the autoOffsets() is responsible for this - it is rather my concept of vectors...

• Regarding the enum names:

As you wrote earlier one goal should be to keep the code backwards compatible - which I totally agree to. That's what lead to those inconsistent names.

Regarding your first solution, all but the enum type name would then reflect the respective device. As a minor detail, I would prefer something like the following

typedef enum MPU9250_ACC_RANGE {
    MPU9250_ACC_RANGE_2G = 0, MPU9250_ACC_RANGE_4G = 1, MPU9250_ACC_RANGE_8G = 2, MPU9250_ACC_RANGE_16G = 3, 
    MPU6500_ACC_RANGE_2G = MPU9250_ACC_RANGE_2G,
    MPU6500_ACC_RANGE_4G = MPU9250_ACC_RANGE_4G,
    MPU6500_ACC_RANGE_8G = MPU9250_ACC_RANGE_8G,
    MPU6500_ACC_RANGE_16G = MPU9250_ACC_RANGE_16G
} MPU9250_accRange;

Your second proposal of MPU9250_6500_ACC_RANGE would be a non-backward-compatible change. Rather than this, I would leave out the MPU9250_ out of the name of the enum type [as well as the enum values] and instead put the enums inside the class declaration of the class MPU6500_WE. This would accordingly scope those enums. And the inheritance would allow the usage of those in the MPU9250_WE class as well. At the same time one would not have to write MPU9250_6500_ACC_RANGE in both the MPU6500_WE and the MPU9250_WE class methods.

If the last point did not become clear - please don't hesitate to ask again - then I could provide a small example.

And again: this is only my opinion.

If you want to change the enums before merging this request - fell free. Your first option should be fully backwards-compatible.

Other than that thank you for testing my code.

Have a nice weekend.

[wollewald]

Hi @JohannesWilde , great thanks for your contribution!. I merged first and take care for the enums later (in the next days).