Skip to content

Commit

Permalink
pybricks.common.Control: Restore arg order.
Browse files Browse the repository at this point in the history
The integral_range argument was dropped in
3f0042f.

While this setting was virtually never used, the pid()
method is used in some example projects. Typically, all 5 parameters
are unpacked to easily read the existing PID parameters. Removing one
argument would break these projects.

This commit restores the order and number of arguments to avoid a
breaking change. The setting may be reserved for future use instead.
  • Loading branch information
laurensvalk committed Sep 28, 2021
1 parent 11ea2f4 commit e82dd6a
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 3 deletions.
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,15 @@

## [Unreleased]

### Changed:
- Dropped `integral_range` argument from `Control.pid()`. This setting was
ineffective and never used. When set incorrectly, the motor could get stuck
for certain combinations of `kp` and `ki`.
- Improved motor behavior for cases with low-speed, low-load, but high
inertia. ([support#366])

[support#366]: https://github.com/pybricks/support/issues/366

## [3.1.0b1] - 2021-08-21

### Added
Expand Down
9 changes: 6 additions & 3 deletions pybricks/common/pb_type_control.c
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ STATIC mp_obj_t common_Control_pid(size_t n_args, const mp_obj_t *pos_args, mp_m
PB_ARG_DEFAULT_NONE(kp),
PB_ARG_DEFAULT_NONE(ki),
PB_ARG_DEFAULT_NONE(kd),
PB_ARG_DEFAULT_NONE(reserved),
PB_ARG_DEFAULT_NONE(integral_rate));

// Read current values
Expand All @@ -104,14 +105,16 @@ STATIC mp_obj_t common_Control_pid(size_t n_args, const mp_obj_t *pos_args, mp_m
pbio_control_settings_get_pid(&self->control->settings, &kp, &ki, &kd, &integral_rate);

// If all given values are none, return current values
(void)reserved_in;
if (kp_in == mp_const_none && ki_in == mp_const_none && kd_in == mp_const_none &&
integral_rate_in == mp_const_none) {
mp_obj_t ret[4];
mp_obj_t ret[5];
ret[0] = mp_obj_new_int(kp);
ret[1] = mp_obj_new_int(ki);
ret[2] = mp_obj_new_int(kd);
ret[3] = mp_obj_new_int(integral_rate);
return mp_obj_new_tuple(4, ret);
ret[3] = mp_const_none;
ret[4] = mp_obj_new_int(integral_rate);
return mp_obj_new_tuple(5, ret);
}

// Assert control is not active
Expand Down

0 comments on commit e82dd6a

Please sign in to comment.