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.

Sign up for GitHub

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

Jump to bottom

driver/mpu9x50: make mpu9150 more generic #12103

Merged
merged 2 commits into from
Oct 17, 2019
Merged

driver/mpu9x50: make mpu9150 more generic #12103

merged 2 commits into from
Oct 17, 2019

Conversation

JannesVolkens
Copy link
Contributor

Contribution description

The driver for the MPU9150 motion sensor has been modified to work for the MPU9150 and MPU9250. The driver was tested using the CC2650-launchpad.

Testing procedure

The test under tests/driver_mpu9x50 can be performed for both sensors. The following parameter must be set for the MPU9250 sensor: DRIVER=mpu9250

Issues/PRs references

None

@benpicco
Copy link
Contributor

benpicco commented Aug 28, 2019 •
edited
Loading

Thank you for your patch! Can you please squash the commits so there is only one commit that does all the renaming and one to add support for the new chip? And of course if there are independent fixes to the driver, they should also have their own commit as well.

That should make this PR much easier to review.

@benpicco benpicco added Area: drivers Area: Device drivers Type: new feature The issue requests / The PR implemements a new feature for RIOT labels Aug 28, 2019
@MrKevinWeiss MrKevinWeiss self-requested a review August 28, 2019 15:28
@gschorcht
Copy link
Contributor

This PR is related to PR #9516. Please note the discussion there.

gschorcht
gschorcht reviewed Sep 5, 2019
View reviewed changes
Copy link
Contributor

@gschorcht gschorcht left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JannesVolkens Are there real changes in driver implementation or just renamings?

When I was looking to the register maps of both sensors some time ago, I figured out the following:

  • the ACCEL_CONFIG register is splitted into two registers (register 28 an 29). The additional register ACCEL_CONFIG2 only contains the settings ACCEL_FCHOICE_B and A_DLPF_CFG that seem to be new in MPU9250.
  • additional new configuration FCHOICE_B in GYRO_CONFIG register 26
  • in MPU9250, the FIFO mode can be configured by by an additional bit in register 26.

Did you check in detail that all the bits in new registers are not required to be handled?

@MrKevinWeiss
Copy link
Contributor

I should be able to run some tests on it. According to the manufacturer there shouldn't be a difference in basic usage, only low power stuff.

The MPU-6050 is higher power, lower noise, and larger package versus the MPU-6500. Most of your code should port over, but some low power features are different and will need to be recoded in. Basic data acquisitions shouldn’t have changed.
The MPU-9150 contains the MPU-6050 and an AK8975 magnetometer from AKM. The MPU-9250 contains a MPU-6500 and AK8963. The same differences between gyro/accel are the same you see with 6050 v. 6500. The magnetometer on the MPU-9250 is a little better across the board.

@MrKevinWeiss
Copy link
Contributor

It seems like there is a problem with the temperature readings. The formula is wrong for both devices I believe

The mpu-9150
TEMPERATURE SENSOR
Sensitivity Untrimmed = 340 LSB/ºC
Temperature Offset 35ºC = -521 LSB
Linearity Best fit straight line (-40°C to +85°C) ±1 °C

The mpu-9250
TEMPERATURE SENSOR
Sensitivity Untrimmed = 333.87 LSB/°C
Room Temp Offset 21°C = 0 LSB

@PeterKietzmann
Copy link
Member

@MrKevinWeiss do you think we could get this in before the soft feature freeze?

@MrKevinWeiss
Copy link
Contributor

I think this should be ready soon. I only have one day in the office but I looked over the code and if the formula and everything is fix and working I would just like @leandrolanzieri or someone to quick test it to see proper temperature values.

@gschorcht
Copy link
Contributor

@PeterKietzmann My question #12103 (review) is still not answered. When PR #9516 was provided, I used the former MPU9150 driver to test it with a MPU9250. At that time, the driver was working without any changes.

Therefore, I was asking @JannesVolkens whether the PR includes only renamings or whether there are real changes in the driver that would have to be checked. Furthermore, I wanted to know whether he checked the changes in the two registers I mentioned are not affected by the driver.

@JannesVolkens
Copy link
Contributor Author

I've had a look at the changed registers and these extra bits are additional features. If they are not used the driver for the MPU9150 also works for the MPU9250.
Also, I've changed the mpu9x50_read_temperature function to return the correct temperature for the MPU9250.

@MrKevinWeiss
Copy link
Contributor

So far things look pretty good. I will test after you rebase!

@JannesVolkens JannesVolkens force-pushed the mpu9x50 branch 6 times, most recently from cc0a343 to e1b205f Compare October 17, 2019 10:16
@JannesVolkens
driver/mpu9x50: Rename mpu9150 to mpu9x50
2df5d60 
Rename all files

Rename all variables, methods and methodcalls

Rename all folders

Add to the makefiles

Add to doc
@JannesVolkens JannesVolkens reopened this Oct 17, 2019
@JannesVolkens JannesVolkens force-pushed the mpu9x50 branch 2 times, most recently from d414b9f to b276dd1 Compare October 17, 2019 10:56
@MrKevinWeiss MrKevinWeiss added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Oct 17, 2019
@JannesVolkens
Copy link
Contributor Author 

