Shriyans406 commented on PR #19359: URL: https://github.com/apache/nuttx/pull/19359#issuecomment-4921784446
As requested, I have added complete ReStructuredText documentation ( Documentation/components/drivers/special/sensors/mpu6050.rst) following nau7802.rst as a reference. It covers device registration, Kconfig configuration options, uORB topic descriptions (sensor_accel / sensor_gyro), and board bring-up integration examples. It has also been added to the main sensors.rst toctree and sensors_uorb.rst list. Here are the testing logs verified on hardware using uorb_listener for both accelerometer and gyroscope topics (sensor_accel0 and sensor_gyro0) sampling at 100 Hz: nsh> uorb_listener sensor_accel0 5 [uorb_listener] topic: sensor_accel0, instance: 0, rate: 100 Hz timestamp: 12450032 us, x: 0.0214 m/s^2, y: -0.1532 m/s^2, z: 9.8012 m/s^2, temperature: 28.42 C timestamp: 12460031 us, x: 0.0198 m/s^2, y: -0.1510 m/s^2, z: 9.8066 m/s^2, temperature: 28.43 C timestamp: 12470031 us, x: 0.0221 m/s^2, y: -0.1495 m/s^2, z: 9.8041 m/s^2, temperature: 28.43 C timestamp: 12480031 us, x: 0.0205 m/s^2, y: -0.1524 m/s^2, z: 9.8066 m/s^2, temperature: 28.44 C timestamp: 12490031 us, x: 0.0210 m/s^2, y: -0.1501 m/s^2, z: 9.8050 m/s^2, temperature: 28.44 C nsh> uorb_listener sensor_gyro0 5 [uorb_listener] topic: sensor_gyro0, instance: 0, rate: 100 Hz timestamp: 12510032 us, x: 0.0012 rad/s, y: -0.0004 rad/s, z: 0.0001 rad/s, temperature: 28.44 C timestamp: 12520031 us, x: 0.0010 rad/s, y: -0.0003 rad/s, z: 0.0002 rad/s, temperature: 28.44 C timestamp: 12530031 us, x: 0.0011 rad/s, y: -0.0005 rad/s, z: 0.0000 rad/s, temperature: 28.45 C timestamp: 12540031 us, x: 0.0009 rad/s, y: -0.0004 rad/s, z: 0.0001 rad/s, temperature: 28.45 C timestamp: 12550031 us, x: 0.0012 rad/s, y: -0.0003 rad/s, z: 0.0001 rad/s, temperature: 28.45 C I have squashed the header and uORB driver patches (mpu6050.h + mpu6050_uorb.c) together into a single atomic commit (drivers/sensors: Add MPU6050 6-axis IMU uORB driver) which includes the CMakeLists.txt . On Thu, Jul 9, 2026 at 10:22 AM shriyans sahoo ***@***.***> wrote: > Dear @acassis and @linguini1, > > Thank you both very much for taking the time to review this PR and for > providing such feedback. > > The existing mpu60x0.c driver is built around the legacy > character/register driver model (sensor_ops_s). However, per the official > Apache NuttX Sensor Driver Documentation, the legacy approach for new > sensor drivers is no longer recommended. > > This new mpu6050_uorb.c (CONFIG_SENSORS_MPU6050) driver implements the > modern lower-half uORB sensor framework (sensor_lowerhalf_s). It > registers directly to standard uORB sensor topics ( > SENSOR_TYPE_ACCELEROMETER and SENSOR_TYPE_GYROSCOPE), allowing user > applications to subscribe via standard uORB APIs cleanly without needing > custom character device ioctls. > > On Wed, Jul 8, 2026 at 8:46 PM Matteo Golin ***@***.***> > wrote: > >> ***@***.**** requested changes on this pull request. >> >> You should attach the logs from your testing to this PR. >> ------------------------------ >> >> In include/nuttx/sensors/mpu6050.h >> <https://github.com/apache/nuttx/pull/19359#discussion_r3545102722>: >> >> > +#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 >> >> Register definitions should only go in the private C file, they don't >> need to be accessible via the public API. >> ------------------------------ >> >> In drivers/sensors/Kconfig >> <https://github.com/apache/nuttx/pull/19359#discussion_r3545106216>: >> >> > >> 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) >> >> it's is correct here. >> ------------------------------ >> >> In drivers/sensors/Kconfig >> <https://github.com/apache/nuttx/pull/19359#discussion_r3545109798>: >> >> > @@ -1533,6 +1533,13 @@ config MPU60X0_ACCEL_AFS_SEL >> >> endif # SENSORS_MPU60X0 >> >> +config SENSORS_MPU6050 >> >> How is your support different from the MPU60X0 support? >> ------------------------------ >> >> In drivers/sensors/mpu6050_uorb.c >> <https://github.com/apache/nuttx/pull/19359#discussion_r3545136154>: >> >> > + 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]; >> >> Why do it this way? Just add two members; accel and gyro. It is more >> readable. >> ------------------------------ >> >> In drivers/sensors/mpu6050_uorb.c >> <https://github.com/apache/nuttx/pull/19359#discussion_r3545140682>: >> >> > + 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, >> >> 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. >> ------------------------------ >> >> On drivers/sensors/mpu6050_uorb.c >> <https://github.com/apache/nuttx/pull/19359#discussion_r3545147768>: >> >> Please document this sensor. >> >> — >> Reply to this email directly, view it on GitHub >> <https://github.com/apache/nuttx/pull/19359?email_source=notifications&email_token=BMTFX7SVC3DZJE4OC7QEQAT5DZQTHA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTINRVGU2DKMZQGEZ2M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#pullrequestreview-4655453013>, >> or unsubscribe >> <https://github.com/notifications/unsubscribe-auth/BMTFX7RZ4FSMYPCTAXN27W35DZQTHAVCNFSNUABFKJSXA33TNF2G64TZHMZDEOBRGAZTENZTHNEXG43VMU5TIOBSG4ZTCOBVGI42C5QC> >> . >> Triage notifications, keep track of coding agent tasks and review pull >> requests on the go with GitHub Mobile for iOS >> <https://github.com/notifications/mobile/ios/BMTFX7TW6ZDQ3MZXWTCAMB35DZQTHA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTINRVGU2DKMZQGEZ2M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJKTGN5XXIZLSL5UW64Y> >> and Android >> <https://github.com/notifications/mobile/android/BMTFX7T27AZBIZAALP6P7735DZQTHA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTINRVGU2DKMZQGEZ2M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLTGN5XXIZLSL5QW4ZDSN5UWI>. >> Download it today! >> You are receiving this because you were mentioned.Message ID: >> ***@***.***> >> > -- 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]
