Skip to content

Fix ICM20948 ReadExternalRegister argument count#686

Merged
stephenhensley merged 1 commit intoelectro-smith:masterfrom
Brokezawa:fix/icm20948-read-external-register
Feb 19, 2026
Merged

Fix ICM20948 ReadExternalRegister argument count#686
stephenhensley merged 1 commit intoelectro-smith:masterfrom
Brokezawa:fix/icm20948-read-external-register

Conversation

@Brokezawa
Copy link
Contributor

Problem:
ReadExternalRegister() calls AuxillaryRegisterTransaction() with 3 arguments, but the function expects 4 (read, slv_addr, reg_addr, value). This causes compilation errors when using magnetometer functionality.

Solution:
Pass 0 as the 4th argument for read operations. The value parameter is only used for write operations, so passing 0 for reads is safe.

Testing:

  • Verified compilation succeeds with this fix
  • Magnetometer initialization now works correctly
  • No impact on other ICM20948 functionality

Files Changed:

  • src/dev/icm20948.h (line 686): Added missing 4th argument (0)

Problem:
ReadExternalRegister() calls AuxillaryRegisterTransaction() with 3
arguments, but the function expects 4 (read, slv_addr, reg_addr, value).
This causes compilation errors when using magnetometer functionality.

Solution:
Pass 0 as the 4th argument for read operations. The value parameter is
only used for write operations, so passing 0 for reads is safe.

Testing:
- Verified compilation succeeds with this fix
- Magnetometer initialization now works correctly
- No impact on other ICM20948 functionality

Files Changed:
- src/dev/icm20948.h (line 686): Added missing 4th argument (0)
@github-actions
Copy link

Test Results

150 tests  ±0   150 ✅ ±0   0s ⏱️ ±0s
  1 suites ±0     0 💤 ±0 
  1 files   ±0     0 ❌ ±0 

Results for commit 690d076. ± Comparison against base commit f044cdc.

@stephenhensley
Copy link
Collaborator

Sneaky error, there.

I confirmed the error, and that your patch fixes it.
I don't have one of these sensors to confirm that it's functioning, but based on your results, this seems good to go!

Thanks for the contribution!

@stephenhensley stephenhensley merged commit 3953e73 into electro-smith:master Feb 19, 2026
13 checks passed
@dromer
Copy link

dromer commented Feb 20, 2026

@Brokezawa I have these sensors here but have not been able to get them to work. Any clue to how we should connect these? Do they require level conversion?

@Brokezawa
Copy link
Contributor Author

Sorry I don't have one on hand right now, I just happened to stumble across this error while building my Nim wrapper for libDaisy. Maybe look into the Adafruit guide for more info : https://learn.adafruit.com/adafruit-tdk-invensense-icm-20948-9-dof-imu

@Brokezawa Brokezawa deleted the fix/icm20948-read-external-register branch February 24, 2026 11:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants