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

drivers/sensors: Added uORB driver for LSM6DSO32 IMU. #15789

Merged
merged 1 commit into from
Feb 12, 2025

Conversation

linguini1
Copy link
Contributor

@linguini1 linguini1 commented Feb 8, 2025

Summary

Adds support for the LSM6DSO32 IMU by STM using the uORB framework. This only contains limited support for the I2C interface, interrupt and polling driven measurement and standard modes of operation (high performance ODRs). Features like interrupts besides DRDY, filtering, gesture recognition and acting as a bus master are not implemented.

Accompanying documentation included.

Impact

This PR adds another available uORB driver to the NuttX sensor drivers.
It impacts the build system by adding more Kconfig options and impacts the kernel with more driver source files.

Testing

All builds were performed on a Linux host. All testing was performed on an RP2040 MCU on a custom flight computer board.

Below is the script that was used for testing.

#include <errno.h>
#include <fcntl.h>
#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>

#include <nuttx/sensors/lsm6dso32.h>
#include <uORB/uORB.h>

#define SAMPLE_FREQ 50
#define ACCEL_NAME "sensor_accel0"
#define GYRO_NAME "sensor_gyro0"

int main(int argc, char **argv)
{
  int err;
  unsigned frequency;

  int accel;
  struct sensor_accel accel_data;

  int gyro;
  struct sensor_gyro gyro_data;

  struct sensor_device_info_s info;
  bool update = false;

  printf("TESTING ACCELEROMETER IMPLEMENTATION\n");

  /* Get accelerometer metadata */

  const struct orb_metadata *accel_meta = orb_get_meta(ACCEL_NAME);
  if (accel_meta == NULL)
    {
      fprintf(stderr, "Failed to get metadata for %s\n", ACCEL_NAME);
      return EXIT_FAILURE;
    }

  /* Subscribe to accelerometer */

  accel = orb_subscribe(accel_meta);
  if (accel < 0)
    {
      fprintf(stderr, "Could not subsribe to %s: %d\n", ACCEL_NAME, errno);
      return EXIT_FAILURE;
    }

  /* Set sampling rate to 50Hz */

  err = orb_set_frequency(accel, SAMPLE_FREQ);
  if (err)
    {
      fprintf(stderr, "Wasn't able to set frequency to %uHz: %d\n",
              SAMPLE_FREQ, err);
      return EXIT_FAILURE;
    }

  /* Show sampling rate */

  err = orb_get_frequency(accel, &frequency);
  if (err)
    {
      fprintf(stderr, "Could not verify frequency: %d\n", err);
      return EXIT_FAILURE;
    }

  printf("Sampling frequency is %uHz\n", frequency);

  /* Print sensor information */

  err = orb_ioctl(accel, SNIOC_GET_INFO, (unsigned long)&info);
  if (err < 0)
    {
      fprintf(stderr, "Could not get sensor information: %d", errno);
      return EXIT_FAILURE;
    }

  printf("Sensor: %s\n", info.name);
  printf("Manufacturer: %s\n", info.vendor);
  printf("Power consumption: %.2fmA\n", info.power);
  printf("Resolution: %.6f m/s^2\n", info.resolution);
  printf("Max range: %.2f m/s^2\n", info.max_range);
  printf("Min delay: %ld us\n", info.min_delay);

  /* Perform self-test */

  err = orb_ioctl(accel, SNIOC_SELFTEST, 0);
  if (err < 0)
    {
      fprintf(stderr, "Self test failed: %d\n", errno);
      return EXIT_FAILURE;
    }
  printf("Self test passed!\n");

  /* Loop forever while printing out tab separated data as X, Y, Z */

  for (int i = 0; i < 10;)
    {
      err = orb_check(accel, &update);
      if (err)
        {
          fprintf(stderr, "Failed to check for new accel_data: %d\n", err);
          continue;
        }

      /* Grab latest accel_data */

      if (update)
        {
          err = orb_copy(accel_meta, accel, &accel_data);
          if (err)
            {
              fprintf(stderr, "Failed to get new accel_data: %d\n", err);
              continue;
            }

          printf("%f\t%f\t%f\n", accel_data.x, accel_data.y, accel_data.z);
          i++;
        }
    }

  /* Subtract offset */

  float offsets[3] = {0.0f, -0.4f, 9.98f};
  err = orb_ioctl(accel, SNIOC_SET_CALIBVALUE, (unsigned long)offsets);
  if (err < 0)
    {
      fprintf(stderr, "Failed to set calibration values: %d", err);
      return EXIT_FAILURE;
    }

  printf("Offsets set: %.2f X, %.2f Y, %.2f Z\n", offsets[0], offsets[1],
         offsets[2]);

  /* Loop forever while printing out tab separated accel_data as X, Y, Z */

  for (int i = 0; i < 10;)
    {
      err = orb_check(accel, &update);
      if (err)
        {
          fprintf(stderr, "Failed to check for new accel_data: %d\n", err);
          continue;
        }

      /* Grab latest accel_data */

      if (update)
        {
          err = orb_copy(accel_meta, accel, &accel_data);
          if (err)
            {
              fprintf(stderr, "Failed to get new accel_data: %d\n", err);
              continue;
            }

          printf("%f\t%f\t%f\n", accel_data.x, accel_data.y, accel_data.z);
          i++;
        }
    }

  printf("TESTING GYROSCOPE IMPLEMENTATION\n");

  /* Get gyro metadata */

  const struct orb_metadata *gyro_meta = orb_get_meta(GYRO_NAME);
  if (gyro_meta == NULL)
    {
      fprintf(stderr, "Failed to get metadata for %s\n", GYRO_NAME);
      return EXIT_FAILURE;
    }

  /* Subscribe to gyroerometer */

  gyro = orb_subscribe(gyro_meta);
  if (gyro < 0)
    {
      fprintf(stderr, "Could not subsribe to %s: %d\n", GYRO_NAME, errno);
      return EXIT_FAILURE;
    }

  /* Set sampling rate to 50Hz */

  err = orb_set_frequency(gyro, SAMPLE_FREQ);
  if (err)
    {
      fprintf(stderr, "Wasn't able to set frequency to %uHz: %d\n",
              SAMPLE_FREQ, err);
      return EXIT_FAILURE;
    }

  /* Show sampling rate */

  err = orb_get_frequency(gyro, &frequency);
  if (err)
    {
      fprintf(stderr, "Could not verify frequency: %d\n", err);
      return EXIT_FAILURE;
    }

  printf("Sampling frequency is %uHz\n", frequency);

  /* Print sensor information */

  err = orb_ioctl(gyro, SNIOC_GET_INFO, (unsigned long)&info);
  if (err < 0)
    {
      fprintf(stderr, "Could not get sensor information: %d", errno);
      return EXIT_FAILURE;
    }

  printf("Sensor: %s\n", info.name);
  printf("Manufacturer: %s\n", info.vendor);
  printf("Power consumption: %.2fmA\n", info.power);
  printf("Resolution: %.6f rad/s\n", info.resolution);
  printf("Max range: %.2f rad/s\n", info.max_range);
  printf("Min delay: %ld us\n", info.min_delay);

  /* Perform self-test */

  err = orb_ioctl(gyro, SNIOC_SELFTEST, 0);
  if (err < 0)
    {
      fprintf(stderr, "Self test failed: %d\n", errno);
      return EXIT_FAILURE;
    }
  printf("Self test passed!\n");

  /* Loop forever while printing out tab separated gyro_data as X, Y, Z */

  for (int i = 0; i < 10;)
    {
      err = orb_check(gyro, &update);
      if (err)
        {
          fprintf(stderr, "Failed to check for new gyro_data: %d\n", err);
          continue;
        }

      /* Grab latest gyro_data */

      if (update)
        {
          err = orb_copy(gyro_meta, gyro, &gyro_data);
          if (err)
            {
              fprintf(stderr, "Failed to get new gyro_data: %d\n", err);
              continue;
            }

          printf("%f\t%f\t%f\n", gyro_data.x, gyro_data.y, gyro_data.z);
          i++;
        }
    }

  /* Subtract offset */

  float offsets2[3] = {0.0f, -1.0f, 0.0f};
  err = orb_ioctl(gyro, SNIOC_SET_CALIBVALUE, (unsigned long)offsets2);
  if (err < 0)
    {
      fprintf(stderr, "Failed to set calibration values: %d", err);
      return EXIT_FAILURE;
    }

  printf("Offsets set: %.2f X, %.2f Y, %.2f Z\n", offsets2[0], offsets2[1],
         offsets2[2]);

  /* Loop forever while printing out tab separated gyro_data as X, Y, Z */

  for (int i = 0; i < 10;)
    {
      err = orb_check(gyro, &update);
      if (err)
        {
          fprintf(stderr, "Failed to check for new gyro_data: %d\n", err);
          continue;
        }

      /* Grab latest gyro_data */

      if (update)
        {
          err = orb_copy(gyro_meta, gyro, &gyro_data);
          if (err)
            {
              fprintf(stderr, "Failed to get new gyro_data: %d\n", err);
              continue;
            }

          printf("%f\t%f\t%f\n", gyro_data.x, gyro_data.y, gyro_data.z);
          i++;
        }
    }

  uint8_t id = 0;
  err = orb_ioctl(gyro, SNIOC_WHO_AM_I, (unsigned long)&id);
  if (err < 0)
    {
      fprintf(stderr, "Could not get WHOAMI value: %d\n", errno);
      return EXIT_FAILURE;
    }
  printf("WHOAMI: %02X\n", id);

  printf("Changing FSR of accelerometer to +/-32g\n");
  err = orb_ioctl(accel, SNIOC_SETFULLSCALE, LSM6DSO32_FSR_XL_32G);
  if (err < 0)
    {
      fprintf(stderr, "Could not get WHOAMI value: %d\n", errno);
      return EXIT_FAILURE;
    }

  printf("New measurement values:\n");

  for (int i = 0; i < 10;)
    {
      err = orb_check(accel, &update);
      if (err)
        {
          fprintf(stderr, "Failed to check for new accel_data: %d\n", err);
          continue;
        }

      /* Grab latest accel_data */

      if (update)
        {
          err = orb_copy(accel_meta, accel, &accel_data);
          if (err)
            {
              fprintf(stderr, "Failed to get new accel_data: %d\n", err);
              continue;
            }

          printf("%f\t%f\t%f\n", accel_data.x, accel_data.y, accel_data.z);
          i++;
        }
    }

  orb_unsubscribe(accel);
  orb_unsubscribe(gyro);
  return EXIT_SUCCESS;
}