2019-10-17 13:25:15,060 # Accel data [milli g] - X: -29   Y: 13   Z: -984
2019-10-17 13:25:15,064 # Gyro data [dps] - X: 0   Y: 0   Z: -19
2019-10-17 13:25:15,069 # Compass data [micro T] - X: 1   Y: 12   Z: -51
2019-10-17 13:25:15,072 # Temperature [milli deg] : 28922
2019-10-17 13:25:15,072 # 
2019-10-17 13:25:15,075 # +-------------------------------------+
2019-10-17 13:25:16,077 # Accel data [milli g] - X: 152   Y: 368   Z: -868
2019-10-17 13:25:16,081 # Gyro data [dps] - X: -2   Y: 10   Z: 13
2019-10-17 13:25:16,086 # Compass data [micro T] - X: -15   Y: 5   Z: -40
2019-10-17 13:25:16,089 # Temperature [milli deg] : 28955
2019-10-17 13:25:16,090 # 
2019-10-17 13:25:16,093 # +-------------------------------------+
2019-10-17 13:25:17,094 # Accel data [milli g] - X: 92   Y: -191   Z: -974
2019-10-17 13:25:17,098 # Gyro data [dps] - X: 10   Y: -44   Z: -9
2019-10-17 13:25:17,104 # Compass data [micro T] - X: 12   Y: 6   Z: -41
2019-10-17 13:25:17,107 # Temperature [milli deg] : 29021
2019-10-17 13:25:17,107 # 
2019-10-17 13:25:17,110 # +-------------------------------------+
2019-10-17 13:25:18,112 # Accel data [milli g] - X: -417   Y: 245   Z: -844
2019-10-17 13:25:18,116 # Gyro data [dps] - X: -51   Y: 1   Z: -4
2019-10-17 13:25:18,121 # Compass data [micro T] - X: -1   Y: 26   Z: -33
2019-10-17 13:25:18,124 # Temperature [milli deg] : 29027
2019-10-17 13:25:18,125 # 
2019-10-17 13:25:18,128 # +-------------------------------------+
2019-10-17 13:25:19,129 # Accel data [milli g] - X: 271   Y: 42   Z: -987
2019-10-17 13:25:19,133 # Gyro data [dps] - X: 40   Y: -3   Z: 11
2019-10-17 13:25:19,138 # Compass data [micro T] - X: 6   Y: 0   Z: -40
2019-10-17 13:25:19,141 # Temperature [milli deg] : 29066
2019-10-17 13:25:19,142 # 
2019-10-17 13:25:19,145 # +-------------------------------------+
2019-10-17 13:25:20,146 # Accel data [milli g] - X: -428   Y: 398   Z: -791
2019-10-17 13:25:20,150 # Gyro data [dps] - X: -44   Y: 38   Z: 18
2019-10-17 13:25:20,193 # Compass data [micro T] - X: -3   Y: 24   Z: -33
2019-10-17 13:25:20,194 # Temperature [milli deg] : 29074
2019-10-17 13:25:20,195 # 
2019-10-17 13:25:20,196 # +-------------------------------------+
2019-10-17 13:25:21,164 # Accel data [milli g] - X: -3   Y: 27   Z: -966
2019-10-17 13:25:21,168 # Gyro data [dps] - X: -1   Y: 0   Z: -60
2019-10-17 13:25:21,173 # Compass data [micro T] - X: 15   Y: 9   Z: -43
2019-10-17 13:25:21,176 # Temperature [milli deg] : 29095

MrKevinWeiss
MrKevinWeiss approved these changes Oct 17, 2019
View reviewed changes
Copy link
Contributor

@MrKevinWeiss MrKevinWeiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a big improvement. I witnessed the test, ACK!

@benpicco
Copy link
Contributor

Murdock is not happy yet.

@JannesVolkens JannesVolkens force-pushed the mpu9x50 branch 3 times, most recently from df80c0a to 20e6263 Compare October 17, 2019 12:03
@JannesVolkens
drivers/mpu9x50: Correct read_temperature
fdd3449 
Change msbiot Makefile.dep back to mpu9150
@MrKevinWeiss MrKevinWeiss merged commit 0e85425 into RIOT-OS:master Oct 17, 2019
@JannesVolkens JannesVolkens deleted the mpu9x50 branch October 18, 2019 08:02
@fjmolinas fjmolinas added this to the Release 2020.01 milestone Dec 13, 2019
@leandrolanzieri leandrolanzieri mentioned this pull request Dec 8, 2020
Merged
@maribu maribu mentioned this pull request Apr 7, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gschorcht gschorcht gschorcht left review comments

@MrKevinWeiss MrKevinWeiss MrKevinWeiss approved these changes

Assignees

@PeterKietzmann PeterKietzmann

Labels
Area: drivers Area: Device drivers CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: new feature The issue requests / The PR implemements a new feature for RIOT
Projects
None yet
Milestone
Release 2020.01
Development

Successfully merging this pull request may close these issues.
6 participants
@JannesVolkens @benpicco @gschorcht @MrKevinWeiss @PeterKietzmann @fjmolinas