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

LIS2DH SPI Support #22348

Closed
theBASTI0N opened this issue Jan 30, 2020 · 16 comments · Fixed by #23049
Closed

LIS2DH SPI Support #22348

theBASTI0N opened this issue Jan 30, 2020 · 16 comments · Fixed by #23049
Assignees
Labels
area: Sensors Sensors bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug

Comments

@theBASTI0N
Copy link
Contributor

The LIS2DH works well with I2C devices such as the Thingy52, however when testing with a Ruuvitag which has the the sensor on the SPI bus readings are wrong.

The driver looks like it doesn't not configure the SPI bus correctly as it missues the CS controller and CS pin configurations. An example of the correct configuration type can be found for the lis2ds12 driver.

Steps to reproduce the behavior:

  1. git clone https://github.com/theBASTI0N/zephyr-ruuvi.git /boards/arm/nrf52_ruuvi
  2. cd ~/xephyrproject/zephyr/
  3. west build -b nrf52_ruuvi samples/sensor/lis2dh
  4. See readings, I used:
    CONFIG_USE_SEGGER_RTT=y
    CONFIG_RTT_CONSOLE=y
    CONFIG_UART_CONSOLE=n

There is no error in stating the device cannot be found, however the reading are wrong.

The output is:

SEGGER J-Link V6.60f - Real time terminal output
J-Link OB-SAM3U128-V2-NordicSemi compiled Jan 7 2019 14:07:15 V1.0, SN=682061953
Process: JLinkExe
*** Booting Zephyr OS build zephyr-v2.1.0-1478-g702ebb321225 ***
Polling at 0.5 Hz
#1 @ 12600 ms: x -0.009576 , y -0.009576 , z -0.009576
#2 @ 13901 ms: x -0.009576 , y -0.009576 , z -0.009576
#3 @ 15202 ms: x -0.009576 , y -0.009576 , z -0.009576
#4 @ 16503 ms: x -0.009576 , y -0.009576 , z -0.009576
#5 @ 17804 ms: x -0.009576 , y -0.009576 , z -0.009576
#6 @ 19106 ms: x -0.009576 , y -0.009576 , z -0.009576
#7 @ 20407 ms: x -0.009576 , y -0.009576 , z -0.009576
#8 @ 21708 ms: x -0.009576 , y -0.009576 , z -0.009576
#9 @ 23009 ms: x -0.009576 , y -0.009576 , z -0.009576
#10 @ 24310 ms: x -0.009576 , y -0.009576 , z -0.009576

The makes using the lis2dh12 on the ruuvitag almost impossible.

Environment

  • OS:Debian 10
  • Toolchain Zephyr SDK
  • Version 2.1.99
@theBASTI0N theBASTI0N added the bug The issue is a bug, or the PR is fixing a bug label Jan 30, 2020
@jfischer-no jfischer-no added the area: Sensors Sensors label Jan 30, 2020
@avisconti
Copy link
Collaborator

OK, I'll start next week.
I'll put LIS2DH driver similar to LIS2DS12, with a lis2dh_i2c.c and lis2dh_spi.c file, compiled according to dts configuration.

@theBASTI0N
Copy link
Contributor Author

Awesoe thanks for the help.
@https://github.com/ojousima

Did some work on an older version and git it work, which i added to the newest driver version.
This can be found at:
https://github.com/theBASTI0N/zephyr/tree/master/drivers/sensor/lis2dh

The sensor is now working.

@avisconti
Copy link
Collaborator

Did some work on an older version and git it work, which i added to the newest driver version.
This can be found at:
https://github.com/theBASTI0N/zephyr/tree/master/drivers/sensor/lis2dh

The sensor is now working.

Fine!
You may want to create a PR out of this. We can quickly merge it and close the bug then. In fact, I currently have some other urgencies, and working on this stuff may take longer than expected.

@jhedberg jhedberg added the priority: low Low impact/importance bug label Feb 4, 2020
@theBASTI0N
Copy link
Contributor Author

It works sometimes and the interrupts dont seem to work.

@avisconti
Copy link
Collaborator

@theBASTI0N
I'm providing a fix for this bug. Can you possibly cross verify it?

@theBASTI0N
Copy link
Contributor Author

@avisconti
I will be able to verify it later today.

Thanks for the help

@avisconti
Copy link
Collaborator

@avisconti
I will be able to verify it later today.

Thanks for the help

Have you had the chance to give it a try?

@theBASTI0N
Copy link
Contributor Author

@avisconti
I will be able to verify it later today.
Thanks for the help

Have you had the chance to give it a try?
Replaced the files with the ones you worked on, but am getting issue with building. But getting some issues when building

lis2dh.h:12:10: fatal error: misc/util.h: No such file or directory
is one example

@avisconti
Copy link
Collaborator

@avisconti
I will be able to verify it later today.
Thanks for the help

Have you had the chance to give it a try?
Replaced the files with the ones you worked on, but am getting issue with building. But getting some issues when building

lis2dh.h:12:10: fatal error: misc/util.h: No such file or directory
is one example

How is it possible?
Maybe you are using an older zephyr version?

@theBASTI0N
Copy link
Contributor Author

theBASTI0N commented Mar 2, 2020

@avisconti
I will be able to verify it later today.
Thanks for the help

Have you had the chance to give it a try?
Replaced the files with the ones you worked on, but am getting issue with building. But getting some issues when building

lis2dh.h:12:10: fatal error: misc/util.h: No such file or directory
is one example

How is it possible?
Maybe you are using an older zephyr version?

Hey,

Got it to build and is working great. Havn't been able to test the interrupt trigger.

I added CONFIG_LIS2DH_TRIGGER_OWN_THREAD=y to my prj.conf then I get errors.

The errors I am getting:

In function 'setup_int1':
24:2: warning: implicit declaration of function 'gpio_pin_interrupt_configure'; did you mean 'gpio_pin_configure'? [-Wimplicit-function-declaration]
24 | gpio_pin_interrupt_configure(lis2dh->gpio_int1,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
| gpio_pin_configure
27:12: error: 'GPIO_INT_EDGE_TO_ACTIVE' undeclared (first use in this function)
27 | ? GPIO_INT_EDGE_TO_ACTIVE
| ^~~~~~~~~~~~~~~~~~~~~~~
27:12: note: each undeclared identifier is reported only once for each function it appears in
28:12: error: 'GPIO_INT_DISABLE' undeclared (first use in this function); did you mean 'GPIO_INT_LEVEL'?
28 | : GPIO_INT_DISABLE);
| ^~~~~~~~~~~~~~~~
| GPIO_INT_LEVEL
In function 'lis2dh_init_interrupt':
401:9: error: 'GPIO_INPUT' undeclared (first use in this function); did you mean 'GPIO_INT'?
401 | GPIO_INPUT | DT_LIS2DH_INT1_GPIOS_FLAGS);
| ^~~~~~~~~~
| GPIO_INT

It may be due to my board dts but I haven't been able to find an example for using CS pins with SPI. The board files are at : https://github.com/theBASTI0N/zephyr-ruuvi

@avisconti
Copy link
Collaborator

avisconti commented Mar 4, 2020

Hey,

Got it to build and is working great. Havn't been able to test the interrupt trigger.

I added CONFIG_LIS2DH_TRIGGER_OWN_THREAD=y to my prj.conf then I get errors.

The errors I am getting:

In function 'setup_int1':
24:2: warning: implicit declaration of function 'gpio_pin_interrupt_configure'; did you mean 'gpio_pin_configure'? [-Wimplicit-function-declaration]
24 | gpio_pin_interrupt_configure(lis2dh->gpio_int1,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
| gpio_pin_configure
27:12: error: 'GPIO_INT_EDGE_TO_ACTIVE' undeclared (first use in this function)
27 | ? GPIO_INT_EDGE_TO_ACTIVE
| ^~~~~~~~~~~~~~~~~~~~~~~
27:12: note: each undeclared identifier is reported only once for each function it appears in
28:12: error: 'GPIO_INT_DISABLE' undeclared (first use in this function); did you mean 'GPIO_INT_LEVEL'?
28 | : GPIO_INT_DISABLE);
| ^~~~~~~~~~~~~~~~
| GPIO_INT_LEVEL
In function 'lis2dh_init_interrupt':
401:9: error: 'GPIO_INPUT' undeclared (first use in this function); did you mean 'GPIO_INT'?
401 | GPIO_INPUT | DT_LIS2DH_INT1_GPIOS_FLAGS);
| ^~~~~~~~~~
| GPIO_INT

I think you are using an old version: the GPIO APIs have been modified recently. It seems your zephyr version is still using the old one. Please go in sync

@avisconti
Copy link
Collaborator

It may be due to my board dts but I haven't been able to find an example for using CS pins with SPI.

I used the x_nucleo_iks01a3 shield on top of nucleo_f401re board, plugged together thru arduino.
Here the code:

&arduino_spi {
status = "okay";
cs-gpios = <&arduino_header 16 0>; /* D10 */

lis2dh@0 {
	compatible = "st,lis2dh";
	label = "LIS2DH";
	spi-max-frequency = <1000000>;
	irq-gpios = <&arduino_header 5 GPIO_ACTIVE_HIGH>; /* A5 */
	reg = <0>;
};

};

@theBASTI0N
Copy link
Contributor Author

It may be due to my board dts but I haven't been able to find an example for using CS pins with SPI.

I used the x_nucleo_iks01a3 shield on top of nucleo_f401re board, plugged together thru arduino.
Here the code:

&arduino_spi {
status = "okay";
cs-gpios = <&arduino_header 16 0>; /* D10 */

lis2dh@0 {
	compatible = "st,lis2dh";
	label = "LIS2DH";
	spi-max-frequency = <1000000>;
	irq-gpios = <&arduino_header 5 GPIO_ACTIVE_HIGH>; /* A5 */
	reg = <0>;
};

};

If i use

&spi0 {
	#compatible = "nordic,nrf-spi";
	/* Cannot be used together with i2c0. */
	status = "okay";
	sck-pin = <29>;
	mosi-pin = <25>;
	miso-pin = <28>;							/* Not sure if needed */
    cs-gpios = <&gpio0 8 0>;		/* Not sure if needed */

	lis2dh@1 {
		compatible = "st,lis2dh";
		reg = <1>;
		spi-max-frequency = <10000000>;
		irq-gpios = <&gpio0 2 GPIO_INT_ACTIVE_HIGH>;
		label = "LIS2DH";
	};
};

I get this error:

../src/main.c:57:10: error: 'DT_INST_0_ST_LIS2DH_LABEL' undeclared (first use in this function); did you mean 'DT_INST_0_SOC_NV_FLASH_LABEL'?
   57 |          DT_INST_0_ST_LIS2DH_LABEL);
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~
      |          DT_INST_0_SOC_NV_FLASH_LABEL

@avisconti
Copy link
Collaborator

avisconti commented Mar 4, 2020

Sorry, I'm completeley confused. Let me try to recap everything.

  1. we had an issue in LIS2DH for which was not possible to communicate thru SPI. Now, after the
    driver/sensors: lis2dh: Fix I2C and SPI bus communication #23049 you are able to use the LIS2DH thru SPI. Is this correct?

  2. Yet you are not able to test triggers, because when you enable it it does not build properly, right?
    Now, for this point, I believe the issue is that you are not aligned to latest zephyr version.
    Can you do it?

Thanks

@theBASTI0N
Copy link
Contributor Author

2. is point, I be

Hey,

I am able to use the LIS2DH through SPI and am using the LIS2DH sample by changing it to use SPI instead of I2C.

When I enable the trigger it fails to build.

I have Zephyr version 2.1.99.

@avisconti
Copy link
Collaborator

When I enable the trigger it fails to build.

I have Zephyr version 2.1.99.

That's why it is not building.
I think you should align to v2.2.0-rc3 now or to v2.2.0 as soon as it will be available.

avisconti added a commit to avisconti/zephyr that referenced this issue Mar 6, 2020
Add I2C and SPI bus communication routines in separate files,
and register one or the other as read/write callbacks based
on bus selection in DTS.

This commit is fixing issue zephyrproject-rtos#22348 as well, as the SPI part
is handling in the proper way the CS GPIO part.

Signed-off-by: Armando Visconti <[email protected]>
jhedberg pushed a commit that referenced this issue Mar 10, 2020
Add I2C and SPI bus communication routines in separate files,
and register one or the other as read/write callbacks based
on bus selection in DTS.

This commit is fixing issue #22348 as well, as the SPI part
is handling in the proper way the CS GPIO part.

Signed-off-by: Armando Visconti <[email protected]>
hakehuang pushed a commit to hakehuang/zephyr that referenced this issue Mar 18, 2020
Add I2C and SPI bus communication routines in separate files,
and register one or the other as read/write callbacks based
on bus selection in DTS.

This commit is fixing issue zephyrproject-rtos#22348 as well, as the SPI part
is handling in the proper way the CS GPIO part.

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
Labels
area: Sensors Sensors bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants