pbio/control_settings: Validate speed, acceleration.

Ensures limit setters respect numerical bounds imposed by the trajectory math.

Properly fixes pybricks/support#484.

CHANGELOG.md

@@ -7,7 +7,14 @@
 ### Fixed
 - Fixed allocator interfering with motor control when memory usage is high ([support#956]).
 
-[support#956]: https://github.com/pybricks/support/issues/977
+### Changed
+- Methods like `control.limits()` now check the user input and raise a
+  `ValueError` if a value is out of bounds  ([support#484]). This affects only
+  settings setters, which are usually used as a one-off. Nothing changes to
+  speed values set at runtime. These are still capped to valid numbers without
+  raising exceptions.
+
+[support#484]: https://github.com/pybricks/support/issues/484
 
 ## [3.3.0b2] - 2023-03-08
 

lib/pbio/include/pbio/trajectory.h

@@ -79,6 +79,8 @@ typedef struct _pbio_trajectory_t {
 
 // Make or modify trajectories:
 
+pbio_error_t pbio_trajectory_validate_speed_limit(int32_t ctl_steps_per_app_step, int32_t speed);
+pbio_error_t pbio_trajectory_validate_acceleration_limit(int32_t ctl_steps_per_app_step, int32_t acceleration);
 pbio_error_t pbio_trajectory_new_angle_command(pbio_trajectory_t *trj, const pbio_trajectory_command_t *command);
 pbio_error_t pbio_trajectory_new_time_command(pbio_trajectory_t *trj, const pbio_trajectory_command_t *command);
 void pbio_trajectory_make_constant(pbio_trajectory_t *trj, const pbio_trajectory_command_t *command);

lib/pbio/src/control_settings.c

@@ -162,8 +162,18 @@ void pbio_control_settings_get_trajectory_limits(const pbio_control_settings_t *
  *                            ::PBIO_ERROR_INVALID_ARG if any argument is negative.
  */
 pbio_error_t pbio_control_settings_set_trajectory_limits(pbio_control_settings_t *s, int32_t speed, int32_t acceleration, int32_t deceleration) {
-    if (speed < 1 || acceleration < 1 || deceleration < 1) {
-        return PBIO_ERROR_INVALID_ARG;
+    // Validate that all inputs are within allowed bounds.
+    pbio_error_t err = pbio_trajectory_validate_speed_limit(s->ctl_steps_per_app_step, speed);
+    if (err != PBIO_SUCCESS) {
+        return err;
+    }
+    err = pbio_trajectory_validate_acceleration_limit(s->ctl_steps_per_app_step, acceleration);
+    if (err != PBIO_SUCCESS) {
+        return err;
+    }
+    err = pbio_trajectory_validate_acceleration_limit(s->ctl_steps_per_app_step, deceleration);
+    if (err != PBIO_SUCCESS) {
+        return err;
     }
     s->speed_max = pbio_control_settings_app_to_ctl(s, speed);
     s->acceleration = pbio_control_settings_app_to_ctl(s, acceleration);
@@ -233,9 +243,14 @@ void pbio_control_settings_get_pid(const pbio_control_settings_t *s, int32_t *pi
  *                                   ::PBIO_ERROR_INVALID_ARG if any argument is negative.
  */
 pbio_error_t pbio_control_settings_set_pid(pbio_control_settings_t *s, int32_t pid_kp, int32_t pid_ki, int32_t pid_kd, int32_t integral_deadzone, int32_t integral_change_max) {
-    if (pid_kp < 0 || pid_ki < 0 || pid_kd < 0 || integral_change_max < 0 || integral_deadzone < 0) {
+    if (pid_kp < 0 || pid_ki < 0 || pid_kd < 0 || integral_deadzone < 0) {
         return PBIO_ERROR_INVALID_ARG;
     }
+    // integral_change_max has physical units of speed, so must satisfy bound.
+    pbio_error_t err = pbio_trajectory_validate_speed_limit(s->ctl_steps_per_app_step, integral_change_max);
+    if (err != PBIO_SUCCESS) {
+        return err;
+    }
 
     s->pid_kp = pid_kp;
     s->pid_ki = pid_ki;
@@ -266,9 +281,13 @@ void pbio_control_settings_get_target_tolerances(const pbio_control_settings_t *
  *                          ::PBIO_ERROR_INVALID_ARG if any argument is negative.
  */
 pbio_error_t pbio_control_settings_set_target_tolerances(pbio_control_settings_t *s, int32_t speed, int32_t position) {
-    if (position < 0 || speed < 0) {
+    if (position < 0) {
         return PBIO_ERROR_INVALID_ARG;
     }
+    pbio_error_t err = pbio_trajectory_validate_speed_limit(s->ctl_steps_per_app_step, speed);
+    if (err != PBIO_SUCCESS) {
+        return err;
+    }
 
     s->position_tolerance = pbio_control_settings_app_to_ctl(s, position);
     s->speed_tolerance = pbio_control_settings_app_to_ctl(s, speed);
@@ -297,8 +316,9 @@ void pbio_control_settings_get_stall_tolerances(const pbio_control_settings_t *s
  *                          ::PBIO_ERROR_INVALID_ARG if any argument is negative.
  */
 pbio_error_t pbio_control_settings_set_stall_tolerances(pbio_control_settings_t *s, int32_t speed, uint32_t time) {
-    if (speed < 0) {
-        return PBIO_ERROR_INVALID_ARG;
+    pbio_error_t err = pbio_trajectory_validate_speed_limit(s->ctl_steps_per_app_step, speed);
+    if (err != PBIO_SUCCESS) {
+        return err;
     }
 
     s->stall_speed_limit = pbio_control_settings_app_to_ctl(s, speed);

lib/pbio/src/trajectory.c

@@ -119,6 +119,43 @@ static int32_t to_trajectory_accel(int32_t control_accel) {
  */
 #define TO_CONTROL_TIME(time) ((uint32_t)(time))
 
+/**
+ * Validates that the given speed limit is within the numerically allowed range.
+ *
+ * @param [in] ctl_steps_per_app_step   Control units per application unit.
+ * @param [in] speed                    Speed limit (application units, positive).
+ * @return                              ::PBIO_SUCCESS on valid values.
+ *                                      ::PBIO_ERROR_INVALID_ARG if any argument is outside the allowed range.
+ */
+pbio_error_t pbio_trajectory_validate_speed_limit(int32_t ctl_steps_per_app_step, int32_t speed) {
+
+    // Speeds can be negative but speed *limit* must be a strictly positive value.
+    const int32_t speed_min = to_control_speed(100) / ctl_steps_per_app_step;
+    const int32_t speed_max = to_control_speed(SPEED_MAX) / ctl_steps_per_app_step;
+
+    if (speed < speed_min || speed > speed_max) {
+        return PBIO_ERROR_INVALID_ARG;
+    }
+    return PBIO_SUCCESS;
+}
+
+/**
+ * Validates that the given acceleration is within the numerically allowed range.
+ *
+ * @param [in] ctl_steps_per_app_step   Control units per application unit.
+ * @param [in] acceleration             Acceleration (application units, positive).
+ * @return                              ::PBIO_SUCCESS on valid values.
+ *                                      ::PBIO_ERROR_INVALID_ARG if any argument is outside the allowed range.
+ */
+pbio_error_t pbio_trajectory_validate_acceleration_limit(int32_t ctl_steps_per_app_step, int32_t acceleration) {
+    const int32_t accel_min = to_control_accel(ACCELERATION_MIN) / ctl_steps_per_app_step;
+    const int32_t accel_max = to_control_accel(ACCELERATION_MAX) / ctl_steps_per_app_step;
+    if (acceleration < accel_min || acceleration > accel_max) {
+        return PBIO_ERROR_INVALID_ARG;
+    }
+    return PBIO_SUCCESS;
+}
+
 /**
  * Reverses a trajectory.
  *