From dad8ebde3d145ff0ac1491960b63d1cc7e7a45b6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jos=C3=A9=20S=C3=A1nchez-Gallego?= <gallegoj@uw.edu>
Date: Mon, 15 Jan 2024 20:03:20 +0000
Subject: [PATCH] Modbus.read_group() calls get_all()

---
 CHANGELOG.md            |  1 +
 python/lvmecp/hvac.py   |  2 +-
 python/lvmecp/modbus.py | 25 ++++++++++++++-----------
 3 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 40f6bd4..8e31c93 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,6 +5,7 @@
 ### 🚀 New
 
 * Added a cache to the registers with default timeout 1 second.
+* `Modbus.read_group()` calls `Modbus.get_all()` instead of reading individual registers sequentially. Since during a `status` all groups are read in quick succession, and with caching, this results in much faster status outputs.
 
 ### ✨ Improved
 
diff --git a/python/lvmecp/hvac.py b/python/lvmecp/hvac.py
index a6e7059..fde9788 100644
--- a/python/lvmecp/hvac.py
+++ b/python/lvmecp/hvac.py
@@ -23,7 +23,7 @@ class HVACController(PLCModule):
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
 
-        self.status: dict[str, float | bool] = {}
+        self.status: dict[str, float | bool | None] = {}
 
     async def _update_internal(self):
         """Update status."""
diff --git a/python/lvmecp/modbus.py b/python/lvmecp/modbus.py
index 4f84e16..4686973 100644
--- a/python/lvmecp/modbus.py
+++ b/python/lvmecp/modbus.py
@@ -336,8 +336,11 @@ async def get_all(self):
             results = await asyncio.gather(*tasks, return_exceptions=True)
 
         if any([isinstance(result, Exception) for result in results]):
-            for result in results:
+            for ii, result in enumerate(results):
                 if isinstance(result, Exception):
+                    log.warning(f"Failed retrieving value for {names[ii]!r}")
+                    results[ii] = None
+
         registers = cast(
             dict[str, int | float | None],
             {names[ii]: results[ii] for ii in range(len(names))},
@@ -351,15 +354,15 @@ async def get_all(self):
     async def read_group(self, group: str):
         """Returns a dictionary of all read registers that match a ``group``."""
 
-        names = []
-        tasks = []
-        async with self:
-            for name in self:
-                register = self[name]
-                if register.group is not None and register.group == group:
-                    names.append(name)
-                    tasks.append(register.get(open_connection=False))
+        registers = await self.get_all()
+
+        group_registers = {}
+        for name in self:
+            register = self[name]
+            if register.group is not None and register.group == group:
+                if name not in registers:
+                    continue
 
-            results = await asyncio.gather(*tasks)
+                group_registers[name] = registers[name]
 
-        return dict(zip(names, results))
+        return group_registers