[pidcontroller] Remove limits, make Ki dependent from the loop time, add reset command #9759
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…he loop time Also fix naming of thread and shutdown executor. Signed-off-by: Fabian Wolter <github@fabian-wolter.de>
J-N-K
reviewed
Jan 9, 2021
| | `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 | |
There was a problem hiding this comment.
What is the benefit of renaming? I guess that outputLowerLimit is more clear
There was a problem hiding this comment.
The parameter does not limit the output anymore, but only the integral part.
J-N-K
reviewed
Jan 9, 2021
| @@ -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); | |||
There was a problem hiding this comment.
But why is this names outputLowerLimit then?
There was a problem hiding this comment.
I overlooked that. Thanks!
|
Also I think a limit for the output is useful. |
Signed-off-by: Fabian Wolter <github@fabian-wolter.de>
|
The opinions differ. A control expert stated in the forum that even a limit on the I part is hacked. A limit to the output could be applied by the user via a rule. |
|
Reading through the thread having a "reset" channel would be better than the limitiation. Why don't we implement that one? |
Signed-off-by: Fabian Wolter <github@fabian-wolter.de>
fwolter
changed the title
|
I removed the limits and added a reset command. |
wborn
added
rebuild
J-N-K
approved these changes
Jan 12, 2021
themillhousegroup
pushed a commit
to themillhousegroup/openhab2-addons
that referenced
this pull request
May 10, 2021
…add reset command (openhab#9759) * [pidcontroller] Remove limits, make Ki dependent from the loop time Also fix naming of thread and shutdown executor. Signed-off-by: Fabian Wolter <github@fabian-wolter.de> Signed-off-by: John Marshall <john.marshall.au@gmail.com>
thinkingstone
pushed a commit
to thinkingstone/openhab-addons
that referenced
this pull request
Nov 7, 2021
…add reset command (openhab#9759) * [pidcontroller] Remove limits, make Ki dependent from the loop time Also fix naming of thread and shutdown executor. Signed-off-by: Fabian Wolter <github@fabian-wolter.de>
marcfischerboschio
pushed a commit
to bosch-io/openhab-addons
that referenced
this pull request
May 5, 2022
…add reset command (openhab#9759) * [pidcontroller] Remove limits, make Ki dependent from the loop time Also fix naming of thread and shutdown executor. Signed-off-by: Fabian Wolter <github@fabian-wolter.de>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Also fix naming of thread and shutdown executor.
Signed-off-by: Fabian Wolter github@fabian-wolter.de