Skip to content

Add shield iks01a3 #15425

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
Merged
merged 3 commits into from
Jul 8, 2019
Merged

Add shield iks01a3 #15425

merged 3 commits into from
Jul 8, 2019

Conversation

@avisconti
Copy link
Contributor

@avisconti avisconti commented Apr 12, 2019

x_nucleo_iks01a3 is a companion board that can be plugged through Arduino connectors on top
of Nucleo boards (such as nucleo_f411re).

This PR is adding shield support plus a couple of samples code, one for standard and one for sensorhub mode, plus proper documentation.
The PR is rebased on top of LPS22HH (#13062) and LSM6DSO (#13478) drivers.

@avisconti avisconti added area: Boards area: Sensors Sensors platform: STM32 ST Micro STM32 area: Shields Shields (add-on boards) labels Apr 12, 2019
@avisconti avisconti self-assigned this Apr 12, 2019
@zephyrbot
Copy link

zephyrbot commented Apr 12, 2019 •
edited
Loading

All checks are passing now.

Review history of this comment for details about previous failed status.
Note that some checks might have not completed yet.

@avisconti avisconti force-pushed the add-shield-iks01a3 branch from 45ce4bb to 79149c1 Compare April 12, 2019 13:39
@erwango
Copy link
Member

erwango commented Apr 12, 2019

@avisconti : I think we can remove tag "platform:STM32". This shield aims to be working on any board supporting arduino header, right?

@avisconti avisconti removed the platform: STM32 ST Micro STM32 label Apr 12, 2019
@avisconti
Copy link
Contributor Author

avisconti commented Apr 12, 2019

@avisconti : I think we can remove tag "platform:STM32". This shield aims to be working on any board supporting arduino header, right?

removed

@ulfalizer
Copy link
Contributor

ulfalizer commented Apr 12, 2019

Sorry about that false positive in a comment.

I really want to move ci-tools back into the main Zephyr repo. That way the whitelist could be modified in the same commit, without having to add yet another separate ad-hoc configuration file (which would have to support comments and stuff too).

There will be a pylint whitelist later too.

dbkinder
dbkinder suggested changes Apr 12, 2019
View reviewed changes
boards/shields/x_nucleo_iks01a3/doc/index.rst Outdated
Copy link
Contributor

@dbkinder dbkinder Apr 12, 2019

Choose a reason for hiding this comment

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

I know it looks better, but if you write this as I2C throughout this document, it will show up in search results while I²C won't, and we use I2C in all our other documentation.

Copy link
Contributor Author

@avisconti avisconti Apr 15, 2019

Choose a reason for hiding this comment

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

ACK

boards/shields/x_nucleo_iks01a3/doc/index.rst Outdated
Copy link
Contributor

@dbkinder dbkinder Apr 12, 2019

Choose a reason for hiding this comment

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

Suggested change
In standard I²C mode the two bus are connected together. As a consequence, all devices on the shield
In standard I²C mode the two buses are connected together. As a consequence, all devices on the shield

Copy link
Contributor Author

@avisconti avisconti Apr 15, 2019

Choose a reason for hiding this comment

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

thx

boards/shields/x_nucleo_iks01a3/doc/index.rst Outdated
Copy link
Contributor

@dbkinder dbkinder Apr 12, 2019

Choose a reason for hiding this comment

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

Suggested change
resides on the same I²C bus and are accessible from the main board thru I²C bus.
reside on the same I²C bus and are accessible from the main board thru the I²C bus.

Copy link
Contributor Author

@avisconti avisconti Apr 15, 2019

Choose a reason for hiding this comment

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

thx

boards/shields/x_nucleo_iks01a3/doc/index.rst Outdated
Copy link
Contributor

@dbkinder dbkinder Apr 12, 2019

Choose a reason for hiding this comment

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

Again, change all uses of I²C to I2C

boards/shields/x_nucleo_iks01a3/doc/index.rst Outdated
Copy link
Contributor

@dbkinder dbkinder Apr 12, 2019

Choose a reason for hiding this comment

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

This would read betters as:

More information about X-NUCLEO-IKS01A3 configuration modes can be found in the 
`X-NUCLEO-IKS01A3 user manual`_.

@avisconti avisconti force-pushed the add-shield-iks01a3 branch from 79149c1 to f52e419 Compare April 15, 2019 08:21
@avisconti
Copy link
Contributor Author

avisconti commented Apr 15, 2019

@dbkinder, thx for reviewing the board doc. I have fixed all your requested changes.
Have you taken a look to the two samples docs as well? Are they ok?

@avisconti avisconti requested a review from dbkinder April 18, 2019 14:57
dbkinder
dbkinder approved these changes Apr 21, 2019
View reviewed changes
Copy link
Contributor

@dbkinder dbkinder left a comment

Choose a reason for hiding this comment

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

LGTM, thanks

ulfalizer pushed a commit to ulfalizer/zephyr that referenced this pull request May 13, 2019
@avisconti @ulfalizer
check_compliance.py: Add configurations to whitelist
85ec480 
Added:
    CONFIG_LSM6DSO_INT_PIN
    CONFIG_PEDO_THS_MIN
    CONFIG_USE_STDC_

Referenced in PR zephyrproject-rtos#14111 and PR zephyrproject-rtos#15425.

Signed-off-by: Armando Visconti <[email protected]>
@avisconti avisconti force-pushed the add-shield-iks01a3 branch from f52e419 to 3c63376 Compare June 27, 2019 11:42
@avisconti avisconti requested a review from nashif as a code owner June 27, 2019 11:42
@zephyrbot zephyrbot added area: Devicetree area: Tests Issues related to a particular existing or missing test labels Jun 27, 2019
@avisconti
Copy link
Contributor Author

avisconti commented Jun 27, 2019

I re-based the PR on top of the latest changes.
Please note that this PR depends from #17112 and #17113.

@zephyrbot zephyrbot added the area: Samples Samples label Jun 27, 2019
@avisconti avisconti force-pushed the add-shield-iks01a3 branch 3 times, most recently from 01ce28e to f7baadd Compare June 28, 2019 08:49
@avisconti avisconti force-pushed the add-shield-iks01a3 branch from f7baadd to ca3c2ea Compare July 4, 2019 07:04
@avisconti
Copy link
Contributor Author

avisconti commented Jul 4, 2019

@erwango
I rebased it on top of master today.
The docuemntation has already been reviewed by dbkinder, but it would be nice if you could
take a look to the actual code itself. It should not take longer as it is quite similar to the other
sensor shields.

erwango
erwango requested changes Jul 4, 2019
View reviewed changes
Copy link
Member

@erwango erwango left a comment

Choose a reason for hiding this comment

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

Need to review headers inclusion due to recent change.

samples/shields/x_nucleo_iks01a3/sensorhub/src/main.c Outdated
Copy link
Member

@erwango erwango Jul 4, 2019

Choose a reason for hiding this comment

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

Should be sys/utils.h according to on recent: #16540

Copy link
Contributor Author

@avisconti avisconti Jul 4, 2019

Choose a reason for hiding this comment

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

ok, it makes sense.

Copy link
Contributor Author

@avisconti avisconti Jul 4, 2019

Choose a reason for hiding this comment

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

Done. I did it for lps22hh and lsm6dso yesterday. But I forgot to apply the same here.
Thx!

@avisconti avisconti force-pushed the add-shield-iks01a3 branch from ca3c2ea to b5d240f Compare July 4, 2019 12:35
@avisconti
Copy link
Contributor Author

avisconti commented Jul 5, 2019

@nashif @galak
Any further comment from your side?

MaureenHelm
MaureenHelm requested changes Jul 5, 2019
View reviewed changes
boards/shields/x_nucleo_iks01a3/x_nucleo_iks01a3.overlay Outdated
Copy link
Member

@MaureenHelm MaureenHelm Jul 5, 2019

Choose a reason for hiding this comment

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

Can you look at the arduino_header introduced in #14057 and commit ce1e990 , and make these gpios not specific to stm32 socs?

Copy link
Contributor Author

@avisconti avisconti Jul 8, 2019

Choose a reason for hiding this comment

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

You mean the drdy gpios? Yes, excellent idea!
I will define arduino_header for nucleo F401re and F411re for testing purpose, and I will
change this shield overlay file to make use of it.

samples/shields/x_nucleo_iks01a3/sensorhub/README.rst Outdated
Copy link
Member

@MaureenHelm MaureenHelm Jul 5, 2019

Choose a reason for hiding this comment

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

This is a little deceiving. Zephyr can certainly support multiple sensor instances, it's the specific drivers that don't actually implement multiple instances.

Copy link
Contributor Author

@avisconti avisconti Jul 8, 2019

Choose a reason for hiding this comment

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

Actually this sentence was copied@paisted from other samples:
https://docs.zephyrproject.org/latest/samples/shields/x_nucleo_iks01a1/README.html
https://docs.zephyrproject.org/latest/samples/shields/x_nucleo_iks01a2/README.html

But I can change it to "... as sensors multiple instances is not supported".

Copy link
Contributor Author

@avisconti avisconti Jul 8, 2019

Choose a reason for hiding this comment

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

I changed it in this way:

Please note that this sample can't be used with boards already supporting
one of the sensors available on the shield (such as disco_l475_iot1)
as sensors multiple instances are not supported.

@MaureenHelm
Pls check my english and tell me if it is better than before. Thank you.

avisconti added 2 commits July 8, 2019 11:43
@avisconti
boards/arm: Add arduino_header support to nucleo f401re and f411re
11c1d10 
Add arduino_header support to nucleo f401re and f411re boards.

Signed-off-by: Armando Visconti <[email protected]>
@avisconti
boards: add support for shields x-nucleo-iks01a3
adcbd96 
x-nucleo-iks01a3 shield is an arduino compatible companion board
which can be used on top of Nucleo standard boards. It extend
the Nucleo functionalities adding following MEMS sensor support:

    - LSM6DSO accel and gyro
    - LIS2MDL magnetometer
    - LIS2DW12 accel
    - LPS22HH pressure
    - HTS221 humidity

Signed-off-by: Armando Visconti <[email protected]>
@avisconti avisconti force-pushed the add-shield-iks01a3 branch from b5d240f to d764f3e Compare July 8, 2019 09:46
@avisconti avisconti requested a review from rsalveti as a code owner July 8, 2019 09:46
@avisconti
Copy link
Contributor Author

avisconti commented Jul 8, 2019

@MaureenHelm @erwango
I added one commit for arduino_header support into nucleo f401re and f411re (it is the same mapping).
Then I changed the shield overlay file in order to have all drdy/irq gpios using the arduino_header.
Then rebased on top of latest master commits and retested both standard and sensorhub samples in both f411 and f401.

@avisconti
sample/shield: add support to x-nucleo-iks01a3 shield
f65c299 
Provide two basic example samples to test the x-nucleo-iks01a3 shield:

    - Standard (Mode 1)
    - SensorHub (Mode 2)

Signed-off-by: Armando Visconti <[email protected]>
@avisconti avisconti force-pushed the add-shield-iks01a3 branch from d764f3e to f65c299 Compare July 8, 2019 11:39
@avisconti avisconti requested a review from MaureenHelm July 8, 2019 14:25
@MaureenHelm MaureenHelm merged commit 75e1f4e into zephyrproject-rtos:master Jul 8, 2019
@avisconti avisconti deleted the add-shield-iks01a3 branch July 8, 2019 15:23
ulfalizer pushed a commit to ulfalizer/zephyr that referenced this pull request Oct 23, 2019
@avisconti @nashif
check_compliance.py: Add configurations to whitelist
2cb3568 
Added:
    CONFIG_LSM6DSO_INT_PIN
    CONFIG_PEDO_THS_MIN
    CONFIG_USE_STDC_

Referenced in PR zephyrproject-rtos#14111 and PR zephyrproject-rtos#15425.

Signed-off-by: Armando Visconti <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MaureenHelm MaureenHelm MaureenHelm approved these changes

@erwango erwango erwango approved these changes

@galak galak Awaiting requested review from galak

@nashif nashif Awaiting requested review from nashif

@rsalveti rsalveti Awaiting requested review from rsalveti

+1 more reviewer

@dbkinder dbkinder dbkinder approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@avisconti avisconti

Labels

area: Boards area: Devicetree area: Samples Samples area: Sensors Sensors area: Shields Shields (add-on boards) area: Tests Issues related to a particular existing or missing test

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@avisconti @zephyrbot @erwango @ulfalizer @MaureenHelm @dbkinder