zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.43k stars 6.39k forks source link

LIS2DH tap interrupt never fires #71570

Closed MaxMaeder closed 2 months ago

MaxMaeder commented 4 months ago

Describe the bug I've modified the lis2dh sample to detect taps, and I can't get it to detect anything, as in my handler never fires.

Please also mention any information which could help others to understand the problem you're facing:

I've also tried incorporating changes proposed by #71229, with no luck.

To Reproduce

CONFIG_NEWLIB_LIBC=y
CONFIG_NEWLIB_LIBC_FLOAT_PRINTF=y
CONFIG_I2C=y
CONFIG_SENSOR=y
CONFIG_LIS2DH=y
CONFIG_LIS2DH_TRIGGER_GLOBAL_THREAD=y
#include <stdio.h>
#include <zephyr/kernel.h>
#include <zephyr/device.h>
#include <zephyr/drivers/sensor.h>

static void trigger_handler(const struct device *dev,
                            const struct sensor_trigger *trig)
{
    printf("TRIG\n");
}

int main(void)
{
    const struct device *sensor = DEVICE_DT_GET(DT_ALIAS(accel0));

    if (sensor == NULL || !device_is_ready(sensor))
    {
        printf("Could not get accel0 device\n");
        return -ENODEV;
    }

    struct sensor_trigger trig;
    int rc;

    trig.type = SENSOR_TRIG_TAP;
    rc = sensor_trigger_set(sensor, &trig, trigger_handler);
    if (rc != 0)
    {
        printf("Failed to set trigger: %d\n", rc);
        return rc;
    }

    printf("Waiting for triggers\n");
    while (true)
    {
        k_sleep(K_MSEC(2000));
    }

    return 0;
}

Build, run, and see error (no output on taps).

Expected behavior "TRIG" printed every time sensor tapped.

Impact Unable to use the sensor for my intended application.

Environment (please complete the following information): MacOS Nordic SDK v2.6.0, latest

Additional context Original PR which implemented feature: #62498 Reached out to PR author, no longer has access to code where he used tap interrupt. @avisconti I know you approved the original PR, mind having a look?

github-actions[bot] commented 4 months ago

Hi @MaxMaeder! We appreciate you submitting your first issue for our open-source project. 🌟

Even though I'm a bot, I can assure you that the whole community is genuinely grateful for your time and effort. 🤖💙

deveritec-rosc commented 4 months ago

Hej, do you have the anym-no-latch set in your dts?

I ran into the problem that, after getting the edging running correctly, an interrupt was detected but the handler wasn't called. I proposed the fix in #71246.

Hope this helps.


Edit: Never mind, for click detection another register is used. It looks as if bit 4 (Sign) of CLICK_SRC is set. From the datasheet, a "positive detection" is denoted by an unset bit and a "negative detection" by a set bit (whatever this means).

The driver currently looks for "negative detection". Could you check

MaxMaeder commented 4 months ago

Hi, thanks for responding.

I currently have the changes proposed in #71229 applied, and anym-no-latch set, as per your suggestion. I added a log statement to lis2dh_thread_cb, and it prints once when the board boots, but never after if I tap the sensor.

Do you have hardware that maybe you could try to reproduce this on? Thanks so much again for the help!

deveritec-rosc commented 4 months ago

Hi, thanks for responding.

I currently have the changes proposed in #71229 applied, and anym-no-latch set, as per your suggestion. I added a log statement to lis2dh_thread_cb, and it prints once when the board boots, but never after if I tap the sensor.

Do you have hardware that maybe you could try to reproduce this on? Thanks so much again for the help!

I just have seen that, though you activated the trigger, you are not setting any threshold. For a test, you may use a value of 0.5g to see if it works.

MaxMaeder commented 4 months ago

Thanks for the response, I'm now trying something like this, but the interrupt doesn't fire, nor does the print statement in the we added to lis2dh_thread_cb print.

Did I configure the threshold correctly? The docs are very spotty, so this is just what I was able to glean by looking at the source code. I'm trying to set it twice, to see if it makes any difference.

    struct sensor_value trigger_threshold = {
        .val1 = 0,
        .val2 = 500000,
    };

    int rc = sensor_attr_set(accel, SENSOR_CHAN_ACCEL_XYZ,
                             SENSOR_ATTR_SLOPE_TH,
                             &trigger_threshold);
    if (rc != 0) {
        printf("Failed to set tap threshold: %d\n", rc);
        return 0;
    }

    struct sensor_trigger tap_trig = {
        .type = SENSOR_TRIG_TAP,
    };

    int ret = sensor_trigger_set(accel, &tap_trig, accel_tap_handler);
    if (ret != 0) {
        printk("Failed to set accel tap trigger: %d\n", ret);
        return ret;
    }

    rc = sensor_attr_set(accel, SENSOR_CHAN_ACCEL_XYZ,
                         SENSOR_ATTR_SLOPE_TH,
                         &trigger_threshold);
    if (rc != 0) {
        printf("Failed to set tap threshold: %d\n", rc);
        return 0;
    }
deveritec-rosc commented 4 months ago

You could check if it is set correctly by adding a breakpoint to lis2dh_acc_slope_config, though it looks fine from my perspective. I'll try to take a look at this in the afternoon/tomorrow if possible.

MaxMaeder commented 4 months ago

I really appreciate it, when I call sensor_attr_set, this is printed <inf> lis2dh: int2_ths=0x3 range_g=2 ums2=499999, which indicates that the threshold should be set correctly. Still no luck detecting actual taps, though.

teburd commented 4 months ago

Did you probe the gpio line, is the hardware trigger actually firing?

MaxMaeder commented 4 months ago

Did you probe the gpio line, is the hardware trigger actually firing?

I know the hardware interrupt works as the anymotion trigger was working fine for me. Unfortunately, the dev board I have does not have interrupt pins broken out so I can't probe them directly.

MaxMaeder commented 4 months ago

@deveritec-rosc Any luck reproducing this issue?

avisconti commented 4 months ago

@MaxMaeder I'll try to reproduce the issue. Sorry if I'm getting into it only now...

deveritec-rosc commented 4 months ago

@MaxMaeder I'll also take a look into it, but I am currently juggling other stuff.

avisconti commented 4 months ago

@MaxMaeder is this issue related to what discovered and attempted to be fixed in #69847? Can you take a look at it?

MaxMaeder commented 3 months ago

@avisconti I checked out #69847, it is essentially a duplicate of #71229. They both change the same trigger mode. I've already tried applying the change, to no luck. The tap interrupt still never fires. Could you perhaps try to see if you can reproduce the issue on your end? I've tried my best to find the issue in the driver, but it is understandably complex and I haven't been able to unearth anything.

avisconti commented 3 months ago

@avisconti I checked out #69847, it is essentially a duplicate of #71229. They both change the same trigger mode. I've already tried applying the change, to no luck. The tap interrupt still never fires. Could you perhaps try to see if you can reproduce the issue on your end? I've tried my best to find the issue in the driver, but it is understandably complex and I haven't been able to unearth anything.

Yes, I'll try to reproduce it even if LIS2DH is really too old. I'm using lis2dh12

Thanks for the response, I'm now trying something like this, but the interrupt doesn't fire, nor does the print statement in the we added to lis2dh_thread_cb print.

Did I configure the threshold correctly? The docs are very spotty, so this is just what I was able to glean by looking at the source code. I'm trying to set it twice, to see if it makes any difference.

    struct sensor_value trigger_threshold = {
        .val1 = 0,
        .val2 = 500000,
    };

    int rc = sensor_attr_set(accel, SENSOR_CHAN_ACCEL_XYZ,
                             SENSOR_ATTR_SLOPE_TH,
                             &trigger_threshold);
    if (rc != 0) {
        printf("Failed to set tap threshold: %d\n", rc);
        return 0;
    }

    struct sensor_trigger tap_trig = {
        .type = SENSOR_TRIG_TAP,
    };

    int ret = sensor_trigger_set(accel, &tap_trig, accel_tap_handler);
    if (ret != 0) {
        printk("Failed to set accel tap trigger: %d\n", ret);
        return ret;
    }

    rc = sensor_attr_set(accel, SENSOR_CHAN_ACCEL_XYZ,
                         SENSOR_ATTR_SLOPE_TH,
                         &trigger_threshold);
    if (rc != 0) {
        printf("Failed to set tap threshold: %d\n", rc);
        return 0;
    }

Have you set both threshold AND duration?

avisconti commented 3 months ago

I mean, both SENSOR_ATTR_SLOPE_TH and SENSOR_ATTR_SLOPE_DUR. They have to be set both. I'll try to do some tests and find out the proper values.

Meanwhile you can try to see if the issue is related to interrupt not actually routed on the int1 or int2 pin, OR if the tap-tap event is not generated at all. You may poll the CLICK_SRC reg (0x39) continuosly, just to see if the event is generated.

avisconti commented 3 months ago

One more thing. You can take a look to this example for the iis2dh StdC: https://github.com/STMicroelectronics/STMems_Standard_C_drivers/blob/master/iis2dh_STdC/examples/iis2dh_tap_single.c

The iis2dh is for inustrial applicatiuon, but it is register compatible with lis2dh. Now, in that example that I just tried it out right now you can see following commented code:

  /* Set Output Data Rate
   * The recommended accelerometer ODR for single and
   * double-click recognition is 400 Hz or higher
   */
  iis2dh_data_rate_set(&dev_ctx, IIS2DH_ODR_400Hz);
  /* Set full scale to 2 g */
  iis2dh_full_scale_set(&dev_ctx, IIS2DH_2g);
  /* Set click threshold to 12h  -> 0.281 g
   * 1 LSB = full scale/128
   * Set TIME_LIMIT to 33h -> 127 ms.
   * 1 LSB = 1/ODR
   */
  iis2dh_tap_threshold_set(&dev_ctx, 0x12);
  iis2dh_shock_dur_set(&dev_ctx, 0x33);

This is an hint on how to configure it on zephyr. Please, consider that this example works in polling mode only, but it is first thing that we need to understand before going on. If this part is ok and event is present in CLICK_SRC, then let's see interrupt part. Does it make sens to you?

EDIT: If not clear, check ODR (recommended 400 Hz at least), threshold (you set 0.5 g, right? Maybe you may want to decrease), and duration (in my case 127ms works fine).

avisconti commented 3 months ago

@MaxMaeder any thoughts?

MaxMaeder commented 3 months ago

Just tried out your fixes, am seeing interrupt events being generated now and the driver is working! I set odr to 400 Hz, full scale to 2g, threshold as 0.5g, and duration as 127ms. I am using a LIS2DH12

A couple follow up questions, though, since the driver isn't super well documented:

  1. How do I decrease the sensitivity of the click detection? I've tried simple things like increasing the threshold, but still multiple events are generated for one physical tap and non-tap events like vibrations from the speaker on my device trigger it.
  2. Is there a way in the driver to enable click/tap detection on only one axis? My understanding is that the sensor supports it.

Thanks so much for your help!

avisconti commented 3 months ago

Just tried out your fixes, am seeing interrupt events being generated now and the driver is working! I set odr to 400 Hz, full scale to 2g, threshold as 0.5g, and duration as 127ms. I am using a LIS2DH12

A couple follow up questions, though, since the driver isn't super well documented:

  1. How do I decrease the sensitivity of the click detection? I've tried simple things like increasing the threshold, but still multiple events are generated for one physical tap and non-tap events like vibrations from the speaker on my device trigger it.

Well, I guess you have to play with the threshold indeed. It sounds strange to me that with an higher threshold you are still seeing taps triggered... You may also want to take a look to chapter 8 of AN5005:

https://www.st.com/resource/en/application_note/an5005-lis2dh12-ultralowpower-highperformance-3axis-nano-accelerometer-with-digital-output-stmicroelectronics.pdf

If still doesn't work I may ask internally of any issue related to this. But I'm currently aware of any such bad behaviours like this.

  1. Is there a way in the driver to enable click/tap detection on only one axis? My understanding is that the sensor supports it.

Isn't it the configuration in CLICK_CFG (38h) register? Have you already tried it out?

PS: if you put XD/XS/YD/YS to 0 and ZD/ZS to 1, then you should get double tap only on Z axis.

MaxMaeder commented 3 months ago

Ok, I got it working, thanks! However, my solution required me to 'hardcode' changing the registers into the driver code. There didn't appear to be any way to change these register values implemented in the public api for the sensor.

I chose to configure it for double-tap detection on the Z-Axis, which required me to change:

This should probably be available in a public api-I'm thinking the best way would be through configuration in the dts binding? I don't have a ton of experience working with Zephyr device drivers (they seem really over-complicated compared to other embedded offerings) so let me know if this seems like a reasonable solution.

avisconti commented 3 months ago

This should probably be available in a public api-I'm thinking the best way would be through configuration in the dts binding? I don't have a ton of experience working with Zephyr device drivers (they seem really over-complicated compared to other embedded offerings) so let me know if this seems like a reasonable solution.

Yes, this sounds reasonable. I may add a PR addressing this request. Thanks for highlighting this issue.

PS Or, maybe, can you add a PR addressing ALL the changes you have done so that the driver can be properly configured in a right way? Is that too much burden?

avisconti commented 2 months ago

I'm closing the issue as it seems solved. Feel free to re-open it if it is the case.