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

Invensense IMU drivers accumulated minor improvements and consistency cleanup #15299

Merged
merged 10 commits into from
Aug 16, 2020
Merged

Invensense IMU drivers accumulated minor improvements and consistency cleanup #15299

merged 10 commits into from
Aug 16, 2020

Conversation

dagar
Copy link
Member

@dagar dagar commented Jul 9, 2020 •
edited
Loading

Nothing too exciting here, but here are the general changes across most of the new(-ish) Invensense IMU drivers.

  • setting the desired power management (PWR_MGMT_1) mode immediately after reset and waiting the appropriate amount of time as per the datasheet (up to 100 ms in certain cases)
    • this ensures clean data initially and better recovery from bad states
  • replace the data ready interrupt interval perf counter with a simple count if data ready was enabled but missed
    • this removes the remaining perf count work (with floating point calculations) from interrupts and still gives us something to review quickly for potential board or sensor problems (drdy interrupt enabled, but not incrementing)
    • saves a small amount of cpu and an even smaller amount of memory
  • disabling the I2C interface immediately (as per the datasheet)
  • only tracking consecutive errors to trigger a full reset, and lowering this threshold quite a bit
    • we want to catch real problems as soon as possible, but careful to not trigger the lengthy full reset unnecessarily
  • style changes to keep these as consistent as possible for the possibility of consolidation at a later date

Testing

  • icm20602
  • icm20608g
  • icm20689
  • icm20649
  • icm20948
  • icm40609d (nearly identical to icm42688p still)
  • icm42688p
  • mpu6000
  • mpu6500
  • mpu9250

@dagar dagar force-pushed the pr-invensense_imu_misc branch 3 times, most recently from 0d14b08 to d2d56d6 Compare July 9, 2020 21:16
@weekly-digest weekly-digest bot mentioned this pull request Jul 12, 2020
Closed
@dagar dagar force-pushed the pr-invensense_imu_misc branch 2 times, most recently from 6ed2036 to b9cf44d Compare July 15, 2020 13:57
@weekly-digest weekly-digest bot mentioned this pull request Jul 19, 2020
Closed
@dagar dagar force-pushed the pr-invensense_imu_misc branch 4 times, most recently from ca4db38 to f18ca94 Compare August 6, 2020 17:43
BazookaJoe1900
BazookaJoe1900 requested changes Aug 7, 2020
View reviewed changes
Copy link
Member

@BazookaJoe1900 BazookaJoe1900 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.

(some) ICM devices, the 42688 included. can change the data endian by setting INTF_CONFIG0:SENSOR_DATA_ENDIAN and INTF_CONFIG0:FIFO_COUNT_ENDIAN bits. so data reading can be highly simplified by setting that bit during initialize.
this should reduce 6 "data flips" for every data sample from the IMU, which can be a lot...
that feature tested on ICM-42605. but there are probably other ICM that has that feature too, like the 42609

@BazookaJoe1900 BazookaJoe1900 mentioned this pull request Aug 7, 2020
Closed
@dagar dagar force-pushed the pr-invensense_imu_misc branch from f18ca94 to a8db169 Compare August 14, 2020 17:30
dagar added 10 commits August 16, 2020 17:58
@dagar
icm20602: accumulated minor improvements and cleanup
1042608 
 - perform reset as per the datasheet (disable I2C immediately, set power mode, wait for appropriate time, etc)
 - track consecutive errors and trigger full reset if necessary
 - remove interrupt perf counter and instead only count misses
 - minor style changes to stay in sync with the other Invensense drivers
 - read FIFO count along with full transfer as a sanity check
@dagar
icm20608g: accumulated minor improvements and cleanup
42562cf 
 - update copyright year
 - perform reset as per the datasheet (disable I2C immediately, set power mode, wait for appropriate time, etc)
 - only track consecutive errors (not total) to trigger full reset if necessary
 - remove interrupt perf counter and instead only count misses
 - minor style changes to stay in sync with the other Invensense drivers
@dagar
icm20689: accumulated minor improvements and cleanup
49632ed 
 - perform reset as per the datasheet (disable I2C immediately, set power mode, wait for appropriate time, etc)
 - only track consecutive errors (not total) to trigger full reset if necessary
 - remove interrupt perf counter and instead only count misses
 - minor style changes to stay in sync with the other Invensense drivers
