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.

Sign up for GitHub

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

[pidcontroller] Remove limits, make Ki dependent from the loop time, add reset command #9759

Merged
merged 3 commits into from
Jan 12, 2021
Merged

[pidcontroller] Remove limits, make Ki dependent from the loop time, add reset command #9759

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e7375fb
[pidcontroller] Apply limits only to I-part, make Ki dependent from t…
fwolter Jan 6, 2021
63acf61
Fix naming of limit variables
fwolter Jan 9, 2021
fb83b4c
Remove limits and add reset command
fwolter Jan 10, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 16 additions & 15 deletions bundles/org.openhab.automation.pidcontroller/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,20 +22,21 @@ This module triggers whenever the `input` or the `setpoint` changes or the `loop
Every trigger calculates the P, the I and the D part and sums them up to form the `output` value.
This is then transferred to the action module.

| Name | Type | Description | Required |
|--------------------|---------|----------------------------------------------------------------------------------------------------------------------------------------------------|----------|
| `input` | Item | Name of the input [Item](https://www.openhab.org/docs/configuration/items.html) (e.g. temperature sensor value) | Y |
| `setpoint` | Item | Name of the setpoint Item (e.g. desired room temperature) | Y |
| `kp` | Decimal | P: [Proportional Gain](#proportional-p-gain-parameter) Parameter | Y |
| `ki` | Decimal | I: [Integral Gain](#integral-i-gain-parameter) Parameter | Y |
| `kd` | Decimal | D: [Derivative Gain](#derivative-d-gain-parameter) Parameter | Y |
| `kdTimeConstant` | Decimal | D-T1: [Derivative Gain Time Constant](#derivative-time-constant-d-t1-parameter) in sec. | Y |
| `outputLowerLimit` | Decimal | The output of the PID controller will be max this value | Y |
| `outputUpperLimit` | Decimal | The output of the PID controller will be min this value | Y |
| `loopTime` | Decimal | The interval the output value will be updated in milliseconds. Note: the output will also be updated when the input value or the setpoint changes. | Y |

The purpose of the limit parameters are to keep the output value and the integral value in a reasonable range, if the regulation cannot meet its setpoint.
| Name | Type | Description | Required |
|----------------------|---------|----------------------------------------------------------------------------------------------------------------------------------------------------|----------|
| `input` | Item | Name of the input [Item](https://www.openhab.org/docs/configuration/items.html) (e.g. temperature sensor value) | Y |
| `setpoint` | Item | Name of the setpoint Item (e.g. desired room temperature) | Y |
| `kp` | Decimal | P: [Proportional Gain](#proportional-p-gain-parameter) Parameter | Y |
| `ki` | Decimal | I: [Integral Gain](#integral-i-gain-parameter) Parameter | Y |
| `kd` | Decimal | D: [Derivative Gain](#derivative-d-gain-parameter) Parameter | Y |
| `kdTimeConstant` | Decimal | D-T1: [Derivative Gain Time Constant](#derivative-time-constant-d-t1-parameter) in sec. | Y |
| `integralLowerLimit` | Decimal | The I part of the PID controller will be max this value | Y |
Copy link
Member

Choose a reason for hiding this comment

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

What is the benefit of renaming? I guess that outputLowerLimit is more clear

Copy link
Member Author

Choose a reason for hiding this comment

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

The parameter does not limit the output anymore, but only the integral part.

| `integralUpperLimit` | Decimal | The I part of the PID controller will be min this value | Y |
| `loopTime` | Decimal | The interval the output value will be updated in milliseconds. Note: the output will also be updated when the input value or the setpoint changes. | Y |

The purpose of the limit parameters are to keep the integral value in a reasonable range, if the regulation cannot meet its setpoint.
E.g. the window is open and the heater doesn't manage to heat up the room.
If you control the opening of a valve, this should be 0% and 100% for example.

The `loopTime` should be max a tenth of the system response.
E.g. the heating needs 10 min to heat up the room, the loop time should be max 1 min.
Expand Down Expand Up @@ -76,8 +77,8 @@ The bigger this parameter, the faster the drifting.

A value of 0 disables the I part.

A value of 1 adds the current setpoint deviation (error) to the output each second.
E.g. the setpoint is 25°C and the measured value is 20°C, the output will be set to 5 after 1 sec.
A value of 1 adds the current setpoint deviation (error) to the output each `loopTime` (in milliseconds).
E.g. (`loopTimeMs=1000`) the setpoint is 25°C and the measured value is 20°C, the output will be set to 5 after 1 sec.
After 2 sec the output will be 10.
If the output is the opening of a valve in %, you might want to set this parameter to a lower value (`ki=0.1` would result in 30% after 60 sec: 5\*0.1\*60=30).

Expand Down
4 changes: 2 additions & 2 deletions ...r/src/main/java/org/openhab/automation/pidcontroller/internal/PIDControllerConstants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ public class PIDControllerConstants {
public static final String AUTOMATION_NAME = "pidcontroller";
public static final String CONFIG_INPUT_ITEM = "input";
public static final String CONFIG_SETPOINT_ITEM = "setpoint";
public static final String CONFIG_OUTPUT_LOWER_LIMIT = "outputLowerLimit";
public static final String CONFIG_OUTPUT_UPPER_LIMIT = "outputUpperLimit";
public static final String CONFIG_INTEGRAL_LOWER_LIMIT = "integralLowerLimit";
public static final String CONFIG_INTEGRAL_UPPER_LIMIT = "integralUpperLimit";
public static final String CONFIG_LOOP_TIME = "loopTime";
public static final String CONFIG_KP_GAIN = "kp";
public static final String CONFIG_KI_GAIN = "ki";
Expand Down
21 changes: 9 additions & 12 deletions ...er/src/main/java/org/openhab/automation/pidcontroller/internal/handler/PIDController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@
*/
@NonNullByDefault
class PIDController {
private final double outputLowerLimit;
private final double outputUpperLimit;
private final double integralLowerLimit;
private final double integralUpperLimit;

private double integralResult;
private double derivativeResult;
Expand All @@ -38,17 +38,17 @@ class PIDController {
private double kd;
private double derivativeTimeConstantSec;

public PIDController(double outputLowerLimit, double outputUpperLimit, double kpAdjuster, double kiAdjuster,
public PIDController(double integralLowerLimit, double integralUpperLimit, double kpAdjuster, double kiAdjuster,
double kdAdjuster, double derivativeTimeConstantSec) {
this.outputLowerLimit = outputLowerLimit;
this.outputUpperLimit = outputUpperLimit;
this.integralLowerLimit = integralLowerLimit;
this.integralUpperLimit = integralUpperLimit;
this.kp = kpAdjuster;
this.ki = kiAdjuster;
this.kd = kdAdjuster;
this.derivativeTimeConstantSec = derivativeTimeConstantSec;
}

public PIDOutputDTO calculate(double input, double setpoint, long lastInvocationMs) {
public PIDOutputDTO calculate(double input, double setpoint, long lastInvocationMs, int loopTimeMs) {
final double lastInvocationSec = lastInvocationMs / 1000d;
final double error = setpoint - input;

Expand All @@ -60,11 +60,11 @@ public PIDOutputDTO calculate(double input, double setpoint, long lastInvocation
}

// integral calculation
integralResult += error * lastInvocationSec;
integralResult += error * lastInvocationMs / loopTimeMs;
// limit to output limits
if (ki != 0) {
final double maxIntegral = outputUpperLimit / ki;
final double minIntegral = outputLowerLimit / ki;
final double maxIntegral = integralUpperLimit / ki;
final double minIntegral = integralLowerLimit / ki;
integralResult = Math.min(maxIntegral, Math.max(minIntegral, integralResult));
}

Expand All @@ -74,9 +74,6 @@ public PIDOutputDTO calculate(double input, double setpoint, long lastInvocation
final double derivativePart = kd * derivativeResult;
output = proportionalPart + integralPart + derivativePart;

// limit output value
output = Math.min(outputUpperLimit, Math.max(outputLowerLimit, output));

return new PIDOutputDTO(output, proportionalPart, integralPart, derivativePart, error);
}
}
14 changes: 10 additions & 4 deletions ...va/org/openhab/automation/pidcontroller/internal/handler/PIDControllerTriggerHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public class PIDControllerTriggerHandler extends BaseTriggerModuleHandler implem
private static final Set<String> SUBSCRIBED_EVENT_TYPES = Set.of(ItemStateEvent.TYPE, ItemStateChangedEvent.TYPE);
private final Logger logger = LoggerFactory.getLogger(PIDControllerTriggerHandler.class);
private final ScheduledExecutorService scheduler = Executors
.newSingleThreadScheduledExecutor(new NamedThreadFactory("OH-automation-" + AUTOMATION_NAME, true));
.newSingleThreadScheduledExecutor(new NamedThreadFactory("automation-" + AUTOMATION_NAME, true));
private final ServiceRegistration<?> eventSubscriberRegistration;
private final PIDController controller;
private final int loopTimeMs;
Expand Down Expand Up @@ -93,13 +93,17 @@ public PIDControllerTriggerHandler(Trigger module, ItemRegistry itemRegistry, Ev
throw new IllegalArgumentException("Configured setpoint item not found: " + setpointItemName, e);
}

double outputLowerLimit = getDoubleFromConfig(config, CONFIG_OUTPUT_LOWER_LIMIT);
double outputUpperLimit = getDoubleFromConfig(config, CONFIG_OUTPUT_UPPER_LIMIT);
double outputLowerLimit = getDoubleFromConfig(config, CONFIG_INTEGRAL_LOWER_LIMIT);
Copy link
Member

Choose a reason for hiding this comment

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

But why is this names outputLowerLimit then?

Copy link
Member Author

Choose a reason for hiding this comment

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

I overlooked that. Thanks!

double outputUpperLimit = getDoubleFromConfig(config, CONFIG_INTEGRAL_UPPER_LIMIT);
double kpAdjuster = getDoubleFromConfig(config, CONFIG_KP_GAIN);
double kiAdjuster = getDoubleFromConfig(config, CONFIG_KI_GAIN);
double kdAdjuster = getDoubleFromConfig(config, CONFIG_KD_GAIN);
double kdTimeConstant = getDoubleFromConfig(config, CONFIG_KD_TIMECONSTANT);

if (outputLowerLimit >= outputUpperLimit) {
throw new IllegalArgumentException("Lower integral limit is bigger or equal to the upper limit");
}

loopTimeMs = ((BigDecimal) requireNonNull(config.get(CONFIG_LOOP_TIME), CONFIG_LOOP_TIME + " is not set"))
.intValue();

Expand Down Expand Up @@ -152,7 +156,7 @@ private void calculate() {

long now = System.currentTimeMillis();

PIDOutputDTO output = controller.calculate(input, setpoint, now - previousTimeMs);
PIDOutputDTO output = controller.calculate(input, setpoint, now - previousTimeMs, loopTimeMs);
previousTimeMs = now;

Map<String, BigDecimal> outputs = new HashMap<>();
Expand Down Expand Up @@ -221,6 +225,8 @@ public void dispose() {
localControllerjob.cancel(true);
}

scheduler.shutdown();

super.dispose();
}
}
27 changes: 16 additions & 11 deletions ...ain/java/org/openhab/automation/pidcontroller/internal/type/PIDControllerTriggerType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,23 +44,28 @@ public static PIDControllerTriggerType initialize() {
.withRequired(true).withReadOnly(true).withMultiple(false).withContext("item").withLabel("Setpoint")
.withDescription("Targeted setpoint").build());
configDescriptions.add(ConfigDescriptionParameterBuilder.create(CONFIG_KP_GAIN, Type.DECIMAL).withRequired(true)
.withMultiple(false).withDefault("1.0").withLabel("Proportional Gain (Kp)")
.withMultiple(false).withDefault("1.0").withMinimum(BigDecimal.ZERO).withLabel("Proportional Gain (Kp)")
.withDescription("Change to output propertional to current error value.").build());
configDescriptions.add(ConfigDescriptionParameterBuilder.create(CONFIG_KI_GAIN, Type.DECIMAL).withRequired(true)
.withMultiple(false).withDefault("1.0").withLabel("Integral Gain (Ki)")
.withMultiple(false).withDefault("1.0").withMinimum(BigDecimal.ZERO).withLabel("Integral Gain (Ki)")
.withDescription("Accelerate movement towards the setpoint.").build());
configDescriptions.add(ConfigDescriptionParameterBuilder.create(CONFIG_KD_GAIN, Type.DECIMAL).withRequired(true)
.withMultiple(false).withDefault("1.0").withLabel("Derivative Gain (Kd)")
.withMultiple(false).withDefault("1.0").withMinimum(BigDecimal.ZERO).withLabel("Derivative Gain (Kd)")
.withDescription("Slows the rate of change of the output.").build());
configDescriptions.add(ConfigDescriptionParameterBuilder.create(CONFIG_KD_TIMECONSTANT, Type.DECIMAL)
.withRequired(true).withMultiple(false).withDefault("1.0").withLabel("Derivative Time Constant")
.withDescription("Slows the rate of change of the D Part (T1) in seconds.").withUnit("s").build());
configDescriptions.add(ConfigDescriptionParameterBuilder.create(CONFIG_OUTPUT_LOWER_LIMIT, Type.DECIMAL)
.withRequired(true).withMultiple(false).withDefault("0").withLabel("Output Lower Limit")
.withDescription("The output of the PID controller will be min this value").build());
configDescriptions.add(ConfigDescriptionParameterBuilder.create(CONFIG_OUTPUT_UPPER_LIMIT, Type.DECIMAL)
.withRequired(true).withMultiple(false).withDefault("100").withLabel("Output Upper Limit")
.withDescription("The output of the PID controller will be max this value").build());
.withRequired(true).withMultiple(false).withMinimum(BigDecimal.ZERO).withDefault("1.0")
.withLabel("Derivative Time Constant")
.withDescription("Slows the rate of change of the D part (T1) in seconds.").withUnit("s").build());
configDescriptions.add(ConfigDescriptionParameterBuilder.create(CONFIG_INTEGRAL_LOWER_LIMIT, Type.DECIMAL)
.withRequired(true).withMultiple(false).withDefault("0").withLabel("I-Part Lower Limit")
.withDescription(
"The I part of the PID controller will be min this value. If you control the opening of a valve in %, you might want to set this to 0 for example.")
.build());
configDescriptions.add(ConfigDescriptionParameterBuilder.create(CONFIG_INTEGRAL_UPPER_LIMIT, Type.DECIMAL)
.withRequired(true).withMultiple(false).withDefault("100").withLabel("I-Part Upper Limit")
.withDescription(
"The I part of the PID controller will be max this value. If you control the opening of a valve in %, you might want to set this to 100 for example.")
.build());
configDescriptions.add(ConfigDescriptionParameterBuilder.create(CONFIG_LOOP_TIME, Type.DECIMAL)
.withRequired(true).withMultiple(false).withDefault(DEFAULT_LOOPTIME_MS).withLabel("Loop Time")
.withDescription("The interval the output value is updated in ms").withUnit("ms").build());
Expand Down