This script tests the following:

  • Measurements are within the correct range for stationary board (i.e. 9.81~m/s^2 in one axis, 0 in others and 0 angular velocity in any direction)
  • Self-tests for the sensors pass
  • Self-tests for the sensors do not affect subsequent measurements due to hysteresis
  • WHOAMI register is read and the value is correct
  • SETFULLSCALE changes the range of the sensor correctly and measurements are correct following this change
  • Measurement values are correctly modified by using SET_CALIBVALUE
  • Measurement frequency can be set
  • Changing FSR and ODR correctly changes the data in get_info
  • get_info returns the correct information depending on accel/gyro

Here is the output of the script in interrupt-driven mode:

TESTING ACCELEROMETER IMPLEMENTATION
Sampling frequency is 50Hz
Sensor: LSM6DSO32
Manufacturer: STMicro
Power consumption: 0.55mA
Resolution: 0.009571 m/s^2
Max range: 313.62 m/s^2
Min delay: 80000 us
Self test passed!
0.248855        -2.871402       79.786682
0.248855        -2.871402       79.786682
0.239283        -1.866411       51.685230
0.095713        -0.038285       0.583852
0.105285        -0.028714       0.220141
0.095713        -0.038285       0.229712
0.105285        -0.057428       0.229712
0.114856        -0.057428       0.210569
0.086142        -0.047856       0.229712
0.124427        0.000000        0.210569
Offsets set: 0.00 X, -0.40 Y, 9.98 Z
0.105285        -0.038285       0.239283
0.105285        -0.038285       0.229712
0.105285        -0.028714       0.229712
0.105285        -0.066999       0.229712
0.105285        -0.047856       0.239283
0.086142        -0.057428       0.220141
0.124427        -0.057428       0.229712
0.114856        -0.019142       0.200998
0.095713        -0.047856       0.229712
0.105285        -0.028714       0.248855
TESTING GYROSCOPE IMPLEMENTATION
Sampling frequency is 50Hz
Sensor: LSM6DSO32
Manufacturer: STMicro
Power consumption: 0.55mA
Resolution: 0.000076 rad/s
Max range: 2.50 rad/s
Min delay: 80000 us
Self test passed!
0.001221        1.003742        0.001450
0.001221        1.003742        0.001450
0.001298        1.003665        0.001221
0.001069        1.003894        0.001450
0.001298        1.003513        0.001527
0.000687        1.003742        0.001450
0.000687        1.003971        0.001985
0.000610        1.003894        0.001679
0.000687        1.003971        0.001603
0.000992        1.003818        0.001832
Offsets set: 0.00 X, -1.00 Y, 0.00 Z
0.001145        1.003894        0.001603
0.001298        1.003742        0.001603
0.001298        1.003665        0.001603
0.001221        1.003818        0.001450
0.001298        1.003818        0.001298
0.000916        1.004276        0.001450
0.000687        1.004047        0.001450
0.000763        1.003971        0.001374
0.000839        1.003818        0.001527
0.000992        1.003818        0.001450
WHOAMI: 6C
Changing FSR of accelerometer to +/-32g
New measurement values:
0.105285        -0.066999       0.258426
0.124427        -0.047856       0.267998
0.124427        -0.047856       0.267998
0.114856        -0.028714       0.267998
0.133999        -0.019142       0.248855
0.114856        -0.019142       0.258426
0.105285        -0.019142       0.258426
0.133999        -0.038285       0.248855
0.124427        -0.057428       0.258426
0.124427        -0.019142       0.248855

Here is the output of the script in polling mode:

TESTING ACCELEROMETER IMPLEMENTATION                                 
Sampling frequency is 50Hz                                           
Sensor: LSM6DSO32                                                    
Manufacturer: STMicro                                                
Power consumption: 0.55mA
Resolution: 0.001196 m/s^2
Max range: 39.20 m/s^2
Min delay: 80000 us
Self test passed!
0.000000        0.000000        0.000000
0.059820        -0.386443       9.972138
0.044267        -0.378068       9.982906
0.056231        -0.378068       9.973335
0.062213        -0.373282       9.972138
0.064606        -0.372086       9.978121
0.049053        -0.363711       9.964960
0.059820        -0.379264       9.976925
0.047856        -0.375675       9.988889
0.039481        -0.373282       9.982906
Offsets set: 0.00 X, -0.40 Y, 9.98 Z
0.041874        -0.082552       0.168695
0.062213        -0.087338       0.169891
0.064606        -0.084945       0.173481
0.056231        -0.068195       0.174677
0.051445        -0.081356       0.172284
0.059820        -0.070588       0.172284
0.050249        -0.074177       0.167498
0.065803        -0.071785       0.187838
0.059820        -0.069392       0.180659
0.045463        -0.075374       0.181855
ENTATION
Sampling frequency is 50Hz
Sensor: LSM6DSO32
Manufacturer: STMicro
Power consumption: 0.55mA
Resolution: 0.000076 rad/s
Max range: 2.50 rad/s
Min delay: 80000 us
Self test passed!
0.000076        -0.007177       0.003054
0.000992        0.003665        0.001450
0.001298        0.003512        0.001450
0.001374        0.003741        0.001145
0.001221        0.003894        0.001527
0.001298        0.003665        0.001374
0.001145        0.003665        0.001756
0.001374        0.003665        0.001527
0.001145        0.003588        0.001832
0.001221        0.003588        0.001527
Offsets set: 0.00 X, -1.00 Y, 0.00 Z
0.001450        1.003589        0.001603
0.001527        1.003513        0.001679
0.001374        1.003360        0.001603
0.001374        1.003665        0.001450
0.001527        1.003513        0.001450
0.001298        1.003742        0.001374
0.001527        1.003589        0.001450
0.001374        1.003589        0.001450
0.001527        1.003513        0.001756
0.001527        1.003513        0.001527
WHOAMI: 6C
Changing FSR of accelerometer to +/-32g
New measurement values:
0.059820        -0.056231       0.189034
0.133999        -0.076570       0.258426
0.133999        -0.057428       0.267998
0.114856        -0.086142       0.248855
0.105285        -0.066999       0.267998
0.105285        -0.076570       0.258426
0.143570        -0.105285       0.277569
0.133999        -0.076570       0.267998
0.114856        -0.086142       0.258426
0.124427        -0.086142       0.277569

Here is the output of uorb_listener in polling mode (there is also a magnetometer on this board, only the accel and gyro readings are from this driver).

object_name:sensor_mag, object_instance:0
object_name:sensor_gyro, object_instance:0
object_name:sensor_accel, object_instance:0
sensor_mag(now:80440000):timestamp:80440000,x:-72.900002,y:0.150000,z:-242.700012,temperature:23.101563,s0
sensor_gyro(now:80450000):timestamp:80440000,x:-0.000076,y:0.000000,z:-0.000076,temperature:23.000000
sensor_accel(now:80450000):timestamp:80450000,x:0.041874,y:-0.029910,z:-0.010767,temperature:23.000000
sensor_mag(now:80470000):timestamp:80470000,x:-74.099998,y:0.450000,z:-241.800003,temperature:23.070313,s0
sensor_gyro(now:80470000):timestamp:80470000,x:-0.050701,y:-0.017944,z:0.028863,temperature:24.000000
sensor_accel(now:80480000):timestamp:80480000,x:0.011964,y:-0.002392,z:-0.297908,temperature:24.000000
sensor_mag(now:80500000):timestamp:80500000,x:-74.250000,y:-0.750000,z:-242.700012,temperature:23.066406,0
sensor_gyro(now:80500000):timestamp:80500000,x:-0.031994,y:-0.001985,z:-0.008704,temperature:24.000000
sensor_accel(now:80510000):timestamp:80510000,x:0.014357,y:-0.017946,z:0.025124,temperature:24.000000
sensor_mag(now:80530000):timestamp:80530000,x:-73.800003,y:0.450000,z:-243.449997,temperature:23.078125,s0
Object name:sensor_mag0, recieved:4
Object name:sensor_gyro0, recieved:3
Object name:sensor_accel0, recieved:3
Total number of received Message:10/10

Here is the output of uorb_listener in interrupt-driven mode (there is also a magnetometer and barometer on this board, only the accel and gyro readings are from this driver).

nsh> uorb_listener -r 50 -n 10

Mointor objects num:4
object_name:sensor_mag, object_instance:0
object_name:sensor_gyro, object_instance:0
object_name:sensor_baro, object_instance:0
object_name:sensor_accel, object_instance:0
sensor_mag(now:54820000):timestamp:54810000,x:-77.100006,y:0.150000,z:-243.600006,temperature:22.992188,s0
sensor_mag(now:54830000):timestamp:54830000,x:-75.599998,y:-0.150000,z:-243.300003,temperature:23.031250,0
sensor_gyro(now:54830000):timestamp:54820000,x:-0.039400,y:-0.034513,z:0.038331,temperature:23.000000
sensor_accel(now:54830000):timestamp:54820000,x:-0.007178,y:-0.093320,z:2.392835,temperature:23.000000
sensor_gyro(now:54840000):timestamp:54840000,x:-0.027107,y:-0.011377,z:-0.004886,temperature:24.000000
sensor_accel(now:54840000):timestamp:54840000,x:0.009571,y:-0.458228,z:9.941032,temperature:24.000000
sensor_baro(now:54850000):timestamp:54850000,pressure:505.600006,temperature:18.510000
sensor_mag(now:54850000):timestamp:54850000,x:-75.300003,y:0.600000,z:-243.300003,temperature:23.039063,s0
sensor_gyro(now:54860000):timestamp:54860000,x:0.126449,y:-0.159665,z:-0.030008,temperature:24.000000
sensor_accel(now:54860000):timestamp:54860000,x:0.039481,y:-0.513263,z:9.961371,temperature:24.000000
Object name:sensor_mag0, recieved:3
Object name:sensor_gyro0, recieved:3
Object name:sensor_baro0, recieved:1
Object name:sensor_accel0, recieved:3
Total number of received Message:10/10