@dagar
icm20649: accumulated minor improvements and cleanup
5e50f3b 
 - perform reset as per the datasheet (disable I2C immediately, set power mode, wait for appropriate time, etc)
 - remove interrupt perf counter and instead only count misses
 - minor style changes to stay in sync with the other Invensense drivers
@dagar
icm20948: accumulated minor improvements and cleanup
3c1634e 
 - remove interrupt perf counter and instead only count misses
 - minor style changes to stay in sync with the other Invensense drivers
@dagar
icm40609d: accumulated minor improvements and cleanup
3049517 
 - perform reset as per the datasheet (wakeup accel/gyro and wait before proceeding)
 - add register bank selection (not yet used)
 - track consecutive errors to trigger full reset if necessary
 - remove interrupt perf counter and instead only count misses
 - minor style changes to stay in sync with the other Invensense drivers
@dagar
icm42688p: accumulated minor improvements and cleanup
6477cb6 
 - perform reset as per the datasheet (wakeup accel/gyro and wait before proceeding)
 - add register bank selection (not yet used)
 - track consecutive errors to trigger full reset if necessary
 - remove interrupt perf counter and instead only count misses
 - minor style changes to stay in sync with the other Invensense drivers
@dagar
mpu6000: accumulated minor improvements and cleanup
2861308 
 - perform reset as per the datasheet (disable I2C immediately, set power mode, wait for appropriate time, etc)
 - only track consecutive errors (not total) to trigger full reset if necessary
 - remove interrupt perf counter and instead only count misses
 - minor style changes to stay in sync with the other Invensense drivers
@dagar
mpu6500: accumulated minor improvements and cleanup
1442bde 
 - perform reset as per the datasheet (disable I2C immediately, set power mode, wait for appropriate time, etc)
 - only track consecutive errors (not total) to trigger full reset if necessary
 - remove interrupt perf counter and instead only count misses
 - minor style changes to stay in sync with the other Invensense drivers
@dagar
mpu9250: accumulated minor improvements and cleanup
3dd4d52 
 - perform reset as per the datasheet (disable I2C immediately, set power mode, wait for appropriate time, etc)
 - remove interrupt perf counter and instead only count misses
 - minor style changes to stay in sync with the other Invensense drivers
@dagar dagar force-pushed the pr-invensense_imu_misc branch from a8db169 to 3dd4d52 Compare August 16, 2020 22:07
@dagar dagar merged commit 33e3456 into master Aug 16, 2020
@dagar dagar deleted the pr-invensense_imu_misc branch August 16, 2020 23:01
@SalimTerryLi
Copy link
Contributor

(some) ICM devices, the 42688 included. can change the data endian by setting INTF_CONFIG0:SENSOR_DATA_ENDIAN and INTF_CONFIG0:FIFO_COUNT_ENDIAN bits. so data reading can be highly simplified by setting that bit during initialize.
this should reduce 6 "data flips" for every data sample from the IMU, which can be a lot...
that feature tested on ICM-42605. but there are probably other ICM that has that feature too, like the 42609

May I ask that is there any existing driver which can be applied to icm42605?

@lukegluke
Copy link
Contributor

May I ask that is there any existing driver which can be applied to icm42605?

Yes, I successfully use icm42688p driver for icm42605 by just changing WHOAMI = 0x42

@SalimTerryLi
Copy link
Contributor

May I ask that is there any existing driver which can be applied to icm42605?

Yes, I successfully use icm42688p driver for icm42605 by just changing WHOAMI = 0x42

Thank you!

@dagar
Copy link
Member Author

dagar commented Aug 17, 2020

I'll add a dedicated icm42605 driver. I'm currently updating the icm42688p to make use of its additional features (18/19 bit output, clock input, etc). At this point I'd still rather err on the side of code duplication to ensure we aren't compromising on these drivers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BazookaJoe1900 BazookaJoe1900 BazookaJoe1900 requested changes

Assignees
No one assigned
Labels
Admin: Cleanup (housekeeping) 🧹 Admin: Enhancement (improvement) 💡 Drivers 🔧 Sensors, Actuators, etc
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@dagar @SalimTerryLi @lukegluke @BazookaJoe1900