linguini1 commented on code in PR #19359: URL: https://github.com/apache/nuttx/pull/19359#discussion_r3545102722
########## include/nuttx/sensors/mpu6050.h: ########## @@ -0,0 +1,119 @@ +/**************************************************************************** + * include/nuttx/sensors/mpu6050.h + * + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. The + * ASF licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the + * License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + ****************************************************************************/ + +#ifndef __INCLUDE_NUTTX_SENSORS_MPU6050_H +#define __INCLUDE_NUTTX_SENSORS_MPU6050_H + +/**************************************************************************** + * Included Files + ****************************************************************************/ + +#include <nuttx/config.h> +#include <nuttx/sensors/ioctl.h> +#include <nuttx/sensors/sensor.h> + +/**************************************************************************** + * Pre-processor Definitions + ****************************************************************************/ + +/* MPU6050 I2C addresses */ + +#define MPU6050_ADDR_LOW 0x68 +#define MPU6050_ADDR_HIGH 0x69 + +/* Register addresses */ + +#define MPU6050_SMPLRT_DIV 0x19 +#define MPU6050_CONFIG 0x1a +#define MPU6050_GYRO_CONFIG 0x1b +#define MPU6050_ACCEL_CONFIG 0x1c +#define MPU6050_INT_ENABLE 0x38 +#define MPU6050_INT_STATUS 0x3a +#define MPU6050_ACCEL_XOUT_H 0x3b +#define MPU6050_ACCEL_XOUT_L 0x3c +#define MPU6050_ACCEL_YOUT_H 0x3d +#define MPU6050_ACCEL_YOUT_L 0x3e +#define MPU6050_ACCEL_ZOUT_H 0x3f +#define MPU6050_ACCEL_ZOUT_L 0x40 +#define MPU6050_TEMP_OUT_H 0x41 +#define MPU6050_TEMP_OUT_L 0x42 +#define MPU6050_GYRO_XOUT_H 0x43 +#define MPU6050_GYRO_XOUT_L 0x44 +#define MPU6050_GYRO_YOUT_H 0x45 Review Comment: Register definitions should only go in the private C file, they don't need to be accessible via the public API. ########## drivers/sensors/Kconfig: ########## @@ -1491,15 +1491,15 @@ config MPU60X0_EXT_SYNC_SET ---help--- EXT_SYNC_SET[2..0] EXT_SYNC_SET: frame sync bit position - (see datasheet, it's ... complicated) + (see datasheet, its ... complicated) config MPU60X0_DLPF_CFG int "MPU60x0 digital low-pass filter bandwidth" default 1 ---help--- DLPF_CFG[2..0] DLPF_CFG: digital low-pass filter bandwidth - (see datasheet, it's ... complicated) + (see datasheet, its ... complicated) Review Comment: `it's` is correct here. ########## drivers/sensors/mpu6050_uorb.c: ########## @@ -0,0 +1,494 @@ +/**************************************************************************** + * drivers/sensors/mpu6050_uorb.c + * + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. The + * ASF licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the + * License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + * + ****************************************************************************/ + +/**************************************************************************** + * Included Files + ****************************************************************************/ + +#include <nuttx/config.h> + +#include <debug.h> +#include <errno.h> +#include <math.h> +#include <stdio.h> +#include <stdlib.h> +#include <string.h> + +#include <nuttx/fs/fs.h> +#include <nuttx/i2c/i2c_master.h> +#include <nuttx/kmalloc.h> +#include <nuttx/mutex.h> +#include <nuttx/sensors/mpu6050.h> +#include <nuttx/sensors/sensor.h> + +#ifdef CONFIG_SENSORS_MPU6050 + +/**************************************************************************** + * Pre-processor Definitions + ****************************************************************************/ + +#define MPU6050_I2C_FREQUENCY 400000 /* 400 kHz */ +#define CONSTANTS_ONE_G 9.80665f + +/**************************************************************************** + * Private Types + ****************************************************************************/ + +enum mpu6050_idx_e +{ + MPU6050_ACCEL_IDX = 0, + MPU6050_GYRO_IDX, + MPU6050_MAX_IDX +}; + +struct mpu6050_dev_s +{ + FAR struct i2c_master_s *i2c; + uint8_t addr; + int freq; + mutex_t dev_lock; +}; + +struct mpu6050_sensor_s +{ + struct sensor_lowerhalf_s lower; + float scale; + FAR struct mpu6050_dev_s *dev; + bool enabled; +}; + +struct mpu6050_uorb_dev_s +{ + struct mpu6050_dev_s base; + struct mpu6050_sensor_s priv[MPU6050_MAX_IDX]; +}; + +/**************************************************************************** + * Private Function Prototypes + ****************************************************************************/ + +static int mpu6050_activate(FAR struct sensor_lowerhalf_s *lower, + FAR struct file *filep, bool enable); +static int mpu6050_fetch(FAR struct sensor_lowerhalf_s *lower, + FAR struct file *filep, + FAR char *buffer, size_t buflen); +static int mpu6050_control(FAR struct sensor_lowerhalf_s *lower, + FAR struct file *filep, + int cmd, unsigned long arg); + +/**************************************************************************** + * Private Data + ****************************************************************************/ + +static const struct sensor_ops_s g_mpu6050_ops = +{ + .activate = mpu6050_activate, + .fetch = mpu6050_fetch, Review Comment: How fast can this sensor sample? It might be worth it to have a kthread or interrupt-based approach so the user doesn't have to poll. ########## drivers/sensors/mpu6050_uorb.c: ########## @@ -0,0 +1,494 @@ +/**************************************************************************** + * drivers/sensors/mpu6050_uorb.c + * + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. The + * ASF licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the + * License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + * + ****************************************************************************/ + +/**************************************************************************** + * Included Files + ****************************************************************************/ + +#include <nuttx/config.h> + +#include <debug.h> +#include <errno.h> +#include <math.h> +#include <stdio.h> +#include <stdlib.h> +#include <string.h> + +#include <nuttx/fs/fs.h> +#include <nuttx/i2c/i2c_master.h> +#include <nuttx/kmalloc.h> +#include <nuttx/mutex.h> +#include <nuttx/sensors/mpu6050.h> +#include <nuttx/sensors/sensor.h> + +#ifdef CONFIG_SENSORS_MPU6050 + +/**************************************************************************** + * Pre-processor Definitions + ****************************************************************************/ + +#define MPU6050_I2C_FREQUENCY 400000 /* 400 kHz */ +#define CONSTANTS_ONE_G 9.80665f + +/**************************************************************************** + * Private Types + ****************************************************************************/ + +enum mpu6050_idx_e +{ + MPU6050_ACCEL_IDX = 0, + MPU6050_GYRO_IDX, + MPU6050_MAX_IDX +}; + +struct mpu6050_dev_s +{ + FAR struct i2c_master_s *i2c; + uint8_t addr; + int freq; + mutex_t dev_lock; +}; + +struct mpu6050_sensor_s +{ + struct sensor_lowerhalf_s lower; + float scale; + FAR struct mpu6050_dev_s *dev; + bool enabled; +}; + +struct mpu6050_uorb_dev_s +{ + struct mpu6050_dev_s base; + struct mpu6050_sensor_s priv[MPU6050_MAX_IDX]; Review Comment: Why do it this way? Just add two members; `accel` and `gyro`. It is more readable. ########## drivers/sensors/mpu6050_uorb.c: ########## Review Comment: Please document this sensor. ########## drivers/sensors/Kconfig: ########## @@ -1533,6 +1533,13 @@ config MPU60X0_ACCEL_AFS_SEL endif # SENSORS_MPU60X0 +config SENSORS_MPU6050 Review Comment: How is your support different from the MPU60X0 support? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