Here are the system logs in debug mode when the driver is registered in polling mode:

lsm6dso32_register: LSM6DSO32 gyro using polling thread.
lsm6dso32_register: LSM6DSO32 accel using polling thread.
lsm6dso32_register: LSM6DSO32 driver registered!

Here are the system logs in debug mode when the driver is registered in interrupt-driven mode:

lis2mdl_register: Registered LIS2MDL driver in interrupt driven mode.
lis2mdl_register: Registered LIS2MDL driver.
sensor_custom_register: Registering /dev/uorb/sensor_gyro0
sensor_custom_register: Registering /dev/uorb/sensor_accel0
lsm6dso32_register: LSM6DSO32 gyro interrupt handler attached to INT1.
lsm6dso32_register: LSM6DSO32 accel interrupt handler attached to INT2.
lsm6dso32_register: LSM6DSO32 driver registered!

Documentation was built and rendered locally and verified for correct formatting.

@github-actions github-actions bot added Area: Documentation Improvements or additions to documentation Area: Sensors Sensors issues Size: XL The size of the change in this PR is very large. Consider breaking down the PR into smaller pieces. labels Feb 8, 2025
@nuttxpr
Copy link

nuttxpr commented Feb 8, 2025

[Experimental Bot, please feedback here]

Yes, this PR substantially meets the NuttX requirements.

The summary clearly explains the change, its scope (limited support), and mentions accompanying documentation. The impact section identifies affected parts (build system, kernel) and correctly notes the addition of a new driver as a feature.

The testing section is comprehensive, detailing the host and target environments, providing the test script, expected behavior, observed output in different modes (interrupt/polling), and even includes relevant system logs. This thorough testing significantly strengthens the PR.

Minor suggestions for improvement:

  • Explicitly state "YES" or "NO" for each Impact item: Even though the general impact is described, going through each specific point (user impact, build impact, etc.) and marking them with YES/NO improves clarity and ensures nothing is overlooked. For instance, explicitly stating "Impact on user: YES (Users can now use the LSM6DSO32 IMU via uORB)" would be beneficial. For any "YES" responses, provide concise descriptions as the template requests.
  • Conciseness in Testing: While comprehensive testing is good, the testing logs could be trimmed down. Focus on key parts that demonstrate functionality and address potential issues, rather than including the entire output. Summaries and specific excerpts would be more effective. For example, showing the change in full-scale range and a few measurements before/after would suffice.
  • NuttX Issue Reference: If this PR addresses a specific issue in the NuttX repository, including a link would be helpful for tracking.

By addressing these minor points, the PR will even more closely adhere to the NuttX requirements and be easier for reviewers to assess.

drivers/sensors/Kconfig Outdated Show resolved Hide resolved
drivers/sensors/Kconfig Outdated Show resolved Hide resolved
drivers/sensors/Kconfig Outdated Show resolved Hide resolved
drivers/sensors/lsm6dso32_uorb.c Outdated Show resolved Hide resolved
drivers/sensors/lsm6dso32_uorb.c Outdated Show resolved Hide resolved
drivers/sensors/lsm6dso32_uorb.c Show resolved Hide resolved
drivers/sensors/lsm6dso32_uorb.c Outdated Show resolved Hide resolved

/* If we got here, there was no error, we can record the activation state */

sens->enabled = enable;
Copy link
Contributor

Choose a reason for hiding this comment

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

