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

hw/bsp/dwm1001-dev Add LIS2DH12 accelerometer support #2639

Closed
wants to merge 1 commit into from

Conversation

seijikun
Copy link

@seijikun seijikun commented Jul 6, 2021

Hey guys,

thanks for this awesome piece of software!
This PR is an attempt at adding support for the LIS2DH12 accelerometer sensor, which is integrated with DWM1001-DEV boards.
I'm fairly new to mynewt, so this PR is more like a shot in the dark, and mostly copied from other BSPs with the same accelerometer.
The sensor_itf.si_ints values are chosen, such that it does not crash - which does not mean that it's working ;). Would be good if somebody with more hardware knowledge could take a look at them (the .active = true certainly isn't true, this should be something like HAL_GPIO_TRIG_RISING or HAL_GPIO_TRIG_FALLING).

Here is an example application:

#include <assert.h>
#include <string.h>
#include <stdio.h>
#include <math.h>
#include "sysinit/sysinit.h"
#include "os/os.h"
#include "bsp/bsp.h"
#include "hal/hal_gpio.h"
#include "hal/hal_bsp.h"
#include "hal/hal_i2c.h"
#include "mcu/nrf52_hal.h"
#ifdef ARCH_sim
#include "mcu/mcu_sim.h"
#endif

#include <config/config.h>

#include <sensor/sensor.h>
#include "sensor/accel.h"
#include <lis2dh12/lis2dh12.h>

#if MYNEWT_VAL(DW1000_DEVICE_0)
#include <dw1000/dw1000_hal.h>
#endif

int accel_data_cb(struct sensor* sensor, void* a, void* databuf, sensor_type_t sensorType) {
    if(!databuf) { return SYS_EINVAL; }
    struct sensor_accel_data* sad = (struct sensor_accel_data*) databuf;
    if (!sad->sad_x_is_valid || !sad->sad_y_is_valid || !sad->sad_z_is_valid) { return SYS_EINVAL; }
    printf("%s: [ secs: %ld usecs: %d cputime: %u ]\n",
                   "LISTENER_CB",
                   (long int)sensor->s_sts.st_ostv.tv_sec,
                   (int)sensor->s_sts.st_ostv.tv_usec,
                   (unsigned int)sensor->s_sts.st_cputime);

    printf("x = %f, y = %f, z = %f\n", sad->sad_x, sad->sad_y, sad->sad_z);
    return 0;
}

static struct sensor_listener sensor_listener = {
    .sl_sensor_type = SENSOR_TYPE_ACCELEROMETER,
    .sl_func = accel_data_cb,  
    .sl_arg = NULL
};

 

int main(int argc, char **argv){
    sysinit();

    struct sensor* accel = sensor_mgr_find_next_bydevname("lis2dh12_0", NULL);
    assert(accel != NULL);

    assert(0 == lis2dh12_set_full_scale(&accel->s_itf, LIS2DH12_FS_2G));
    assert(0 == lis2dh12_set_op_mode(&accel->s_itf, LIS2DH12_OM_HIGH_RESOLUTION));

    sensor_set_poll_rate_ms(accel->s_dev->od_name, 1000 / 50);
    sensor_register_listener(accel, &sensor_listener);

    while (1) {
        os_eventq_run(os_eventq_dflt_get());
    }
    assert(0);
    return 0;
}

@mlaz
Copy link
Contributor

mlaz commented Jul 13, 2021

Sounds like you need to look at the module's (DWM1001, not DWM1001-dev) schematic there you should find all the pinout info.
I'd advise that if you need any help it might be easier if you reach out on slack before posting PRs.

Copy link
Contributor

@kasjer kasjer left a comment

Choose a reason for hiding this comment

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

Minor style (extra spaces and line) changes would be nice.
restriction should be changed to allow turning sensor on and off.


dev = (struct os_dev *) os_dev_open("lis2dh12_0", OS_TIMEOUT_NEVER, NULL);
assert(dev != NULL);

Copy link
Contributor

Choose a reason for hiding this comment

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

extra empty line

struct os_dev *dev;
struct lis2dh12_cfg cfg;

dev = (struct os_dev *) os_dev_open("lis2dh12_0", OS_TIMEOUT_NEVER, NULL);
Copy link
Contributor

Choose a reason for hiding this comment

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

in mynewt code it is custom not to put space after finishing ) in casts like this

dev = (struct os_dev *)os_dev_open(....

(void)rc;

#if MYNEWT_VAL(LIS2DH12_ONB)
rc = os_dev_create((struct os_dev *) &lis2dh12, "lis2dh12_0",
Copy link
Contributor

Choose a reason for hiding this comment

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

extra space after cast expression

Comment on lines +75 to +76
syscfg.restrictions:
- "LIS2DH12_ONB && I2C_0_PIN_SCL == 28 && I2C_0_PIN_SDA == 29"
Copy link
Contributor

Choose a reason for hiding this comment

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

if it was:

syscfg.restrictions:
    - "!LIS2DH12_ONB || (I2C_0 == 1 && I2C_0_PIN_SCL == 28 && I2C_0_PIN_SDA == 29)"

sensor could be turned on and off as syscfg value implies it could.
notice extra I2C_0 == 1 that is also needed I guess

Comment on lines +52 to +56
{ /* TODO: determine correct values */
.host_pin = 25,
.device_pin = 0,
.active = true
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I would remove this second table entry, first one seems to be correct

pkg.deps.LIS2DH12_ONB:
- "@apache-mynewt-core/hw/drivers/sensors/lis2dh12"

pkg.init:
Copy link
Contributor

Choose a reason for hiding this comment

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

if it was changed to:

pkg.init.LIS2DH12_ONB:

whole config_lis2dh12_sensor() function could be ifdef'ed-out not just its body

Copy link
Contributor

Choose a reason for hiding this comment

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

Fist line of commit message would also be more consistent with what is used in mynewt if it looked like text that was chosen for PR title:

hw/bsp/dwm1001-dev: Add LIS2DH12 accelerometer support

instead of "Add LIS2DH12 accelerometer support for DWM1001-dev boards"

@seijikun
Copy link
Author

Sorry, I don't have access to the hardware anymore, so I can't continue on this with testing.
I tried having only one entry in the interrupts array, that crashed on initialization.
I'm going to close this. Sorry again!

@seijikun seijikun closed this Sep 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants