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.

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

adxl345: I²C support #67

Merged
merged 29 commits into from
Jan 2, 2024
Merged

adxl345: I²C support #67

merged 29 commits into from
Jan 2, 2024

Conversation

benoit-pereira-da-silva
Copy link
Contributor

  1. Please prefix the issue title with the primary package affected. For example,
    if this PR fixes periph.io/x/devices/v3/cap1xxx, prefix the PR title with "cap1xxx:".
    adxl345: support both SPI and I²C.
  • Refactored a little bit to make it more consistant with other devices implementation.
  1. Mention the issue number it fixes or add the details of the changes if it
    doesn't have a specific issue. Examples:

  2. Once integrated, send a PR to https://github.com/periph/cmd to leverage the
    new functionality (if relevant).

- Moved the Sensitivity constants to a dedicated section.
- SpiFrequency is set to 2Mz ( ADXL345 supports up to 5Mz )
- Removed the TurnOnOnStart options, we start measuring on instantiation.
- Minor typo and example.go fix.
…ch to automatically recognize the device or enable to test one by declaring its expected value.
- Removed useless ReadRaw
- Changed the scope of ReadAndCombine to private readAndCombine
@benoit-pereira-da-silva
Copy link
Contributor Author

benoit-pereira-da-silva commented Dec 29, 2023

@maruel using mmr.Dev16 was not convenient. The adxl345 returns positive and negative int16 values. So i decided to keep the infamous :) conversion int16(binary.LittleEndian.Uint16(rx)) this implementation should work for most of the adxlXXX line so i ve enabled the ability to use another device by passing explicitly its Opts.ExpectedDeviceID.

@maruel
Copy link
Member

maruel commented Dec 29, 2023

Thanks for the change! Can you rebase it onto origin/main first?

@codecov-commenter
Copy link

codecov-commenter commented Jan 1, 2024

Codecov Report

Attention: 50 lines in your changes are missing coverage. Please review.

Comparison is base (931687b) 62.5% compared to head (60fc493) 61.7%.

Files Patch % Lines
adxl345/adxl345.go 0.0% 50 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##            main     #67     +/-   ##
=======================================
- Coverage   62.5%   61.7%   -0.8%     
=======================================
  Files         64      65      +1     
  Lines       7337    7442    +105     
=======================================
+ Hits        4583    4589      +6     
- Misses      2600    2701    +101     
+ Partials     154     152      -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

adxl345/example/examples.go Outdated Show resolved Hide resolved
adxl345/example/examples.go Outdated Show resolved Hide resolved
adxl345/example/examples.go Outdated Show resolved Hide resolved
// Use of this source code is governed under the Apache License, Version 2.0
// that can be found in the LICENSE file.

package adxl345
Copy link
Member

Choose a reason for hiding this comment

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

Optional: if you use package adxl345_test, the example in the documentation becomes copy-pastable as-is.

Copy link
Member

Choose a reason for hiding this comment

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

I'll resolve to commit, this can be done in a follow up and is optional.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've been pushing the change.

@maruel maruel merged commit bd7de1d into periph:main Jan 2, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants