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

Add lsm6dsl specific samples + change its dts binding #11848

Merged
merged 5 commits into from
Jan 23, 2019

Conversation

avisconti
Copy link
Collaborator

This PR is composed by 3 commits, all 3 somehow related to lsm6dsl sensor.

@avisconti
Copy link
Collaborator Author

@galak @MaureenHelm
If you can also provide your inputs would be good!

@avisconti avisconti added area: ARM ARM (32-bit) Architecture area: Samples Samples area: Devicetree area: Sensors Sensors labels Dec 4, 2018
@zephyrbot
Copy link
Collaborator

zephyrbot commented Dec 4, 2018

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.

@codecov-io
Copy link

codecov-io commented Dec 4, 2018

Codecov Report

Merging #11848 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #11848   +/-   ##
=======================================
  Coverage   53.94%   53.94%           
=======================================
  Files         242      242           
  Lines       27654    27654           
  Branches     6717     6717           
=======================================
  Hits        14917    14917           
  Misses       9932     9932           
  Partials     2805     2805

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 87cf468...5f70824. Read the comment docs.

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.

Thanks for the documentation, one little tweak.

References
**********

- LSM6DSL http://www.st.com/en/mems-and-sensors/lsm6dsl.html
Copy link
Contributor

Choose a reason for hiding this comment

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

Rendering is better without the leading space:

Suggested change
- LSM6DSL http://www.st.com/en/mems-and-sensors/lsm6dsl.html
- LSM6DSL http://www.st.com/en/mems-and-sensors/lsm6dsl.html

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ack

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.

Seems in conflict with #11854. Can you have a check?

@avisconti
Copy link
Collaborator Author

@erwango

Seems in conflict with #11854. Can you have a check?

Yes, I already noticed it, as @galak added me in the review of #11854.
Actually, I have also to change the commit (just meged) for lis2dh as well have the one for lis3dh (currently under review #11814)

@avisconti
Copy link
Collaborator Author

I have repushed it on top of #11854. Now both i2c and spi yaml file are using same compatible
string. Moreover I took care of the inputs from @dbkinder on README doc file.

@galak
Copy link
Collaborator

galak commented Dec 5, 2018

Going to mark this DNM for now until #11854 goes in.

@galak galak added the DNM This PR should not be merged (Do Not Merge) label Dec 5, 2018
@galak galak self-assigned this Dec 5, 2018
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.

doc changes LGTM, thanks!

@galak galak removed the DNM This PR should not be merged (Do Not Merge) label Dec 7, 2018
@galak
Copy link
Collaborator

galak commented Dec 7, 2018

Can you rebase now that we merged #11854

@avisconti
Copy link
Collaborator Author

Can you rebase now that we merged #11854

done!

@avisconti
Copy link
Collaborator Author

recheck

@avisconti
Copy link
Collaborator Author

avisconti commented Dec 10, 2018

@galak
Am I the only one who is getting this error during CI?


L/home/buildslave/src/github.com/zephyrproject-rtos/zephyr/out-3nd-pass/disco_l475_iot1/samples/net/wifi/test/zephyr -lgcc -Wl,--print-memory-usage -mthumb -nostdlib -static -no-pie --coverage -Wl,-X -Wl,-N -Wl,--gc-sections -Wl,--build-id=none -Wl,--orphan-handling=warn -mabi=aapcs && :
Memory region Used Size Region Size %age Used
FLASH: 81788 B 1 MB 7.80%
SRAM: 32640 B 96 KB 33.20%
IDT_LIST: 184 B 2 KB 8.98zephyr/libzephyr.a(eswifi_bus_spi.c.obj):(.data.eswifi_bus_ops_spi+0x0): multiple definition of eswifi_bus_ops_spi' zephyr/libzephyr.a(eswifi_core.c.obj):(.bss.eswifi_bus_ops_spi+0x0): first defined here zephyr/libzephyr.a(eswifi_bus_spi.c.obj): In function eswifi_spi_init':
/home/buildslave/src/github.com/zephyrproject-rtos/zephyr/drivers/wifi/eswifi/eswifi_bus_spi.c:101: multiple definition of uarthost' zephyr/libzephyr.a(eswifi_core.c.obj):/home/buildslave/src/github.com/zephyrproject-rtos/zephyr/drivers/wifi/eswifi/eswifi_core.c:294: first defined here zephyr/libzephyr.a(eswifi_offload.c.obj):(.bss.eswifi_bus_ops_spi+0x0): multiple definition of eswifi_bus_ops_spi'
zephyr/libzephyr.a(eswifi_core.c.obj):/home/buildslave/src/github.com/zephyrproject-rtos/zephyr/drivers/wifi/eswifi/eswifi_core.c:294: first defined here
zephyr/libzephyr.a(eswifi_offload.c.obj): In function eswifi_off_accept': /home/buildslave/src/github.com/zephyrproject-rtos/zephyr/drivers/wifi/eswifi/eswifi_offload.c:88: multiple definition of uarthost'
zephyr/libzephyr.a(eswifi_core.c.obj):/home/buildslave/src/github.com/zephyrproject-rtos/zephyr/drivers/wifi/eswifi/eswifi_core.c:294: first defined here
collect2: error: ld returned 1 exit status
%
ninja: build stopped: subcommand failed.


@galak
Copy link
Collaborator

galak commented Dec 10, 2018

@galak
Am I the only one who is getting this error during CI?

Nope, you aren't. Looking to see how this happened.

@galak
Copy link
Collaborator

galak commented Dec 10, 2018

Nope, you aren't. Looking to see how this happened.

Fix in PR #11988

@avisconti
Copy link
Collaborator Author

Fix in PR #11988

OK, I'll wait for the merge of that PR

@avisconti
Copy link
Collaborator Author

Merged!

Copy link
Collaborator

@galak galak left a comment

Choose a reason for hiding this comment

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

Just noticed, and while not directly related CONFIG_LSM6DSL_BUS_TYPE we should be able to replace with {DT_ST_LSM6DSL_BUS_I2C and/or DT_ST_LSM6DSL_BUS_SPI} that are now generated.

Copy link
Collaborator

@galak galak left a comment

Choose a reason for hiding this comment

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

The sample never gets built for an platform via sanitycheck:

./scripts/sanitycheck -T samples/sensor/lsm6dsl/

I wondering if we can wait for #11800 and than use a filter on DT_COMPAT_ST_LSM6DSL

@avisconti
Copy link
Collaborator Author

avisconti commented Dec 14, 2018

@galak

Just noticed, and while not directly related CONFIG_LSM6DSL_BUS_TYPE we should be able to
replace with {DT_ST_LSM6DSL_BUS_I2C and/or DT_ST_LSM6DSL_BUS_SPI} that are now
generated.

sure! I didn't notice it.
I'll do it in a separate PR.
thanks!

EDIT:
opened PR #12105

@avisconti
Copy link
Collaborator Author

@galak

The sample never gets built for an platform via sanitycheck:
./scripts/sanitycheck -T samples/sensor/lsm6dsl/
I wondering if we can wait for #11800 and than use a filter on DT_COMPAT_ST_LSM6DSL

PR #11800 has been integrated, but I'm not sure I got what these filters are about...
Is this for CI?

@avisconti
Copy link
Collaborator Author

This PR has been rebased onto #12105
Must wait til that one will be merged

@avisconti
Copy link
Collaborator Author

rebased after #12105 merging.
@galak
I added DT_COMPAT_ST_LSM6DSL filter in sample.yaml, but still it seems sanitycheck is never
called for any platform. I'm not so familiar with this scripts, so I may need an help from you.

name: LSM6DSL accelerometer and gyrometer sensor
tests:
test:
filter: DT_COMPAT_ST_LSM6DSL
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove the filter:

Copy link
Collaborator

@galak galak left a comment

Choose a reason for hiding this comment

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

You need to add 'lsm6dsl' to the board yaml that support it.

Add the irq-gpios property to LSM6DSL sensor yaml description
file for I2C case.

Signed-off-by: Armando Visconti <armando.visconti@st.com>
@avisconti
Copy link
Collaborator Author

@galak

you need to add 'lsm6dsl' to the board yaml that support it.

removed the filter and added lsm6dsl to ArgonKey board.
Then re-based and re-pushed!
Thx for your support.

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.

2 minor comments, otherwise tested ok

@@ -14,4 +14,7 @@ supported:
- hts221
- rtc
- spi
- lsm6dsl
Copy link
Member

Choose a reason for hiding this comment

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

Can you add it in disco_l475_iot1 as well?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure.

}

while (1) {
/* lsm6dsl accel */
Copy link
Member

Choose a reason for hiding this comment

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

Can you add following to ease result view:

		/* Erase previous */
		printf("\0033\014");

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ack

Provide through the dts the gpio on which the lsm6dsl INT1 is
connected. Enable also the lsm6dsl trigger mode.

Signed-off-by: Armando Visconti <armando.visconti@st.com>
Add LPS22HB and LSM6DSL sensors to yaml file.

Signed-off-by: Armando Visconti <armando.visconti@st.com>
Add supported sensor to yaml file.
The ArgonKey board supports following sensors:
	- HTS221 (humidity)
	- LPS22HB (pressure)
	- LSM6DSL (imu)
	- VL53L0X (proximity)

Signed-off-by: Armando Visconti <armando.visconti@st.com>
This commit provides sample application for sensor lsm6dsl.
This sample has been tested on both 96b_argonkey board,
where lsm6dsl is connect to the micro thru SPI bus, and on
disco_l475_iot1 board, where instead it is connected to I2C bus.

Signed-off-by: Armando Visconti <armando.visconti@st.com>
@avisconti
Copy link
Collaborator Author

Modified according to @erwango observation and re-pushed.

@galak galak merged commit b8d2e19 into zephyrproject-rtos:master Jan 23, 2019
@avisconti avisconti deleted the lsm6dsl-dts-i2c branch January 23, 2019 10:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants