M-Labs/thermostat
9
0
Fork 5
Code Issues 9 Pull Requests 7 Releases Wiki Activity

PyThermostat: Modify PIDAutotune for GUI usage #162

Open
atse wants to merge 3 commits from atse/thermostat:autotune-state into master
pull from: atse/thermostat:autotune-state
merge into: M-Labs:master
Conversation 0 Commits 3 Files Changed 1 +39 -22
1 changed files with 39 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

61
pythermostat/pythermostat/autotune.py
View File

@ -2,7 +2,7 @@ import math
import logging
import time
from collections import deque, namedtuple
from enum import Enum
from enum import Enum, auto
from pythermostat.client import Client
@ -13,11 +13,12 @@ from pythermostat.client import Client
class PIDAutotuneState(Enum):
STATE_OFF = 'off'
STATE_RELAY_STEP_UP = 'relay step up'
STATE_RELAY_STEP_DOWN = 'relay step down'
STATE_SUCCEEDED = 'succeeded'
STATE_FAILED = 'failed'
OFF = auto()
RELAY_STEP_UP = auto()
RELAY_STEP_DOWN = auto()
SUCCEEDED = auto()
FAILED = auto()
READY = auto()
sb10q commented 2025-01-15 21:49:11 +08:00
Outdated
Review
Copy Link

Inconsistent string delimiter.

Can't it just use auto() anyway?

Inconsistent string delimiter. Can't it just use ``auto()`` anyway?
atse commented 2025-01-20 10:56:13 +08:00
Outdated
Review
Copy Link

Actually yes, switched to auto() in force-push to 828648ed76.

Then I believe #71 would be unnecessary since the string values aren't used anywhere.

Actually yes, switched to `auto()` in force-push to 828648ed76. Then I believe #71 would be unnecessary since the string values aren't used anywhere.
atse commented 2025-01-20 11:28:21 +08:00
Outdated
Review
Copy Link

Also just realised this highlights the verbosity of the STATE_ prefix.

Also just realised this highlights the verbosity of the `STATE_` prefix.
atse commented 2025-01-20 11:59:02 +08:00
Outdated
Review
Copy Link

Refactored in force-push to 869922c928.

Refactored in force-push to 869922c928.
class PIDAutotune:
@ -45,7 +46,7 @@ class PIDAutotune:
self._noiseband = noiseband
self._out_min = -out_step
self._out_max = out_step
self._state = PIDAutotuneState.STATE_OFF
self._state = PIDAutotuneState.OFF
self._peak_timestamps = deque(maxlen=5)
self._peaks = deque(maxlen=5)
self._output = 0
@ -57,6 +58,21 @@ class PIDAutotune:
self._Ku = 0
self._Pu = 0
def set_param(self, target, step, noiseband, sampletime, lookback):
self._setpoint = target
self._outputstep = step
self._out_max = step
self._out_min = -step
self._noiseband = noiseband
self._inputs = deque(maxlen=round(lookback / sampletime))
def set_ready(self):
self._state = PIDAutotuneState.READY
self._peak_count = 0
def set_off(self):
self._state = PIDAutotuneState.OFF
def state(self):
"""Get the current state."""
return self._state
@ -94,29 +110,30 @@ class PIDAutotune:
"""
now = time_input * 1000
if (self._state == PIDAutotuneState.STATE_OFF
or self._state == PIDAutotuneState.STATE_SUCCEEDED
or self._state == PIDAutotuneState.STATE_FAILED):
self._state = PIDAutotuneState.STATE_RELAY_STEP_UP
if self._state not in {
esavkin commented 2025-01-14 10:28:18 +08:00
Outdated
Review
Copy Link

Isn't there a way to shorten it?

Isn't there a way to shorten it?
atse commented 2025-01-20 10:53:05 +08:00
Outdated
Review
Copy Link

Yep, did it in the latest force-push to 828648ed76.

Yep, did it in the latest force-push to 828648ed76.
👍 1
esavkin commented 2025-01-20 15:36:41 +08:00
Outdated
Review
Copy Link

Wouldn't the negative logic be applicable here? Like not in (STEP_UP, STEP_DOWN).

Wouldn't the negative logic be applicable here? Like `not in (STEP_UP, STEP_DOWN)`.
atse commented 2025-01-20 16:20:43 +08:00
Outdated
Review
Copy Link

Humm, that's true. Was worried about self._state being set to a non-PIDAutotuneState value, but that shouldn't happen under normal circumstances.

Humm, that's true. Was worried about `self._state` being set to a non-`PIDAutotuneState` value, but that shouldn't happen under normal circumstances.
atse commented 2025-01-20 16:30:47 +08:00
Outdated
Review
Copy Link

Done in force-push to f4c1dab00f.

Done in force-push to f4c1dab00f.
👍 1
PIDAutotuneState.RELAY_STEP_DOWN,
PIDAutotuneState.RELAY_STEP_UP,
}:
self._state = PIDAutotuneState.RELAY_STEP_UP
self._last_run_timestamp = now
# check input and change relay state if necessary
if (self._state == PIDAutotuneState.STATE_RELAY_STEP_UP
and input_val > self._setpoint + self._noiseband):
self._state = PIDAutotuneState.STATE_RELAY_STEP_DOWN
if self._state == PIDAutotuneState.RELAY_STEP_UP
and input_val > self._setpoint + self._noiseband:
self._state = PIDAutotuneState.RELAY_STEP_DOWN
logging.debug('switched state: {0}'.format(self._state))
logging.debug('input: {0}'.format(input_val))
elif (self._state == PIDAutotuneState.STATE_RELAY_STEP_DOWN
and input_val < self._setpoint - self._noiseband):
self._state = PIDAutotuneState.STATE_RELAY_STEP_UP
elif self._state == PIDAutotuneState.RELAY_STEP_DOWN
and input_val < self._setpoint - self._noiseband:
self._state = PIDAutotuneState.RELAY_STEP_UP
logging.debug('switched state: {0}'.format(self._state))
logging.debug('input: {0}'.format(input_val))
# set output
if (self._state == PIDAutotuneState.STATE_RELAY_STEP_UP):
if self._state == PIDAutotuneState.RELAY_STEP_UP:
self._output = self._initial_output - self._outputstep
elif self._state == PIDAutotuneState.STATE_RELAY_STEP_DOWN:
elif self._state == PIDAutotuneState.RELAY_STEP_DOWN:
self._output = self._initial_output + self._outputstep
# respect output limits
@ -184,16 +201,16 @@ class PIDAutotune:
logging.debug('amplitude deviation: {0}'.format(amplitude_dev))
if amplitude_dev < PIDAutotune.PEAK_AMPLITUDE_TOLERANCE:
self._state = PIDAutotuneState.STATE_SUCCEEDED
self._state = PIDAutotuneState.SUCCEEDED
# if the autotune has not already converged
# terminate after 10 cycles
if self._peak_count >= 20:
self._output = 0
self._state = PIDAutotuneState.STATE_FAILED
self._state = PIDAutotuneState.FAILED
return True
if self._state == PIDAutotuneState.STATE_SUCCEEDED:
if self._state == PIDAutotuneState.SUCCEEDED:
self._output = 0
logging.debug('peak finding successful')