From 34b168536ed6a730e3e8697e5cac26479c6edb3b Mon Sep 17 00:00:00 2001 From: Mike Langmayr <1809691+mikelangmayr@users.noreply.github.com> Date: Mon, 12 Oct 2026 17:31:05 -0700 Subject: [PATCH] Report write-only keywords in keys.list so pollers skip them --- libby/client.py | 6 ++++++ libby/keygrabber/collection.py | 14 +++++++++----- libby/libby.py | 8 +++++++- tests/test_client_integration.py | 15 +++++++++++++++ tests/test_keygrabber_collection.py | 26 ++++++++++++++++++++++++++ 5 files changed, 63 insertions(+), 6 deletions(-) diff --git a/libby/client.py b/libby/client.py index 0d6b1b0..f5ad646 100644 --- a/libby/client.py +++ b/libby/client.py @@ -63,6 +63,10 @@ def _chunked(items: Sequence[str], size: int) -> Iterator[Sequence[str]]: class KeyListing: """One peer's answer to a ``keys.list`` request. + ``writeonly`` names those of ``names`` that have no value to read, so a + caller polling the peer can skip them. It is bare keyword names, matching + ``matches`` on the wire rather than the qualified ``names`` above. + ``services`` lists the non-keyword keys the peer answers, such as ``keys.read``. It is empty for a peer on a libby old enough not to report the field, which is how a caller decides whether bulk reads are available: @@ -71,6 +75,7 @@ class KeyListing: """ names: Tuple[str, ...] + writeonly: Tuple[str, ...] services: Tuple[str, ...] @@ -157,6 +162,7 @@ def listing(self, pattern: str, *, timeout_s: float = DEFAULT_TIMEOUT_S) -> KeyL return KeyListing( names=tuple(f"{group}.{daemon}.{match}" for match in response.get("matches", [])), + writeonly=tuple(response.get("writeonly", [])), services=tuple(response.get("services", [])), ) diff --git a/libby/keygrabber/collection.py b/libby/keygrabber/collection.py index 7e50d1c..1ee6e5d 100644 --- a/libby/keygrabber/collection.py +++ b/libby/keygrabber/collection.py @@ -109,14 +109,18 @@ def backoff_interval_s(self) -> float: def resolve(self, client: Client) -> Tuple[str, ...]: """Ask the peer what it serves and select the configured keywords. - One ``keys.list`` covers both: the peer's keyword names, and whether it - serves ``keys.read``. Selection is then local, so a collection with - several patterns still costs one request. + One ``keys.list`` covers all of it: the peer's keyword names, which of + them cannot be read, and whether it serves ``keys.read``. Selection is + then local, so a collection with several patterns still costs one + request. """ listing = client.listing(f"{self.config.peer}.%", timeout_s=self.config.timeout_s) - available = [name.rsplit(".", 1)[-1] for name in listing.names] - self._names = select_keywords(self.config, available) + # A trigger answers a show with `false`, and a setter-only keyword with + # an error, so neither is worth asking for on every tick + readable = [name.rsplit(".", 1)[-1] for name in listing.names + if name.rsplit(".", 1)[-1] not in listing.writeonly] + self._names = select_keywords(self.config, readable) self._bulk_read = BULK_READ_SERVICE in listing.services self._resolved_at = self._clock() return self._names diff --git a/libby/libby.py b/libby/libby.py index 5f6664a..946ad10 100644 --- a/libby/libby.py +++ b/libby/libby.py @@ -265,9 +265,15 @@ def _keys_list(self, payload: dict, _ctx: dict) -> dict: pattern = payload.get("pattern", "%") if not isinstance(pattern, str): return {"ok": False, "error": "pattern must be a string"} + matches = match_pattern(pattern, self._keywords) return { "ok": True, - "matches": match_pattern(pattern, self._keywords), + "matches": matches, + # Which of those have no value to read, so a caller polling a peer + # does not ask for them. A trigger answers a show with `false`, + # which would otherwise be archived as data. + "writeonly": [name for name in matches + if self._keywords[name].writeonly], "services": self._served_services(), "is_daemon": self.is_daemon, } diff --git a/tests/test_client_integration.py b/tests/test_client_integration.py index a9e47a2..33e7662 100644 --- a/tests/test_client_integration.py +++ b/tests/test_client_integration.py @@ -61,6 +61,9 @@ def on_start(self, libby) -> None: units="mm", ) self.keyword_registry.bool("isreferenced", getter=lambda: True) + # Setter-only, like a gauge's unit selector: listed, but showing it + # yields an error rather than a value + self.keyword_registry.int("mode", setter=lambda v: state.update(mode=v)) class _LifecycleDaemon(LibbyDaemon): @@ -193,6 +196,18 @@ def test_listing_advertises_bulk_read(self): self.assertIn("keys.read", listing.services) self.assertIn(self._name("positionvalue"), listing.names) + def test_listing_names_the_keywords_that_cannot_be_read(self): + """A setter-only keyword is listed, and flagged as unreadable. + + It stays in names so `libby list` still shows it; a caller polling + the peer uses writeonly to skip it rather than reading an error + every tick. + """ + listing = self.client.listing(self._name("%"), timeout_s=RPC_TIMEOUT_S) + self.assertIn(self._name("mode"), listing.names) + self.assertEqual(listing.writeonly, ("mode",)) + self.assertNotIn("positionvalue", listing.writeonly) + def test_describe_reports_type_and_units(self): """Read one keyword's metadata.""" meta = self.client.describe(self._name("positionvalue"), diff --git a/tests/test_keygrabber_collection.py b/tests/test_keygrabber_collection.py index 4ebad91..41a3305 100644 --- a/tests/test_keygrabber_collection.py +++ b/tests/test_keygrabber_collection.py @@ -44,11 +44,13 @@ def __init__( self, names: Sequence[str] = ("positionvalue", "isconnected"), services: Sequence[str] = ("keys.read",), + writeonly: Sequence[str] = (), values: Optional[Dict[str, Dict[str, Any]]] = None, timeouts_after: Optional[int] = None, ) -> None: self.names = list(names) self.services = list(services) + self.writeonly = list(writeonly) self.values = values or {} self.timeouts_after = timeouts_after self.read_calls: List[Tuple[str, ...]] = [] @@ -61,6 +63,7 @@ def listing(self, pattern: str, *, timeout_s: float = 3.0) -> KeyListing: prefix = pattern.rsplit(".", 1)[0] return KeyListing( names=tuple(f"{prefix}.{name}" for name in self.names), + writeonly=tuple(self.writeonly), services=tuple(self.services), ) @@ -209,6 +212,29 @@ def test_fallback_abandons_the_tick_after_a_timeout(self): self.assertEqual(result.read_errors, 4 - 1) self.assertEqual(len(result.samples), 1) + def test_write_only_keywords_are_never_read(self): + """A trigger answers a show with `false`, which is not a measurement. + + Archiving it would record `shutdown` as a datapoint on every peer at + every interval, forever. + """ + client = _FakeClient(names=("pressure", "shutdown", "units_code"), + writeonly=("shutdown", "units_code"), + values={"pressure": {"ok": True, "value": 1.0}}) + collection = Collection(_collection_config()) + + self.assertEqual(collection.resolve(client), ("pressure",)) + result = collection.tick(client, TIMESTAMP) + self.assertEqual(client.read_calls, [("hsfei.adc.pressure",)]) + self.assertEqual(result.read_errors, 0) + self.assertEqual([s.keyword for s in result.samples], ["pressure"]) + + def test_a_peer_reporting_none_is_read_as_before(self): + """An older libby omits the field, so nothing is filtered.""" + client = _FakeClient(names=("pressure", "shutdown"), writeonly=()) + self.assertEqual(Collection(_collection_config()).resolve(client), + ("pressure", "shutdown")) + if __name__ == "__main__": unittest.main()