why not use interrupt ways at all times?

Copy link
Contributor Author

@linguini1 linguini1 Feb 8, 2025

Choose a reason for hiding this comment

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

I'm not sure what you mean, as opposed to polling from a thread? This allows flexibility for users to choose to poll.

include/nuttx/sensors/lsm6dso32.h Outdated Show resolved Hide resolved
drivers/sensors/lsm6dso32_uorb.c Outdated Show resolved Hide resolved
Adds support for the LSM6DSO32 IMU by STM using the uORB framework. This
only contains limited support for the I2C interface, interrupt and
polling driven measurement and standard modes of operation (high
performance ODRs). Features like interrupts besides DRDY, filtering,
gesture recognition and acting as a bus master are not implemented.
Copy link
Contributor

@jerpelea jerpelea left a comment

Choose a reason for hiding this comment

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

please move the documentation to a new PR for PR

@linguini1
Copy link
Contributor Author

please move the documentation to a new PR for PR

May I ask why? Wouldn't it be best to merge the documentation about this new feature at the same time that it's merged?

@jerpelea
Copy link
Contributor

For a LTS release we will rebase commits for several years and there is a high chance that documentation or other areas will need changes. I think that having separate PR with same name but numbers at the end {1/2}, {2/2} will make the rebase and conflict resolution easy.

Best regards
Alin

@linguini1
Copy link
Contributor Author

linguini1 commented Feb 10, 2025

For a LTS release we will rebase commits for several years and there is a high chance that documentation or other areas will need changes. I think that having separate PR with same name but numbers at the end {1/2}, {2/2} will make the rebase and conflict resolution easy.

From my perspective this will make it more difficult to ensure that contributors are putting forward documentation with their PRs, and it adds more work for contributors and reviewers to look between two PRs and have to update two PRs when changes are made.

For this PR I've already had to update the docs to match changes I made based on the reviews. If we introduce dual PRs I'll have to swap between two branches to make changes and push to two branches. It makes more sense to me to have the docs be combined with the changes being made.

If this is going to be a hard requirement from NuttX I will split the PR, but this should be mentioned in the contribution guidelines as well.

@acassis
Copy link
Contributor

acassis commented Feb 10, 2025

@jerpelea I think this "enforcement" doesn't exist in others more "stable" projects with LTS, see this example:
zephyrproject-rtos/zephyr@5c629f2

Board support and documentation in the same PR (and he is not a random contributor)

@jerpelea
Copy link
Contributor

@acassis during the 12.8.0 stabilization period there were some conflicts that made me ask for this change. If we plan to maintain the LTS release we should make it manageable. Unless we have PR and commits affecting only 1 area they will create "hidden" requirements and add "hidden" changes which will impact future PRs affecting the same area. Please remember that we are talking about 2400+ commits / year that must be checked and ported for LTS

@xiaoxiang781216
Copy link
Contributor

@acassis during the 12.8.0 stabilization period there were some conflicts that made me ask for this change. If we plan to maintain the LTS release we should make it manageable. Unless we have PR and commits affecting only 1 area they will create "hidden" requirements and add "hidden" changes which will impact future PRs affecting the same area. Please remember that we are talking about 2400+ commits / year that must be checked and ported for LTS

LTS should avoid backport the change except the bug fix report by end user as much as possible, I think.

@acassis
Copy link
Contributor

acassis commented Feb 11, 2025

@jerpelea @xiaoxiang781216 maybe it is worth to investigate how other projects that have LTS are solving it. Forcing our (few) contributors to create a single PR for each modified are seems no feasible.

@raiden00pl
Copy link
Member

@jerpelea If we don't have the resources to deal with LTS, why do it at all? making life difficult for (few) contributors is a tragic solution. Another issue is that some changes must affect more than one area (eg. arch + boards), otherwise CI will be useless.

@acassis acassis merged commit 729f8a7 into apache:master Feb 12, 2025
40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Documentation Improvements or additions to documentation Area: Sensors Sensors issues Size: XL The size of the change in this PR is very large. Consider breaking down the PR into smaller pieces.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants