Skip to content

drivers/lis2dh12: add I2C mode#12290

Merged
benpicco merged 4 commits intoRIOT-OS:masterfrom
benpicco:opt_driver_lis2dh12i2c
Oct 2, 2019
Merged

drivers/lis2dh12: add I2C mode#12290
benpicco merged 4 commits intoRIOT-OS:masterfrom
benpicco:opt_driver_lis2dh12i2c

Conversation

@benpicco
Copy link
Copy Markdown
Contributor

@benpicco benpicco commented Sep 23, 2019

Contribution description

The PR adapts the lis2dh12 driver to work also in I2C mode, which is the used configuration on the thingy52.

The PR is a rebase of #8419 by @haukepetersen and adapts it to changes in the I2C API during the last two years.

Testing procedure

On a board with lis2dh12 over I2C, run tests/driver_lis2dh12 and change DRIVER ?= lis2dh12_spi to DRIVER ?= lis2dh12_i2c in the Makefile.

@MrKevinWeiss tested the original PR with the thingy52. I don't have the thingy52 but a custom samd51 board with the same chip - here the driver is working as expected.

Issues/PRs references

#8419 rebased & updated to the API changes in master (see fixup)

@benpicco benpicco added Type: new feature The issue requests / The PR implemements a new feature for RIOT Area: drivers Area: Device drivers labels Sep 23, 2019
@benpicco benpicco changed the title Opt driver lis2dh12i2c drivers/lis2dh12: add I2C mode Sep 23, 2019
@benpicco benpicco mentioned this pull request Sep 23, 2019
Closed
@MrKevinWeiss
Copy link
Copy Markdown
Contributor

MrKevinWeiss commented Sep 24, 2019

I have a thingy52 but I cannot find my jlink to flash it. Ping me after Oct. 1st and hopefully I can test it (vacation).

@haukepetersen
Copy link
Copy Markdown
Contributor

haukepetersen commented Sep 30, 2019

@benpicco thanks for taking this over! I just could not find the time to finally get this merged... I will close #8419 in favor of this PR.

@benpicco benpicco requested review from MrKevinWeiss and removed request for MrKevinWeiss October 1, 2019 12:56
@MrKevinWeiss MrKevinWeiss added Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines labels Oct 2, 2019
@MrKevinWeiss
Copy link
Copy Markdown
Contributor

MrKevinWeiss commented Oct 2, 2019

Tested with BOARD=thingy52 DRIVER=lis2dh12_i2c make flash term -C tests/driver_lis2dh12/
and got reasonable numbers:

2019-10-02 11:06:45,318 - INFO # X:   -0.050 Y:   -0.003 Z:   -1.007
2019-10-02 11:06:45,425 - INFO # X:   -0.050 Y:   -0.003 Z:   -1.023
2019-10-02 11:06:45,522 - INFO # X:   -0.054 Y:    0.000 Z:   -1.015

and it makes sense.

MrKevinWeiss
MrKevinWeiss approved these changes Oct 2, 2019
View reviewed changes
Copy link
Copy Markdown
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.

Looks good to me.

ACK!

@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 2, 2019
@MrKevinWeiss
Copy link
Copy Markdown
Contributor

MrKevinWeiss commented Oct 2, 2019

Squash please

@MrKevinWeiss MrKevinWeiss added Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines labels Oct 2, 2019
@benpicco benpicco force-pushed the opt_driver_lis2dh12i2c branch from 2dca61b to 060eac7 Compare October 2, 2019 09:17
@benpicco benpicco force-pushed the opt_driver_lis2dh12i2c branch from 060eac7 to 96db5c3 Compare October 2, 2019 09:40
@benpicco
Copy link
Copy Markdown
Contributor Author

benpicco commented Oct 2, 2019

Thank you for the test and review!
Had to push a small fixup: for C++, the struct members have to have the same order for initialization and declaration.

@benpicco benpicco merged commit 69d274d into RIOT-OS:master Oct 2, 2019
@benpicco benpicco deleted the opt_driver_lis2dh12i2c branch October 2, 2019 10:06
@aabadie aabadie mentioned this pull request Oct 2, 2019
Merged
@aabadie aabadie mentioned this pull request Oct 4, 2019
Merged
@kb2ma kb2ma added this to the Release 2019.10 milestone Oct 28, 2019
@jan-mo jan-mo mentioned this pull request Jan 7, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MrKevinWeiss MrKevinWeiss MrKevinWeiss approved these changes

@haukepetersen haukepetersen Awaiting requested review from haukepetersen

@kaspar030 kaspar030 Awaiting requested review from kaspar030

Assignees

No one assigned

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 Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines Type: new feature The issue requests / The PR implemements a new feature for RIOT

Projects

None yet

Milestone

Release 2019.10

Development

Successfully merging this pull request may close these issues.

4 participants

@benpicco @MrKevinWeiss @haukepetersen @kb2ma