Extra bail removed

OSError raised anyways

flake.nix

@@ -118,6 +118,20 @@
         ];
       };
 
+      pytec-dev-wrappers = pkgs.runCommandNoCC "pytec-dev-wrappers" {}
+        ''
+        mkdir -p $out/bin
+        for program in ${self}/pytec/*.py; do
+          if [ -x $program ]; then
+            progname=`basename -s .py $program`
+            outname=$out/bin/$progname
+            echo "#!${pkgs.bash}/bin/bash" >> $outname
+            echo "exec python3 -m pytec.$progname \"\$@\"" >> $outname
+            chmod 755 $outname
+          fi
+        done
+        '';
+
       thermostat_gui = pkgs.python3Packages.buildPythonPackage {
         pname = "thermostat_gui";
         version = "0.0.0";
@@ -163,6 +177,7 @@
             rust
             openocd
             dfu-util
+            pytec-dev-wrappers
           ]
           ++ (with python3Packages; [
             numpy
@@ -174,6 +189,9 @@
             pglive
             qtextras
           ]);
+          shellHook = ''
+            export PYTHONPATH=`pwd`/pytec:$PYTHONPATH
+          '';
       };
       defaultPackage.x86_64-linux = thermostat;
     };

pytec/examples/aioexample.py

@@ -1,10 +1,10 @@
 import asyncio
-from pytec.aioclient import Client
+from pytec.aioclient import AsyncioClient
 
 
 async def main():
-    tec = Client()
-    await tec.start_session()  # (host="192.168.1.26", port=23)
+    tec = AsyncioClient()
+    await tec.connect()  # (host="192.168.1.26", port=23)
     await tec.set_param("s-h", 1, "t0", 20)
     print(await tec.get_pwm())
     print(await tec.get_pid())

pytec/pytec/aioclient.py

@@ -7,56 +7,30 @@ class CommandError(Exception):
     pass
 
 
-class StoppedConnecting(Exception):
-    pass
-
-
-class Client:
+class AsyncioClient:
     def __init__(self):
         self._reader = None
         self._writer = None
-        self._connecting_task = None
         self._command_lock = asyncio.Lock()
         self._report_mode_on = False
         self.timeout = None
 
-    async def start_session(self, host="192.168.1.26", port=23, timeout=None):
-        """Start session to Thermostat at specified host and port.
-        Throws StoppedConnecting if disconnect was called while connecting.
-        Throws asyncio.TimeoutError if timeout was exceeded.
+    async def connect(self, host="192.168.1.26", port=23):
+        """Connect to Thermostat at specified host and port.
 
         Example::
-            client = Client()
-            try:
-                await client.start_session()
-            except StoppedConnecting:
-                print("Stopped connecting")
+            client = AsyncioClient()
+            await client.connect()
         """
-        self._connecting_task = asyncio.create_task(
-            asyncio.wait_for(asyncio.open_connection(host, port), timeout)
-        )
-        self.timeout = timeout
-        try:
-            self._reader, self._writer = await self._connecting_task
-        except asyncio.CancelledError:
-            raise StoppedConnecting
-        finally:
-            self._connecting_task = None
-
+        self._reader, self._writer = await asyncio.open_connection(host, port)
         await self._check_zero_limits()
 
-    def connecting(self):
-        """Returns True if client is connecting"""
-        return self._connecting_task is not None
-
     def connected(self):
         """Returns True if client is connected"""
         return self._writer is not None
 
-    async def end_session(self):
-        """End session to Thermostat if connected, cancel connection if connecting"""
-        if self._connecting_task is not None:
-            self._connecting_task.cancel()
+    async def disconnect(self):
+        """Disconnect from the Thermostat"""
 
         if self._writer is None:
             return
@@ -93,11 +67,10 @@ class Client:
 
     async def _command(self, *command):
         async with self._command_lock:
-            # protect the read-write process from being cancelled midway
-            line = await asyncio.shield(self._read_write(command))
+            line = await self._read_write(command)
 
         response = json.loads(line)
-        logging.debug(f"{command}: {response}")
+        logging.debug("%s: %s", command, response)
         if "error" in response:
             raise CommandError(response["error"])
         return response
@@ -268,7 +241,7 @@ class Client:
             self._writer.write("reset\n".encode("utf-8"))
             await self._writer.drain()
 
-        await self.end_session()
+        await self.disconnect()
 
     async def dfu(self):
         """Put the Thermostat in DFU update mode
@@ -281,7 +254,7 @@ class Client:
             self._writer.write("dfu\n".encode("utf-8"))
             await self._writer.drain()
 
-        await self.end_session()
+        await self.disconnect()
 
     async def ipv4(self):
         """Get the IPv4 settings of the Thermostat"""

pytec/pytec/gui/model/pid_autotuner.py

@@ -4,10 +4,10 @@ from autotune import PIDAutotuneState, PIDAutotune
 
 
 class PIDAutoTuner(QObject):
-    def __init__(self, parent, client, num_of_channel):
+    def __init__(self, parent, thermostat, num_of_channel):
         super().__init__(parent)
 
-        self._client = client
+        self._thermostat = thermostat
         self.autotuners = [PIDAutotune(25) for _ in range(num_of_channel)]
         self.target_temp = [20.0 for _ in range(num_of_channel)]
         self.test_current = [1.0 for _ in range(num_of_channel)]
@@ -37,7 +37,7 @@ class PIDAutoTuner(QObject):
 
     async def stop_pid_from_running(self, ch):
         self.autotuners[ch].setOff()
-        await self._client.set_param("pwm", ch, "i_set", 0)
+        await self._thermostat.set_param("pwm", ch, "i_set", 0)
 
     @asyncSlot(list)
     async def tick(self, report):
@@ -56,21 +56,21 @@ class PIDAutoTuner(QObject):
                     self.autotuners[ch].run(
                         channel_report["temperature"], channel_report["time"]
                     )
-                    await self._client.set_param(
+                    await self._thermostat.set_param(
                         "pwm", ch, "i_set", self.autotuners[ch].output()
                     )
                 case PIDAutotuneState.STATE_SUCCEEDED:
                     kp, ki, kd = self.autotuners[ch].get_tec_pid()
                     self.autotuners[ch].setOff()
 
-                    await self._client.set_param("pid", ch, "kp", kp)
-                    await self._client.set_param("pid", ch, "ki", ki)
-                    await self._client.set_param("pid", ch, "kd", kd)
-                    await self._client.set_param("pwm", ch, "pid")
+                    await self._thermostat.set_param("pid", ch, "kp", kp)
+                    await self._thermostat.set_param("pid", ch, "ki", ki)
+                    await self._thermostat.set_param("pid", ch, "kd", kd)
+                    await self._thermostat.set_param("pwm", ch, "pid")
 
-                    await self._client.set_param(
+                    await self._thermostat.set_param(
                         "pid", ch, "target", self.target_temp[ch]
                     )
                 case PIDAutotuneState.STATE_FAILED:
                     self.autotuners[ch].setOff()
-                    await self._client.set_param("pwm", ch, "i_set", 0)
+                    await self._thermostat.set_param("pwm", ch, "i_set", 0)

pytec/pytec/gui/model/thermostat.py

@@ -1,20 +1,16 @@
-from pytec.aioclient import Client
 from PyQt6.QtCore import pyqtSignal, QObject, pyqtSlot
 from qasync import asyncSlot
 from pytec.gui.model.property import Property, PropertyMeta
 import asyncio
 import logging
+from enum import Enum
+from pytec.aioclient import AsyncioClient
 
 
-class WrappedClient(QObject, Client):
-    connection_error = pyqtSignal()
-
-    async def _read_line(self):
-        try:
-            return await super()._read_line()
-        except (Exception, TimeoutError, asyncio.exceptions.TimeoutError):
-            logging.error("Client connection error, disconnecting", exc_info=True)
-            self.connection_error.emit()
+class ThermostatConnectionState(Enum):
+    DISCONNECTED = "disconnected"
+    CONNECTING = "connecting"
+    CONNECTED = "connected"
 
 
 class Thermostat(QObject, metaclass=PropertyMeta):
@@ -27,53 +23,70 @@ class Thermostat(QObject, metaclass=PropertyMeta):
     interval = Property(list)
     report = Property(list)
     info_box_trigger = pyqtSignal(str, str)
+    connection_error = pyqtSignal()
 
-    def __init__(self, parent, client, update_s):
+    def __init__(self, parent, update_s):
         self._update_s = update_s
-        self._client = client
+        self._client = AsyncioClient()
         self._watch_task = None
         self._report_mode_task = None
         self._poll_for_report = True
+        self._update_params_task = None
+        self.connection_errored = False
         super().__init__(parent)
 
+    async def start_session(self, host, port):
+        await self._client.connect(host, port)
+        hw_rev_data = await self.get_hw_rev()
+        self.start_watching()
+        return hw_rev_data
+
     async def run(self):
-        self.task = asyncio.create_task(self.update_params())
+        self._update_params_task = asyncio.create_task(self.update_params())
         while True:
-            if self.task.done():
-                if self.task.exception() is not None:
+            if self._update_params_task.done():
                 try:
-                        raise self.task.exception()
-                    except asyncio.TimeoutError:
+                    self._update_params_task.result()
+                except OSError:
                     logging.error(
-                            "Encountered an error while updating parameter tree.",
+                        "Encountered an error while polling for information from Thermostat.",
                         exc_info=True,
                     )
-                _ = self.task.result()
-                self.task = asyncio.create_task(self.update_params())
+                    self.connection_error.emit()
+                    return
+                self._update_params_task = asyncio.create_task(self.update_params())
             await asyncio.sleep(self._update_s)
 
+    @pyqtSlot()
+    def timed_out(self):
+        self.connection_errored = True
+
     async def get_hw_rev(self):
         self.hw_rev = await self._client.hw_rev()
         return self.hw_rev
 
     async def update_params(self):
-        self.fan = await self._client.get_fan()
-        self.pwm = await self._client.get_pwm()
+        fan_task = asyncio.create_task(self._client.get_fan())
+        pwm_task = asyncio.create_task(self._client.get_pwm())
+        pid_task = asyncio.create_task(self._client.get_pid())
+        report_task = asyncio.create_task(self._client.report())
+        thermistor_task = asyncio.create_task(self._client.get_steinhart_hart())
+        postfilter_task = asyncio.create_task(self._client.get_postfilter())
+
+        self.fan = await fan_task
+        self.pwm = await pwm_task
         if self._poll_for_report:
-            self.report = await self._client.report()
+            self.report = await report_task
             self.interval = [
                 self.report[i]["interval"] for i in range(len(self.report))
             ]
-        self.pid = await self._client.get_pid()
-        self.thermistor = await self._client.get_steinhart_hart()
-        self.postfilter = await self._client.get_postfilter()
+        self.pid = await pid_task
+        self.thermistor = await thermistor_task
+        self.postfilter = await postfilter_task
 
     def connected(self):
         return self._client.connected()
 
-    def connecting(self):
-        return self._client.connecting()
-
     def start_watching(self):
         self._watch_task = asyncio.create_task(self.run())
 
@@ -83,8 +96,8 @@ class Thermostat(QObject, metaclass=PropertyMeta):
             await self.set_report_mode(False)
             self._watch_task.cancel()
             self._watch_task = None
-            self.task.cancel()
-            self.task = None
+            self._update_params_task.cancel()
+            self._update_params_task = None
 
     async def set_report_mode(self, enabled: bool):
         self._poll_for_report = not enabled
@@ -101,7 +114,10 @@ class Thermostat(QObject, metaclass=PropertyMeta):
             ]
 
     async def end_session(self):
-        await self._client.end_session()
+        await self.set_report_mode(False)
+        self.stop_watching()
+        await self._client.disconnect()
+        self.connection_errored = False
 
     async def set_ipv4(self, ipv4):
         await self._client.set_param("ipv4", ipv4)
@@ -132,3 +148,12 @@ class Thermostat(QObject, metaclass=PropertyMeta):
     @pyqtSlot(float)
     def set_update_s(self, update_s):
         self._update_s = update_s
+
+    async def set_fan(self, power="auto"):
+        await self._client.set_fan(power)
+
+    async def get_fan(self):
+        return await self._client.get_fan()
+
+    async def set_param(self, topic, channel, field="", value=""):
+        await self._client.set_param(topic, channel, field, value)

pytec/tec_qt.py

@@ -7,12 +7,11 @@ from pytec.gui.view.live_plot_view import LiveDataPlotter
 from pytec.gui.view.ctrl_panel import CtrlPanel
 from pytec.gui.view.info_box import InfoBox
 from pytec.gui.model.pid_autotuner import PIDAutoTuner
-from pytec.gui.model.thermostat import WrappedClient, Thermostat
+from pytec.gui.model.thermostat import Thermostat, ThermostatConnectionState
 import json
 from autotune import PIDAutotuneState
 from qasync import asyncSlot, asyncClose
 import qasync
-from pytec.aioclient import StoppedConnecting
 import asyncio
 import logging
 import argparse
@@ -30,9 +29,9 @@ def get_argparser():
         "--connect",
         default=None,
         action="store_true",
-        help="Automatically connect to the specified Thermostat in IP:port format",
+        help="Automatically connect to the specified Thermostat in host:port format",
     )
-    parser.add_argument("IP", metavar="ip", default=None, nargs="?")
+    parser.add_argument("HOST", metavar="host", default=None, nargs="?")
     parser.add_argument("PORT", metavar="port", default=None, nargs="?")
     parser.add_argument(
         "-l",
@@ -63,17 +62,25 @@ class MainWindow(QtWidgets.QMainWindow):
         self.hw_rev_data = None
         self.info_box = InfoBox()
 
-        self.client = WrappedClient(self)
-        self.client.connection_error.connect(self.bail)
-
         self.thermostat = Thermostat(
-            self, self.client, self.report_refresh_spin.value()
+            self, self.report_refresh_spin.value()
+        )
+        self._connecting_task = None
+
+        def handle_connection_error():
+            self.info_box.display_info_box(
+                "Connection Error", "Thermostat connection lost. Is it unplugged?"
             )
 
-        self.autotuners = PIDAutoTuner(self, self.client, 2)
+        self.thermostat.connection_error.connect(handle_connection_error)
+
+        self.thermostat.connection_error.connect(self.thermostat.timed_out)
+        self.thermostat.connection_error.connect(self.bail)
+
+        self.autotuners = PIDAutoTuner(self, self.thermostat, 2)
 
         def get_ctrl_panel_config(args):
-            with open(args.param_tree, "r") as f:
+            with open(args.param_tree, "r", encoding="utf-8") as f:
                 return json.load(f)["ctrl_panel"]
 
         param_tree_sigActivated_handles = [
@@ -167,20 +174,33 @@ class MainWindow(QtWidgets.QMainWindow):
         self.channel_graphs.clear_graphs()
 
     async def _on_connection_changed(self, result):
-        self.graph_group.setEnabled(result)
-        self.report_group.setEnabled(result)
-        self.thermostat_settings.setEnabled(result)
+        match result:
+            case ThermostatConnectionState.CONNECTED:
+                self.graph_group.setEnabled(True)
+                self.report_group.setEnabled(True)
+                self.thermostat_settings.setEnabled(True)
 
-        self.conn_menu.host_set_line.setEnabled(not result)
-        self.conn_menu.port_set_spin.setEnabled(not result)
-        self.connect_btn.setText("Disconnect" if result else "Connect")
-        if result:
-            self.hw_rev_data = await self.thermostat.get_hw_rev()
-            logging.debug(self.hw_rev_data)
+                self.conn_menu.host_set_line.setEnabled(False)
+                self.conn_menu.port_set_spin.setEnabled(False)
+                self.connect_btn.setText("Disconnect")
 
                 self._status(self.hw_rev_data)
-            self.thermostat.start_watching()
-        else:
+
+            case ThermostatConnectionState.CONNECTING:
+                self.status_lbl.setText("Connecting...")
+                self.connect_btn.setText("Stop")
+                self.conn_menu.host_set_line.setEnabled(False)
+                self.conn_menu.port_set_spin.setEnabled(False)
+
+            case ThermostatConnectionState.DISCONNECTED:
+                self.graph_group.setEnabled(False)
+                self.report_group.setEnabled(False)
+                self.thermostat_settings.setEnabled(False)
+
+                self.conn_menu.host_set_line.setEnabled(True)
+                self.conn_menu.port_set_spin.setEnabled(True)
+                self.connect_btn.setText("Connect")
+
                 self.status_lbl.setText("Disconnected")
                 self.background_task_lbl.setText("Ready.")
                 self.loading_spinner.hide()
@@ -189,15 +209,15 @@ class MainWindow(QtWidgets.QMainWindow):
                 self.thermostat_ctrl_menu.fan_pwm_warning.setToolTip("")
                 self.clear_graphs()
                 self.report_box.setChecked(False)
-            if not Thermostat.connecting or Thermostat.connected:
                 for ch in range(self.NUM_CHANNELS):
                     if self.autotuners.get_state(ch) != PIDAutotuneState.STATE_OFF:
+                        if self.thermostat.connection_errored:
+                            # Don't send any commands, just reset local state
+                            self.autotuners.autotuners[ch].setOff()
+                        else:
                             await self.autotuners.stop_pid_from_running(ch)
-            await self.thermostat.set_report_mode(False)
-            self.thermostat.stop_watching()
 
     def _status(self, hw_rev_d: dict):
-        logging.debug(hw_rev_d)
         self.status_lbl.setText(
             f"Connected to Thermostat v{hw_rev_d['rev']['major']}.{hw_rev_d['rev']['minor']}"
         )
@@ -221,7 +241,7 @@ class MainWindow(QtWidgets.QMainWindow):
         await self.thermostat.set_report_mode(enabled)
 
     @asyncClose
-    async def closeEvent(self, event):
+    async def closeEvent(self, _event):
         try:
             await self.bail()
         except:
@@ -229,36 +249,36 @@ class MainWindow(QtWidgets.QMainWindow):
 
     @asyncSlot()
     async def on_connect_btn_clicked(self):
-        host, port = (
-            self.conn_menu.host_set_line.text(),
-            self.conn_menu.port_set_spin.value(),
+        if (self._connecting_task is None) and (not self.thermostat.connected()):
+            await self._on_connection_changed(ThermostatConnectionState.CONNECTING)
+
+            self._connecting_task = asyncio.create_task(
+                self.thermostat.start_session(
+                    host=self.conn_menu.host_set_line.text(),
+                    port=self.conn_menu.port_set_spin.value(),
+                )
             )
             try:
-            if not (self.client.connecting() or self.client.connected()):
-                self.status_lbl.setText("Connecting...")
-                self.connect_btn.setText("Stop")
-                self.conn_menu.host_set_line.setEnabled(False)
-                self.conn_menu.port_set_spin.setEnabled(False)
-
-                try:
-                    await self.client.start_session(host=host, port=port, timeout=5)
-                except StoppedConnecting:
+                self.hw_rev_data = await self._connecting_task
+            except (OSError, asyncio.CancelledError) as exc:
+                await self.bail()
+                if isinstance(exc, asyncio.CancelledError):
                     return
-                await self._on_connection_changed(True)
+                raise
+            finally:
+                self._connecting_task = None
+
+            await self._on_connection_changed(ThermostatConnectionState.CONNECTED)
+
+        elif self._connecting_task is not None:
+            self._connecting_task.cancel()
         else:
             await self.bail()
 
-        # TODO: Remove asyncio.TimeoutError in Python 3.11
-        except (OSError, asyncio.TimeoutError):
-            try:
-                await self.bail()
-            except ConnectionResetError:
-                pass
-
     @asyncSlot()
     async def bail(self):
-        await self._on_connection_changed(False)
-        await self.client.end_session()
+        await self._on_connection_changed(ThermostatConnectionState.DISCONNECTED)
+        await self.thermostat.end_session()
 
     @asyncSlot(object, object)
     async def send_command(self, param, changes):
@@ -280,7 +300,7 @@ class MainWindow(QtWidgets.QMainWindow):
                     else:
                         set_param_args = (*thermostat_param, data)
                     param.child(*param.childPath(inner_param)).setOpts(lock=True)
-                    await self.client.set_param(*set_param_args)
+                    await self.thermostat.set_param(*set_param_args)
                     param.child(*param.childPath(inner_param)).setOpts(lock=False)
 
                 if inner_param.opts.get("pid_autotune", None) is not None:
@@ -296,7 +316,7 @@ class MainWindow(QtWidgets.QMainWindow):
                     if activater is not None:
                         if activater[1] == "ch":
                             activater[1] = ch
-                        await self.client.set_param(*activater)
+                        await self.thermostat.set_param(*activater)
 
     @asyncSlot()
     async def pid_auto_tune_request(self, ch=0):
@@ -359,24 +379,24 @@ class MainWindow(QtWidgets.QMainWindow):
 
     @asyncSlot(int)
     async def fan_set_request(self, value):
-        assert self.client.connected()
+        assert self.thermostat.connected()
 
         if self.thermostat_ctrl_menu.fan_auto_box.isChecked():
             with QSignalBlocker(self.thermostat_ctrl_menu.fan_auto_box):
                 self.thermostat_ctrl_menu.fan_auto_box.setChecked(False)
-        await self.client.set_fan(value)
+        await self.thermostat.set_fan(value)
         if not self.hw_rev_data["settings"]["fan_pwm_recommended"]:
             self.thermostat_ctrl_menu.set_fan_pwm_warning()
 
     @asyncSlot(int)
     async def fan_auto_set_request(self, enabled):
-        assert self.client.connected()
+        assert self.thermostat.connected()
 
         if enabled:
-            await self.client.set_fan("auto")
-            self.fan_update(await self.client.get_fan())
+            await self.thermostat.set_fan("auto")
+            self.fan_update(await self.thermostat.get_fan())
         else:
-            await self.client.set_fan(
+            await self.thermostat.set_fan(
                 self.thermostat_ctrl_menu.fan_power_slider.value()
             )
 
@@ -396,14 +416,14 @@ class MainWindow(QtWidgets.QMainWindow):
     async def dfu_request(self, _):
         assert self.thermostat.connected()
 
-        await self._on_connection_changed(False)
+        await self._on_connection_changed(ThermostatConnectionState.DISCONNECTED)
         await self.thermostat.dfu()
 
     @asyncSlot(bool)
     async def reset_request(self, _):
         assert self.thermostat.connected()
 
-        await self._on_connection_changed(False)
+        await self._on_connection_changed(ThermostatConnectionState.DISCONNECTED)
         await self.thermostat.reset()
         await asyncio.sleep(0.1)  # Wait for the reset to start
 
@@ -422,8 +442,7 @@ class MainWindow(QtWidgets.QMainWindow):
         assert self.thermostat.connected()
 
         await self.thermostat.set_ipv4(ipv4_settings)
-        await self.thermostat._client.end_session()
-        await self._on_connection_changed(False)
+        await self.bail()
 
 
 async def coro_main():