From de0931c12769e22807c6c6a50066985bc7a42247 Mon Sep 17 00:00:00 2001 From: Sean McLellan Date: Wed, 4 Feb 2026 19:18:31 -0500 Subject: [PATCH 1/3] =?UTF-8?q?feat:=20v0.6.0=20=E2=80=94=20generic=20tele?= =?UTF-8?q?metry=20events,=20default=20filters,=20per-frame=20TUI=20update?= =?UTF-8?q?s?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All 230+ telemetry fields now produce OpenClaw events via a generic fallback emitter (PascalCase → snake_case event types). Unconfigured fields pass through DualGateFilter with sensible defaults instead of being silently dropped. Added telemetry.get read handler and MCP tool for arbitrary field reads. TUI panels now update per-frame instead of polling on a 1-second timer. Co-Authored-By: Claude Opus 4.5 --- CHANGELOG.md | 14 + pyproject.toml | 2 +- src/tescmd/__init__.py | 2 +- src/tescmd/cli/serve.py | 439 ++++++++++++++++++++++++++---- src/tescmd/openclaw/bridge.py | 261 +++++++++++++----- src/tescmd/openclaw/config.py | 32 ++- src/tescmd/openclaw/dispatcher.py | 167 ++++++++++-- src/tescmd/openclaw/emitter.py | 34 ++- src/tescmd/openclaw/filters.py | 49 +++- src/tescmd/telemetry/tui.py | 98 +++++-- src/tescmd/triggers/manager.py | 108 +++++++- src/tescmd/triggers/models.py | 1 + tests/openclaw/test_bridge.py | 420 +++++++++++++++++++++++++++- tests/openclaw/test_config.py | 17 +- tests/openclaw/test_dispatcher.py | 432 +++++++++++++++++++++++++---- tests/openclaw/test_emitter.py | 40 ++- tests/openclaw/test_filters.py | 91 ++++++- tests/openclaw/test_gateway.py | 21 +- tests/telemetry/test_tui.py | 98 ++++++- tests/triggers/test_manager.py | 130 ++++++++- 20 files changed, 2168 insertions(+), 288 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e14b57..c604d9d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,20 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [0.6.0] - 2026-02-04 + +### Added + +- **Generic telemetry events** — all 230+ telemetry fields now produce OpenClaw events via a generic fallback in the emitter; unmapped fields are emitted as snake_case event types (e.g. `PackVoltage` → `pack_voltage`) with `field` and `value` payload keys +- **Default filter for unconfigured fields** — `DualGateFilter` now applies a sensible default (any-change granularity, 5s throttle, 2min staleness) to unconfigured fields instead of silently dropping them; explicitly disabled fields are still blocked +- **`telemetry.get` handler** — generic read handler in `CommandDispatcher` allows agents to read the latest value of any telemetry field from the in-memory store +- **`telemetry_get` MCP tool** — exposes `telemetry.get` as an MCP tool so agent frameworks can read arbitrary telemetry fields +- **Trigger commands in capabilities** — `NodeCapabilities` now advertises `telemetry.get`, `trigger.list`, `trigger.poll`, `trigger.create`, and `trigger.delete` to the gateway + +### Fixed + +- **TUI panel widgets not updating** — telemetry panel DataTables now update per-frame as data arrives instead of relying solely on a 1-second timer poll; header, server info, and trigger displays remain on the timer + ## [0.5.0] - 2026-02-03 ### Changed diff --git a/pyproject.toml b/pyproject.toml index 1665302..bf74a9a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "tescmd" -version = "0.5.0" +version = "0.6.0" description = "A Python CLI for querying and controlling Tesla vehicles via the Fleet API" readme = "README.md" license = "MIT" diff --git a/src/tescmd/__init__.py b/src/tescmd/__init__.py index 8dbfbae..c8ebe21 100644 --- a/src/tescmd/__init__.py +++ b/src/tescmd/__init__.py @@ -1,3 +1,3 @@ """tescmd — A Python CLI for querying and controlling Tesla vehicles via the Fleet API.""" -__version__ = "0.5.0" +__version__ = "0.6.0" diff --git a/src/tescmd/cli/serve.py b/src/tescmd/cli/serve.py index 98eefe7..fe5b7a1 100644 --- a/src/tescmd/cli/serve.py +++ b/src/tescmd/cli/serve.py @@ -2,6 +2,7 @@ from __future__ import annotations +import contextlib import logging import random import signal @@ -370,6 +371,10 @@ async def _jsonl_sink(frame: object) -> None: fanout.add_sink(_jsonl_sink) + # Telemetry store used for immediate trigger evaluation at creation + # time. Set below in whichever branch owns the store. + _telemetry_store = None + # OpenClaw sink — optional bridge to an OpenClaw gateway if openclaw_url: from pathlib import Path @@ -390,6 +395,7 @@ async def _jsonl_sink(frame: object) -> None: ) gw = oc_pipeline.gateway oc_bridge = oc_pipeline.bridge + _telemetry_store = oc_pipeline.telemetry_store # Push trigger notifications to gateway if trigger_manager is not None: @@ -424,6 +430,7 @@ async def _jsonl_sink(frame: object) -> None: from tescmd.openclaw.telemetry_store import TelemetryStore as _TStore _trigger_store = _TStore() + _telemetry_store = _trigger_store async def _trigger_sink(frame: object) -> None: from tescmd.telemetry.decoder import TelemetryFrame @@ -442,7 +449,7 @@ async def _trigger_sink(frame: object) -> None: # -- Register MCP trigger tools (when both MCP and telemetry are active) -- if mcp_server is not None and trigger_manager is not None: - _register_trigger_tools(mcp_server, trigger_manager) + _register_trigger_tools(mcp_server, trigger_manager, _telemetry_store) tool_count = len(mcp_server.list_tools()) # -- Tailscale Funnel setup (optional, MCP-only mode) -- @@ -490,6 +497,8 @@ async def _trigger_sink(frame: object) -> None: if public_url: tui.set_tunnel_url(public_url) tui.set_sink_count(fanout.sink_count) + if trigger_manager is not None: + tui.set_trigger_manager(trigger_manager) if csv_sink is not None: tui.set_log_path(csv_sink.log_path) @@ -646,21 +655,49 @@ def _handle_sigterm() -> None: ) -def _register_trigger_tools(mcp_server: Any, trigger_manager: Any) -> None: - """Register trigger CRUD tools on the MCP server.""" - from tescmd.triggers.models import TriggerCondition, TriggerDefinition, TriggerOperator +def _register_trigger_tools( + mcp_server: Any, trigger_manager: Any, telemetry_store: Any = None +) -> None: + """Register domain-specific trigger CRUD tools on the MCP server. + + Each trigger domain (cabin_temp, outside_temp, battery, location) gets + its own create, list, and delete tools. Temperature triggers accept + values in °F and convert to °C internally (matching the dispatcher's + convenience aliases). ``trigger_poll`` is shared across all domains. + + When *telemetry_store* is provided, newly created triggers are + immediately evaluated against the current value. If the condition + is already satisfied the response includes ``"immediate": True``. + One-shot triggers are **not** deleted immediately — the push + callback handles deletion after confirmed WebSocket delivery. + """ + from tescmd.triggers.manager import _matches + from tescmd.triggers.models import ( + TriggerCondition, + TriggerDefinition, + TriggerNotification, + TriggerOperator, + ) + + def _f_to_c(f: float) -> float: + return round((f - 32.0) * 5.0 / 9.0, 1) + + def _c_to_f(c: float) -> float: + return round(c * 9.0 / 5.0 + 32.0, 1) - def _handle_create(params: dict[str, Any]) -> dict[str, Any]: - field = params.get("field") - if not field: - raise ValueError("trigger_create requires 'field' parameter") + def _create_trigger( + field: str, params: dict[str, Any], *, convert_temp: bool = False + ) -> dict[str, Any]: op_str = params.get("operator") if not op_str: - raise ValueError("trigger_create requires 'operator' parameter") + raise ValueError("Trigger requires 'operator' parameter") + value = params.get("value") + if convert_temp and value is not None: + value = _f_to_c(float(value)) condition = TriggerCondition( field=field, operator=TriggerOperator(op_str), - value=params.get("value"), + value=value, ) trigger = TriggerDefinition( condition=condition, @@ -668,73 +705,365 @@ def _handle_create(params: dict[str, Any]) -> dict[str, Any]: cooldown_seconds=params.get("cooldown_seconds", 60.0), ) created = trigger_manager.create(trigger) - return dict(created.model_dump(mode="json")) - - def _handle_delete(params: dict[str, Any]) -> dict[str, Any]: + result = dict(created.model_dump(mode="json")) + + # Immediate evaluation: if the telemetry store already has a + # value that satisfies the condition, fire now. + # One-shot triggers are NOT deleted here — the push callback + # handles deletion after confirmed WebSocket delivery. + if telemetry_store is not None: + snap = telemetry_store.get(field) + if snap is not None and _matches(condition, snap.value, None): + from datetime import UTC, datetime + + notification = TriggerNotification( + trigger_id=created.id, + field=field, + operator=condition.operator, + threshold=condition.value, + value=snap.value, + previous_value=None, + fired_at=datetime.now(UTC), + vin=trigger_manager.vin, + once=trigger.once, + ) + trigger_manager.queue_notification(notification) + result["immediate"] = True + if trigger.once: + trigger_manager.mark_fired_once(created.id) + + return result + + def _list_triggers( + field: str, *, show_fahrenheit: bool = False + ) -> dict[str, Any]: + triggers = [ + t for t in trigger_manager.list_all() if t.condition.field == field + ] + result = [] + for t in triggers: + entry: dict[str, Any] = { + "id": t.id, + "field": t.condition.field, + "operator": t.condition.operator.value, + "value": t.condition.value, + "once": t.once, + "cooldown_seconds": t.cooldown_seconds, + } + if show_fahrenheit and t.condition.value is not None: + with contextlib.suppress(TypeError, ValueError): + entry["value_f"] = _c_to_f(float(t.condition.value)) + result.append(entry) + return {"triggers": result} + + def _delete_trigger(params: dict[str, Any]) -> dict[str, Any]: trigger_id = params.get("id") if not trigger_id: - raise ValueError("trigger_delete requires 'id' parameter") + raise ValueError("Trigger delete requires 'id' parameter") deleted = trigger_manager.delete(trigger_id) return {"deleted": deleted, "id": trigger_id} - def _handle_list(params: dict[str, Any]) -> dict[str, Any]: - triggers = trigger_manager.list_all() - return {"triggers": [t.model_dump(mode="json") for t in triggers]} + # -- Trigger option schemas ----------------------------------------------- - def _handle_poll(params: dict[str, Any]) -> dict[str, Any]: - notifications = trigger_manager.drain_pending() - return {"notifications": [n.model_dump(mode="json") for n in notifications]} - - mcp_server.register_custom_tool( - "trigger_create", - _handle_create, - "Create a telemetry trigger condition", - { - "type": "object", - "properties": { - "field": {"type": "string", "description": "Telemetry field name"}, - "operator": { - "type": "string", - "description": "Operator: lt, gt, lte, gte, eq, neq, changed, enter, leave", - }, - "value": {"description": "Threshold value (optional for 'changed')"}, - "once": { - "type": "boolean", - "description": "Fire once then auto-delete (default: false)", - }, - "cooldown_seconds": { - "type": "number", - "description": "Cooldown between firings (default: 60)", + _temp_trigger_schema: dict[str, Any] = { + "type": "object", + "properties": { + "operator": { + "type": "string", + "description": "Comparison: lt, gt, lte, gte, eq, neq, changed", + }, + "value": { + "type": "number", + "description": "Temperature threshold in °F", + }, + "once": { + "type": "boolean", + "description": "Fire once then auto-delete (default: false)", + }, + "cooldown_seconds": { + "type": "number", + "description": "Cooldown between firings in seconds (default: 60)", + }, + }, + "required": ["operator"], + } + + _battery_trigger_schema: dict[str, Any] = { + "type": "object", + "properties": { + "operator": { + "type": "string", + "description": "Comparison: lt, gt, lte, gte, eq, neq, changed", + }, + "value": { + "type": "number", + "description": "Battery level threshold (0-100 percent)", + }, + "once": { + "type": "boolean", + "description": "Fire once then auto-delete (default: false)", + }, + "cooldown_seconds": { + "type": "number", + "description": "Cooldown between firings in seconds (default: 60)", + }, + }, + "required": ["operator"], + } + + _location_trigger_schema: dict[str, Any] = { + "type": "object", + "properties": { + "operator": { + "type": "string", + "description": "Geofence operator: enter or leave", + }, + "value": { + "type": "object", + "description": "Geofence: {latitude, longitude, radius_m}", + "properties": { + "latitude": {"type": "number"}, + "longitude": {"type": "number"}, + "radius_m": {"type": "number"}, }, + "required": ["latitude", "longitude", "radius_m"], + }, + "once": { + "type": "boolean", + "description": "Fire once then auto-delete (default: false)", + }, + "cooldown_seconds": { + "type": "number", + "description": "Cooldown between firings in seconds (default: 60)", }, - "required": ["field", "operator"], }, + "required": ["operator", "value"], + } + + _trigger_list_schema: dict[str, Any] = { + "type": "object", + "properties": {}, + } + + _trigger_delete_schema: dict[str, Any] = { + "type": "object", + "properties": {"id": {"type": "string", "description": "Trigger ID"}}, + "required": ["id"], + } + + # -- Cabin temperature triggers ------------------------------------------- + + mcp_server.register_custom_tool( + "cabin_temp_trigger", + lambda p: _create_trigger("InsideTemp", p, convert_temp=True), + "Create a cabin temperature trigger (value in °F)", + _temp_trigger_schema, is_write=True, ) mcp_server.register_custom_tool( - "trigger_delete", - _handle_delete, - "Delete a telemetry trigger by ID", - { - "type": "object", - "properties": {"id": {"type": "string", "description": "Trigger ID"}}, - "required": ["id"], - }, + "cabin_temp_trigger_list", + lambda p: _list_triggers("InsideTemp", show_fahrenheit=True), + "List cabin temperature triggers with IDs and thresholds", + _trigger_list_schema, + ) + mcp_server.register_custom_tool( + "cabin_temp_trigger_delete", + _delete_trigger, + "Delete a cabin temperature trigger by ID", + _trigger_delete_schema, is_write=True, ) + + # -- Outside temperature triggers ---------------------------------------- + mcp_server.register_custom_tool( - "trigger_list", - _handle_list, - "List all active telemetry triggers", - {"type": "object", "properties": {}}, + "outside_temp_trigger", + lambda p: _create_trigger("OutsideTemp", p, convert_temp=True), + "Create an outside temperature trigger (value in °F)", + _temp_trigger_schema, + is_write=True, + ) + mcp_server.register_custom_tool( + "outside_temp_trigger_list", + lambda p: _list_triggers("OutsideTemp", show_fahrenheit=True), + "List outside temperature triggers with IDs and thresholds", + _trigger_list_schema, + ) + mcp_server.register_custom_tool( + "outside_temp_trigger_delete", + _delete_trigger, + "Delete an outside temperature trigger by ID", + _trigger_delete_schema, + is_write=True, + ) + + # -- Battery triggers ---------------------------------------------------- + + mcp_server.register_custom_tool( + "battery_trigger", + lambda p: _create_trigger("BatteryLevel", p), + "Create a battery level trigger (value in percent 0-100)", + _battery_trigger_schema, + is_write=True, + ) + mcp_server.register_custom_tool( + "battery_trigger_list", + lambda p: _list_triggers("BatteryLevel"), + "List battery level triggers with IDs and thresholds", + _trigger_list_schema, + ) + mcp_server.register_custom_tool( + "battery_trigger_delete", + _delete_trigger, + "Delete a battery level trigger by ID", + _trigger_delete_schema, + is_write=True, + ) + + # -- Location triggers --------------------------------------------------- + + mcp_server.register_custom_tool( + "location_trigger", + lambda p: _create_trigger("Location", p), + "Create a location geofence trigger (enter/leave)", + _location_trigger_schema, + is_write=True, + ) + mcp_server.register_custom_tool( + "location_trigger_list", + lambda p: _list_triggers("Location"), + "List location geofence triggers with IDs and boundaries", + _trigger_list_schema, ) + mcp_server.register_custom_tool( + "location_trigger_delete", + _delete_trigger, + "Delete a location trigger by ID", + _trigger_delete_schema, + is_write=True, + ) + + # -- Generic trigger create ----------------------------------------------- + + _generic_trigger_schema: dict[str, Any] = { + "type": "object", + "properties": { + "field": { + "type": "string", + "description": ( + "Telemetry field name (e.g. BatteryLevel, InsideTemp," + " OutsideTemp, Location, VehicleSpeed, Soc," + " ChargeState, Locked, Gear, EstBatteryRange)" + ), + }, + "operator": { + "type": "string", + "description": ( + "Comparison: lt, gt, lte, gte, eq, neq," + " changed, enter, leave" + ), + }, + "value": { + "description": ( + "Threshold value (number for comparisons," + " object with latitude/longitude/radius_m" + " for geofence)" + ), + }, + "once": { + "type": "boolean", + "description": ( + "Fire once then delete after delivery" + " (default false)" + ), + }, + "cooldown_seconds": { + "type": "number", + "description": ( + "Minimum seconds between fires (default 60)" + ), + }, + }, + "required": ["field", "operator"], + } + + mcp_server.register_custom_tool( + "trigger_create", + lambda p: _create_trigger(p.pop("field", ""), p), + "Create a trigger on any telemetry field", + _generic_trigger_schema, + is_write=True, + ) + + # -- Shared: poll -------------------------------------------------------- + + def _handle_poll(params: dict[str, Any]) -> dict[str, Any]: + notifications = trigger_manager.drain_pending() + return {"notifications": [n.model_dump(mode="json") for n in notifications]} + mcp_server.register_custom_tool( "trigger_poll", _handle_poll, - "Poll for fired trigger notifications", + "Poll for fired trigger notifications across all domains", + {"type": "object", "properties": {}}, + ) + + # -- Shared: list all triggers ------------------------------------------- + + def _handle_trigger_list(params: dict[str, Any]) -> dict[str, Any]: + triggers = trigger_manager.list_all() + result = [] + for t in triggers: + entry: dict[str, Any] = { + "id": t.id, + "field": t.condition.field, + "operator": t.condition.operator.value, + "value": t.condition.value, + "once": t.once, + "cooldown_seconds": t.cooldown_seconds, + } + result.append(entry) + return {"triggers": result} + + mcp_server.register_custom_tool( + "trigger_list", + _handle_trigger_list, + "List all triggers across all domains", {"type": "object", "properties": {}}, ) + # -- Shared: telemetry_get ----------------------------------------------- + + def _handle_telemetry_get(params: dict[str, Any]) -> dict[str, Any]: + field = params.get("field", "") + if not field: + raise ValueError("telemetry_get requires 'field' parameter") + if telemetry_store is None: + return {"field": field, "pending": True} + snap = telemetry_store.get(field) + if snap is None: + return {"field": field, "pending": True} + return {"field": field, "value": snap.value} + + mcp_server.register_custom_tool( + "telemetry_get", + _handle_telemetry_get, + "Read the latest value of any telemetry field", + { + "type": "object", + "properties": { + "field": { + "type": "string", + "description": ( + "Telemetry field name" + " (e.g. PackVoltage, HvacFanSpeed)" + ), + }, + }, + "required": ["field"], + }, + ) + class _LoggingASGI: """Thin ASGI wrapper that logs every HTTP and WebSocket request.""" diff --git a/src/tescmd/openclaw/bridge.py b/src/tescmd/openclaw/bridge.py index a315099..c82038e 100644 --- a/src/tescmd/openclaw/bridge.py +++ b/src/tescmd/openclaw/bridge.py @@ -8,6 +8,7 @@ import logging import time +from collections import deque from dataclasses import dataclass from datetime import UTC, datetime from typing import TYPE_CHECKING, Any @@ -27,6 +28,7 @@ _RECONNECT_BASE = 5.0 _RECONNECT_MAX = 120.0 +_MAX_PENDING_PUSH = 1000 class TelemetryBridge: @@ -65,6 +67,7 @@ def __init__( self._reconnect_at: float = 0.0 self._reconnect_backoff: float = _RECONNECT_BASE self._shutting_down = False + self._pending_push: deque[Any] = deque(maxlen=_MAX_PENDING_PUSH) @property def event_count(self) -> int: @@ -102,6 +105,9 @@ async def _maybe_reconnect(self) -> None: await self.send_connected() except Exception: logger.warning("Failed to send connected event after reconnect", exc_info=True) + # Flush any queued trigger notifications now that WS is available. + if self._pending_push: + await self.flush_pending_push() def _build_lifecycle_event(self, event_type: str) -> dict[str, Any]: """Build a ``req:agent`` lifecycle event (connecting/disconnecting).""" @@ -117,42 +123,114 @@ def _build_lifecycle_event(self, event_type: str) -> dict[str, Any]: } def make_trigger_push_callback(self) -> Any: - """Return an async callback that pushes trigger notifications to the gateway. + """Return an async callback that pushes trigger notifications. - Suitable for passing to ``TriggerManager.add_on_fire()``. Returns - ``None`` when in dry-run mode (caller should skip registration). + Delivery is via WebSocket (``gateway.send_event``). On failure + the notification is queued in ``_pending_push`` for later + :meth:`flush_pending_push`. + + One-shot triggers (``n.once is True``) are deleted from the + trigger manager only after confirmed delivery — guaranteeing the + notification reaches the gateway before the trigger disappears. + + Returns ``None`` only when in dry-run mode (caller should skip + registration). """ if self._dry_run: return None gateway = self._gateway - client_id = self._client_id + pending_push = self._pending_push + trigger_manager = self._trigger_manager async def _push_trigger_notification(n: Any) -> None: if gateway.is_connected: - try: - await gateway.send_event( - { - "method": "req:agent", - "params": { - "event_type": "trigger.fired", - "source": client_id, - "vin": n.vin, - "timestamp": n.fired_at.isoformat(), - "data": n.model_dump(mode="json"), - }, - } - ) - except Exception: - logger.warning( - "Failed to push trigger notification (trigger=%s field=%s)", + await gateway.send_event( + { + "method": "tescmd.trigger.fired", + "params": { + "trigger_id": n.trigger_id, + "field": n.field, + "operator": n.operator.value, + "value": n.value, + "vin": n.vin, + "fired_at": n.fired_at.isoformat(), + }, + } + ) + # send_event() never raises — check is_connected to detect failure. + if gateway.is_connected: + logger.info( + "Pushed trigger notification: trigger=%s", n.trigger_id, - n.field, - exc_info=True, ) + if n.once and trigger_manager is not None: + trigger_manager.delete(n.trigger_id) + logger.info( + "Deleted one-shot trigger %s after confirmed delivery", + n.trigger_id, + ) + return + logger.warning( + "WS push failed for trigger=%s", + n.trigger_id, + ) + + pending_push.append(n) + logger.warning( + "Trigger notification queued: trigger=%s", n.trigger_id, + ) return _push_trigger_notification + async def flush_pending_push(self) -> int: + """Replay queued trigger notifications via WebSocket. + + One-shot triggers are deleted after each successful send, + mirroring the behaviour of the push callback. + + Returns the number of notifications successfully flushed. + """ + if not self._pending_push or self._dry_run: + return 0 + if not self._gateway.is_connected: + return 0 + + total = len(self._pending_push) + sent = 0 + + while self._pending_push: + n = self._pending_push[0] # peek without removing + await self._gateway.send_event( + { + "method": "tescmd.trigger.fired", + "params": { + "trigger_id": n.trigger_id, + "field": n.field, + "operator": n.operator.value, + "value": n.value, + "vin": n.vin, + "fired_at": n.fired_at.isoformat(), + }, + } + ) + # send_event() never raises — check is_connected to detect failure. + if not self._gateway.is_connected: + logger.warning("Flush stopped at %d/%d", sent, total) + break + self._pending_push.popleft() + sent += 1 + if n.once and self._trigger_manager is not None: + self._trigger_manager.delete(n.trigger_id) + logger.info( + "Deleted one-shot trigger %s after flush delivery", + n.trigger_id, + ) + + if sent: + logger.info("Flushed %d/%d queued trigger notification(s)", sent, total) + return sent + async def send_connected(self) -> bool: """Send a ``node.connected`` lifecycle event to the gateway. @@ -193,62 +271,108 @@ async def send_disconnecting(self) -> None: except Exception: logger.warning("Failed to send disconnecting event", exc_info=True) + async def _send_datum( + self, + datum: Any, + frame: TelemetryFrame, + now: float, + ) -> bool: + """Emit a single datum as a gateway event. + + Returns ``True`` if the event was sent (or printed in dry-run mode). + Handles reconnection and error logging internally. + """ + event = self._emitter.to_event( + field_name=datum.field_name, + value=datum.value, + vin=frame.vin, + timestamp=frame.created_at, + ) + + if event is None: + self._drop_count += 1 + return False + + self._filter.record_emit(datum.field_name, datum.value, now) + self._event_count += 1 + self._last_event_time = now + + if self._dry_run: + import json + + print(json.dumps(event, default=str), flush=True) + return True + + if not self._gateway.is_connected: + await self._maybe_reconnect() + if not self._gateway.is_connected: + self._drop_count += 1 + return False + + try: + await self._gateway.send_event(event) + logger.info("Sent %s event for %s", datum.field_name, frame.vin) + return True + except Exception: + logger.warning( + "Failed to send event for %s — discarding", + datum.field_name, + exc_info=True, + ) + return False + async def on_frame(self, frame: TelemetryFrame) -> None: """Process a decoded telemetry frame through the filter pipeline. For each datum in the frame, check the dual-gate filter. If it passes, transform to an OpenClaw event and send to the gateway. + + Trigger evaluation runs on **all** datums regardless of the filter. + When a trigger fires on a datum that was blocked by the filter, the + datum is force-emitted so the gateway always receives the value that + caused the trigger to fire. + Failed sends are logged and discarded — never crash the server. If the gateway is disconnected, a reconnection attempt is made (with exponential backoff) before dropping events. """ now = time.monotonic() - + trigger_count = ( + len(self._trigger_manager.list_all()) if self._trigger_manager is not None else 0 + ) + logger.debug( + "on_frame: vin=%s datums=%d connected=%s triggers=%d", + frame.vin, + len(frame.data), + self._gateway.is_connected, + trigger_count, + ) + + # --- Phase 1: filtered telemetry emission --- + emitted = 0 + emitted_fields: set[str] = set() for datum in frame.data: if not self._filter.should_emit(datum.field_name, datum.value, now): self._drop_count += 1 continue - event = self._emitter.to_event( - field_name=datum.field_name, - value=datum.value, - vin=frame.vin, - timestamp=frame.created_at, - ) - - if event is None: - self._drop_count += 1 - continue - - self._filter.record_emit(datum.field_name, datum.value, now) - self._event_count += 1 - self._last_event_time = now - - if self._dry_run: - import json + if await self._send_datum(datum, frame, now): + emitted += 1 + emitted_fields.add(datum.field_name) - print(json.dumps(event, default=str), flush=True) - continue - - if not self._gateway.is_connected: - await self._maybe_reconnect() - if not self._gateway.is_connected: - self._drop_count += 1 - continue - - try: - await self._gateway.send_event(event) - logger.info("Sent %s event for %s", datum.field_name, frame.vin) - except Exception: - logger.warning( - "Failed to send event for %s — discarding", - datum.field_name, - exc_info=True, - ) + if emitted > 0 or logger.isEnabledFor(logging.DEBUG): + logger.debug( + "Frame summary: emitted=%d dropped=%d total_events=%d", + emitted, + self._drop_count, + self._event_count, + ) - # Update telemetry store and evaluate triggers for ALL datums - # (not just filtered ones) so read handlers always see the latest - # values and triggers fire on every change. + # --- Phase 2: telemetry store + trigger evaluation --- + # Runs on ALL datums (not just filtered ones) so read handlers + # always see the latest values and triggers fire on every change. + # When a trigger fires on a datum that wasn't emitted in phase 1, + # force-emit it so the gateway receives the triggering value. if self._telemetry_store is not None or self._trigger_manager is not None: for datum in frame.data: prev_snap = ( @@ -262,9 +386,22 @@ async def on_frame(self, frame: TelemetryFrame) -> None: self._telemetry_store.update(datum.field_name, datum.value, frame.created_at) if self._trigger_manager is not None: - await self._trigger_manager.evaluate( + fired = await self._trigger_manager.evaluate( datum.field_name, datum.value, prev_value, frame.created_at ) + # Force-emit the telemetry event when a trigger fires + # but the filter gate blocked it in phase 1. + if ( + fired + and datum.field_name not in emitted_fields + and await self._send_datum(datum, frame, now) + ): + emitted_fields.add(datum.field_name) + logger.info( + "Force-emitted %s for %s (trigger fired)", + datum.field_name, + frame.vin, + ) # -- Pipeline factory ------------------------------------------------------- diff --git a/src/tescmd/openclaw/config.py b/src/tescmd/openclaw/config.py index adb0e9e..1a25a6f 100644 --- a/src/tescmd/openclaw/config.py +++ b/src/tescmd/openclaw/config.py @@ -31,9 +31,14 @@ class NodeCapabilities(BaseModel): reads: list[str] = [ "location.get", + "telemetry.get", + "trigger.list", + "trigger.poll", ] writes: list[str] = [ "system.run", + "trigger.create", + "trigger.delete", ] @property @@ -75,23 +80,32 @@ class FieldFilter(BaseModel): """ throttle_seconds: float = Field(default=1.0, ge=0) """Minimum seconds between emissions for this field.""" + max_seconds: float = Field(default=0.0, ge=0) + """Maximum seconds of silence before forcing emission regardless of delta. + + A value of ``0`` disables the staleness gate (only delta + throttle apply). + When set, ensures periodic updates even when values barely change — + useful for numeric fields on an idle/parked vehicle. + """ -# Default filter configurations per PRD +# Default filter configurations — low granularity thresholds so events +# flow frequently while still deduplicating truly identical values. _DEFAULT_FILTERS: dict[str, FieldFilter] = { - "Location": FieldFilter(granularity=50.0, throttle_seconds=1.0), - "Soc": FieldFilter(granularity=5.0, throttle_seconds=10.0), - "InsideTemp": FieldFilter(granularity=5.0, throttle_seconds=30.0), - "OutsideTemp": FieldFilter(granularity=5.0, throttle_seconds=30.0), - "VehicleSpeed": FieldFilter(granularity=5.0, throttle_seconds=2.0), + "Location": FieldFilter(granularity=5.0, throttle_seconds=1.0, max_seconds=60.0), + "Soc": FieldFilter(granularity=0.5, throttle_seconds=10.0, max_seconds=120.0), + "InsideTemp": FieldFilter(granularity=0.5, throttle_seconds=10.0, max_seconds=60.0), + "OutsideTemp": FieldFilter(granularity=0.5, throttle_seconds=10.0, max_seconds=60.0), + "VehicleSpeed": FieldFilter(granularity=1.0, throttle_seconds=2.0, max_seconds=30.0), "ChargeState": FieldFilter(granularity=0.0, throttle_seconds=0.0), "DetailedChargeState": FieldFilter(granularity=0.0, throttle_seconds=0.0), "Locked": FieldFilter(granularity=0.0, throttle_seconds=0.0), "SentryMode": FieldFilter(granularity=0.0, throttle_seconds=0.0), - "BatteryLevel": FieldFilter(granularity=1.0, throttle_seconds=10.0), - "EstBatteryRange": FieldFilter(granularity=5.0, throttle_seconds=30.0), - "Odometer": FieldFilter(granularity=1.0, throttle_seconds=60.0), + "BatteryLevel": FieldFilter(granularity=0.1, throttle_seconds=10.0, max_seconds=120.0), + "EstBatteryRange": FieldFilter(granularity=1.0, throttle_seconds=10.0, max_seconds=120.0), + "Odometer": FieldFilter(granularity=0.1, throttle_seconds=30.0, max_seconds=300.0), "Gear": FieldFilter(granularity=0.0, throttle_seconds=0.0), + "DefrostMode": FieldFilter(granularity=0.0, throttle_seconds=0.0), } diff --git a/src/tescmd/openclaw/dispatcher.py b/src/tescmd/openclaw/dispatcher.py index b962f76..f5701dd 100644 --- a/src/tescmd/openclaw/dispatcher.py +++ b/src/tescmd/openclaw/dispatcher.py @@ -9,6 +9,7 @@ from __future__ import annotations import asyncio +import contextlib import logging from typing import TYPE_CHECKING, Any @@ -28,6 +29,17 @@ logger = logging.getLogger(__name__) + +def _fahrenheit_to_celsius(f: float) -> float: + """Convert Fahrenheit to Celsius, rounded to 1 decimal place.""" + return round((f - 32.0) * 5.0 / 9.0, 1) + + +def _celsius_to_fahrenheit(c: float) -> float: + """Convert Celsius to Fahrenheit, rounded to 1 decimal place.""" + return round(c * 9.0 / 5.0 + 32.0, 1) + + # API snake_case → OpenClaw dot notation aliases for system.run _METHOD_ALIASES: dict[str, str] = { "door_lock": "door.lock", @@ -35,6 +47,7 @@ "auto_conditioning_start": "climate.on", "auto_conditioning_stop": "climate.off", "set_temps": "climate.set_temp", + "set_preconditioning_max": "climate.defrost", "charge_start": "charge.start", "charge_stop": "charge.stop", "set_charge_limit": "charge.set_limit", @@ -46,6 +59,7 @@ "navigation_sc_request": "nav.supercharger", "navigation_waypoints_request": "nav.waypoints", "trigger_homelink": "homelink.trigger", + "list_triggers": "trigger.list", } @@ -84,12 +98,14 @@ def __init__( "speed.get": self._handle_speed_get, "charge_state.get": self._handle_charge_state_get, "security.get": self._handle_security_get, + "telemetry.get": self._handle_telemetry_get, # Writes "door.lock": self._handle_door_lock, "door.unlock": self._handle_door_unlock, "climate.on": self._handle_climate_on, "climate.off": self._handle_climate_off, "climate.set_temp": self._handle_climate_set_temp, + "climate.defrost": self._handle_climate_defrost, "charge.start": self._handle_charge_start, "charge.stop": self._handle_charge_stop, "charge.set_limit": self._handle_charge_set_limit, @@ -105,16 +121,24 @@ def __init__( "nav.waypoints": self._handle_nav_waypoints, "homelink.trigger": self._handle_homelink, "system.run": self._handle_system_run, - # Trigger commands + # Shared trigger operations "trigger.create": self._handle_trigger_create, - "trigger.delete": self._handle_trigger_delete, - "trigger.list": self._handle_trigger_list, + "trigger.list": self._handle_trigger_list_all, "trigger.poll": self._handle_trigger_poll, - # Convenience trigger aliases + "trigger.delete": self._handle_trigger_delete, + # Domain-specific trigger CRUD "cabin_temp.trigger": self._handle_cabin_temp_trigger, + "cabin_temp.trigger.list": self._handle_cabin_temp_list, + "cabin_temp.trigger.delete": self._handle_trigger_delete, "outside_temp.trigger": self._handle_outside_temp_trigger, + "outside_temp.trigger.list": self._handle_outside_temp_list, + "outside_temp.trigger.delete": self._handle_trigger_delete, "battery.trigger": self._handle_battery_trigger, + "battery.trigger.list": self._handle_battery_list, + "battery.trigger.delete": self._handle_trigger_delete, "location.trigger": self._handle_location_trigger, + "location.trigger.list": self._handle_location_list, + "location.trigger.delete": self._handle_trigger_delete, } async def dispatch(self, msg: dict[str, Any]) -> dict[str, Any] | None: @@ -285,6 +309,16 @@ async def _handle_security_get(self, params: dict[str, Any]) -> dict[str, Any]: "sentry_mode": vs.get("sentry_mode"), } + async def _handle_telemetry_get(self, params: dict[str, Any]) -> dict[str, Any]: + """Read the latest value of any telemetry field from the store.""" + field = params.get("field", "") + if not field: + raise ValueError("telemetry.get requires 'field' parameter") + value = self._store_get(field) + if value is None: + return {"field": field, "pending": True} + return {"field": field, "value": value} + # -- Write helpers ------------------------------------------------------- async def _auto_wake(self, operation: Any) -> Any: @@ -350,6 +384,12 @@ async def _handle_climate_set_temp(self, params: dict[str, Any]) -> dict[str, An "set_temps", {"driver_temp": temp_f, "passenger_temp": temp_f} ) + async def _handle_climate_defrost(self, params: dict[str, Any]) -> dict[str, Any]: + on = params.get("on", True) + return await self._simple_command( + "set_preconditioning_max", {"on": bool(on), "manual_override": True} + ) + async def _handle_charge_start(self, params: dict[str, Any]) -> dict[str, Any]: return await self._simple_command("charge_start") @@ -454,7 +494,13 @@ def _require_trigger_manager(self) -> TriggerManager: return self._trigger_manager async def _handle_trigger_create(self, params: dict[str, Any]) -> dict[str, Any]: - """Create a new trigger from the given condition parameters.""" + """Create a new trigger from the given condition parameters. + + After creation, immediately evaluates the trigger against the + current telemetry store value. If the condition is already + satisfied, the event is fired and the trigger is deleted — the + response includes ``"immediate": True`` so the caller knows. + """ mgr = self._require_trigger_manager() field = params.get("field", "") if not field: @@ -476,12 +522,36 @@ async def _handle_trigger_create(self, params: dict[str, Any]) -> dict[str, Any] cooldown_seconds=params.get("cooldown_seconds", 60.0), ) created = mgr.create(trigger) - return { + + result: dict[str, Any] = { "id": created.id, "field": created.condition.field, "operator": created.condition.operator.value, } + # Immediate evaluation: if the telemetry store already has a + # value for this field that satisfies the condition, fire now. + # One-shot triggers are NOT deleted here — the push callback + # handles deletion after confirmed WebSocket delivery. + if self._store is not None: + snap = self._store.get(field) + if snap is not None: + from datetime import UTC, datetime + + fired = await mgr.evaluate_single( + created.id, snap.value, None, datetime.now(UTC) + ) + if fired: + result["immediate"] = True + logger.info( + "Trigger %s fired immediately (value=%s)%s", + created.id, + snap.value, + " — once, pending delivery" if created.once else " — kept (persistent)", + ) + + return result + async def _handle_trigger_delete(self, params: dict[str, Any]) -> dict[str, Any]: """Delete a trigger by ID.""" mgr = self._require_trigger_manager() @@ -491,23 +561,60 @@ async def _handle_trigger_delete(self, params: dict[str, Any]) -> dict[str, Any] deleted = mgr.delete(trigger_id) return {"deleted": deleted, "id": trigger_id} - async def _handle_trigger_list(self, params: dict[str, Any]) -> dict[str, Any]: - """List all registered triggers.""" + def _list_triggers_for_field( + self, field: str, *, show_fahrenheit: bool = False + ) -> dict[str, Any]: + """List triggers filtered to a specific telemetry field. + + When *show_fahrenheit* is ``True``, a ``value_f`` display field is + added for numeric thresholds (temperature triggers stored in °C). + """ + mgr = self._require_trigger_manager() + triggers = [t for t in mgr.list_all() if t.condition.field == field] + result = [] + for t in triggers: + entry: dict[str, Any] = { + "id": t.id, + "field": t.condition.field, + "operator": t.condition.operator.value, + "value": t.condition.value, + "once": t.once, + "cooldown_seconds": t.cooldown_seconds, + } + if show_fahrenheit and t.condition.value is not None: + with contextlib.suppress(TypeError, ValueError): + entry["value_f"] = _celsius_to_fahrenheit(float(t.condition.value)) + result.append(entry) + return {"triggers": result} + + async def _handle_cabin_temp_list(self, params: dict[str, Any]) -> dict[str, Any]: + return self._list_triggers_for_field("InsideTemp", show_fahrenheit=True) + + async def _handle_outside_temp_list(self, params: dict[str, Any]) -> dict[str, Any]: + return self._list_triggers_for_field("OutsideTemp", show_fahrenheit=True) + + async def _handle_battery_list(self, params: dict[str, Any]) -> dict[str, Any]: + return self._list_triggers_for_field("BatteryLevel") + + async def _handle_location_list(self, params: dict[str, Any]) -> dict[str, Any]: + return self._list_triggers_for_field("Location") + + async def _handle_trigger_list_all(self, params: dict[str, Any]) -> dict[str, Any]: + """List ALL triggers across all fields.""" mgr = self._require_trigger_manager() triggers = mgr.list_all() - return { - "triggers": [ - { - "id": t.id, - "field": t.condition.field, - "operator": t.condition.operator.value, - "value": t.condition.value, - "once": t.once, - "cooldown_seconds": t.cooldown_seconds, - } - for t in triggers - ] - } + result = [] + for t in triggers: + entry: dict[str, Any] = { + "id": t.id, + "field": t.condition.field, + "operator": t.condition.operator.value, + "value": t.condition.value, + "once": t.once, + "cooldown_seconds": t.cooldown_seconds, + } + result.append(entry) + return {"triggers": result} async def _handle_trigger_poll(self, params: dict[str, Any]) -> dict[str, Any]: """Drain and return pending trigger notifications.""" @@ -518,10 +625,24 @@ async def _handle_trigger_poll(self, params: dict[str, Any]) -> dict[str, Any]: # -- Convenience trigger aliases ----------------------------------------- async def _handle_cabin_temp_trigger(self, params: dict[str, Any]) -> dict[str, Any]: - return await self._handle_trigger_create({**params, "field": "InsideTemp"}) + converted = {**params, "field": "InsideTemp"} + if "value" in converted and converted["value"] is not None: + f_val = float(converted["value"]) + converted["value"] = _fahrenheit_to_celsius(f_val) + logger.debug( + "cabin_temp.trigger: converted %.1f°F → %.1f°C", f_val, converted["value"] + ) + return await self._handle_trigger_create(converted) async def _handle_outside_temp_trigger(self, params: dict[str, Any]) -> dict[str, Any]: - return await self._handle_trigger_create({**params, "field": "OutsideTemp"}) + converted = {**params, "field": "OutsideTemp"} + if "value" in converted and converted["value"] is not None: + f_val = float(converted["value"]) + converted["value"] = _fahrenheit_to_celsius(f_val) + logger.debug( + "outside_temp.trigger: converted %.1f°F → %.1f°C", f_val, converted["value"] + ) + return await self._handle_trigger_create(converted) async def _handle_battery_trigger(self, params: dict[str, Any]) -> dict[str, Any]: return await self._handle_trigger_create({**params, "field": "BatteryLevel"}) diff --git a/src/tescmd/openclaw/emitter.py b/src/tescmd/openclaw/emitter.py index 735ecaa..9081155 100644 --- a/src/tescmd/openclaw/emitter.py +++ b/src/tescmd/openclaw/emitter.py @@ -15,10 +15,28 @@ from __future__ import annotations +import re from datetime import UTC, datetime from typing import Any +def _to_snake_case(name: str) -> str: + """Convert PascalCase field name to snake_case event type. + + Examples:: + + >>> _to_snake_case("PackVoltage") + 'pack_voltage' + >>> _to_snake_case("TpmsPressureFl") + 'tpms_pressure_fl' + >>> _to_snake_case("HvacFanSpeed") + 'hvac_fan_speed' + """ + s = re.sub(r"([A-Z]+)([A-Z][a-z])", r"\1_\2", name) + s = re.sub(r"([a-z\d])([A-Z])", r"\1_\2", s) + return s.lower() + + def _celsius_to_fahrenheit(c: float) -> float: return c * 9.0 / 5.0 + 32.0 @@ -26,7 +44,10 @@ def _celsius_to_fahrenheit(c: float) -> float: class EventEmitter: """Stateless transformer: telemetry datum → OpenClaw req:agent payload. - Returns ``None`` for fields that don't map to an event type. + Explicitly mapped fields (Location, Soc, temps, charge state, etc.) + use domain-specific formatting. All other fields fall through to a + generic handler that produces a snake_case event type from the + PascalCase field name. """ def __init__(self, client_id: str = "node-host") -> None: @@ -83,7 +104,8 @@ def _build_payload(self, field_name: str, value: Any) -> dict[str, Any] | None: return self._range_payload(value) if field_name == "Gear": return self._gear_payload(value) - return None + # Generic fallback — any field not explicitly mapped above + return self._generic_payload(field_name, value) def _location_payload(self, value: Any) -> dict[str, Any] | None: try: @@ -173,3 +195,11 @@ def _gear_payload(self, value: Any) -> dict[str, Any] | None: "_event_type": "gear_changed", "gear": str(value), } + + def _generic_payload(self, field_name: str, value: Any) -> dict[str, Any]: + """Produce a generic event payload for any unmapped telemetry field.""" + return { + "_event_type": _to_snake_case(field_name), + "field": field_name, + "value": value, + } diff --git a/src/tescmd/openclaw/filters.py b/src/tescmd/openclaw/filters.py index 88eb2b9..a9c6e16 100644 --- a/src/tescmd/openclaw/filters.py +++ b/src/tescmd/openclaw/filters.py @@ -6,6 +6,10 @@ threshold since the last emitted value. 2. **Throttle gate** — enough time has elapsed since the last emission. +An optional **staleness gate** (``max_seconds``) overrides the delta gate +when too much time passes without emission — ensuring periodic updates for +numeric fields that change slowly (e.g. parked vehicle). + Fields with ``granularity=0`` emit on any value change (state fields like ``ChargeState``, ``Locked``). Fields with ``throttle_seconds=0`` have no time constraint. @@ -14,10 +18,17 @@ from __future__ import annotations import math -from typing import TYPE_CHECKING, Any +from typing import Any + +from tescmd.openclaw.config import FieldFilter -if TYPE_CHECKING: - from tescmd.openclaw.config import FieldFilter +# Sensible defaults for telemetry fields that have no explicit filter +# configuration. Any-change delta, 5-second throttle, 2-minute staleness. +_DEFAULT_FALLBACK = FieldFilter( + granularity=0.0, + throttle_seconds=5.0, + max_seconds=120.0, +) def haversine(lat1: float, lon1: float, lat2: float, lon2: float) -> float: @@ -81,19 +92,35 @@ def __init__(self, filters: dict[str, FieldFilter]) -> None: self._last_emit_times: dict[str, float] = {} def should_emit(self, field: str, value: Any, now: float) -> bool: - """Check whether a field value passes both gates. + """Check whether a field value passes the filter gates. + + Gate evaluation order: + + 1. **Config gate** — field must be configured and enabled. + 2. **Throttle gate** — minimum interval since last emission. + 3. **Staleness gate** — if ``max_seconds > 0`` and that much time + has elapsed since the last emission, force through regardless + of the delta gate. Prevents prolonged silence for numeric + fields that change slowly (e.g. parked vehicle). + 4. **Delta gate** — value must have changed beyond granularity. Returns ``True`` if the value should be emitted downstream. """ cfg = self._filters.get(field) - if cfg is None or not cfg.enabled: + if cfg is not None and not cfg.enabled: return False + if cfg is None: + cfg = _DEFAULT_FALLBACK + + last_time = self._last_emit_times.get(field) # Throttle gate: enforce minimum interval - if cfg.throttle_seconds > 0: - last_time = self._last_emit_times.get(field) - if last_time is not None and (now - last_time) < cfg.throttle_seconds: - return False + if ( + cfg.throttle_seconds > 0 + and last_time is not None + and (now - last_time) < cfg.throttle_seconds + ): + return False # Delta gate: value must have changed beyond granularity last_value = self._last_values.get(field) @@ -101,6 +128,10 @@ def should_emit(self, field: str, value: Any, now: float) -> bool: # First value for this field — always emit return True + # Staleness gate: force emission after max_seconds of silence + if cfg.max_seconds > 0 and last_time is not None and (now - last_time) >= cfg.max_seconds: + return True + if field in _LOCATION_FIELDS: delta = _location_delta(last_value, value) else: diff --git a/src/tescmd/telemetry/tui.py b/src/tescmd/telemetry/tui.py index e591d78..5e9ef87 100644 --- a/src/tescmd/telemetry/tui.py +++ b/src/tescmd/telemetry/tui.py @@ -27,6 +27,7 @@ from tescmd.output.rich_output import DisplayUnits from tescmd.telemetry.decoder import TelemetryFrame + from tescmd.triggers.manager import TriggerManager logger = logging.getLogger(__name__) @@ -429,10 +430,20 @@ class TelemetryTUI(App[None]): grid-size: 2; grid-gutter: 0; } - #activity-log { + #sidebar { width: 1fr; min-width: 30; height: 1fr; + } + #triggers-table { + height: auto; + min-height: 5; + max-height: 12; + border: solid #e67e22; + padding: 0; + } + #activity-log { + height: 1fr; border: solid $accent; padding: 0 1; } @@ -556,6 +567,10 @@ def __init__( self._activity_file_handler: logging.FileHandler | None = None self._activity_log_path: str = "" + # Trigger manager reference (set by serve.py after construction). + self._trigger_manager: TriggerManager | None = None + self._trigger_table_keys: set[str] = set() + # Debug logger for command attempts — always writes to a file. self._cmd_logger = logging.getLogger("tescmd.tui.commands") self._cmd_log_handler: logging.FileHandler | None = None @@ -571,7 +586,9 @@ def compose(self) -> ComposeResult: yield DataTable( id=f"{panel_id}-table", cursor_type="none", zebra_stripes=True ) - yield RichLog(id="activity-log", wrap=True, markup=True) + with Vertical(id="sidebar"): + yield DataTable(id="triggers-table", cursor_type="none") + yield RichLog(id="activity-log", wrap=True, markup=True) yield Static(id="server-bar") yield Static(id="command-status") yield Footer() @@ -586,6 +603,13 @@ def on_mount(self) -> None: table.add_column("Field", key="field", width=24) table.add_column("Value", key="value", width=24) + # Triggers table. + triggers_table = self.query_one("#triggers-table", DataTable) + triggers_table.border_title = "Triggers" + triggers_table.add_column("Field", key="field", width=14) + triggers_table.add_column("Condition", key="condition", width=10) + triggers_table.add_column("Type", key="type", width=6) + # Activity sidebar title. activity_log = self.query_one("#activity-log", RichLog) activity_log.border_title = "Activity" @@ -759,7 +783,7 @@ async def push_frame(self, frame: TelemetryFrame) -> None: self._queue.put_nowait(frame) async def _process_queue(self) -> None: - """Background worker: drain the queue and update state.""" + """Background worker: drain the queue and update state + widgets.""" while True: try: frame = await asyncio.wait_for(self._queue.get(), timeout=0.5) @@ -774,6 +798,7 @@ async def _process_queue(self) -> None: for datum in frame.data: self._state[datum.field_name] = datum.value self._timestamps[datum.field_name] = frame.created_at + self._update_panel_cell(datum.field_name, datum.value) # -- Server info setters (called from serve.py) --------------------------- @@ -796,13 +821,16 @@ def set_cache_stats(self, frames: int, fields: int, pending: int) -> None: def set_log_path(self, path: Path | str) -> None: self._log_path = str(path) + def set_trigger_manager(self, mgr: TriggerManager) -> None: + self._trigger_manager = mgr + # -- UI update (runs every 1 second) -------------------------------------- def _update_ui(self) -> None: """Refresh all visible widgets from current state.""" self._update_header() - self._update_panels() self._update_server_info() + self._update_triggers() self._maybe_log_frame_summary() def _update_header(self) -> None: @@ -821,20 +849,18 @@ def _update_header(self) -> None: f"Frames: {self._frame_count:,} Up: {hours:02d}:{minutes:02d}:{seconds:02d}" ) - def _update_panels(self) -> None: - """Route each field to its categorized panel DataTable.""" - for field_name in sorted(self._state.keys()): - panel_id = _FIELD_TO_PANEL.get(field_name, "diagnostics") - value = self._state[field_name] - display_value = _format_value(field_name, value, self._units) - tracked = self._panel_table_fields[panel_id] + def _update_panel_cell(self, field_name: str, value: Any) -> None: + """Update a single field's cell in the appropriate panel DataTable.""" + panel_id = _FIELD_TO_PANEL.get(field_name, "diagnostics") + display_value = _format_value(field_name, value, self._units) + tracked = self._panel_table_fields[panel_id] - table = self.query_one(f"#{panel_id}-table", DataTable) - if field_name in tracked: - table.update_cell(field_name, "value", display_value) - else: - table.add_row(field_name, display_value, key=field_name) - tracked.add(field_name) + table = self.query_one(f"#{panel_id}-table", DataTable) + if field_name in tracked: + table.update_cell(field_name, "value", display_value) + else: + table.add_row(field_name, display_value, key=field_name) + tracked.add(field_name) def _update_server_info(self) -> None: parts: list[str] = [] @@ -862,6 +888,44 @@ def _update_server_info(self) -> None: info_widget.update("") info_widget.display = False + def _update_triggers(self) -> None: + """Refresh the triggers DataTable from the trigger manager.""" + if self._trigger_manager is None: + return + + table = self.query_one("#triggers-table", DataTable) + triggers = self._trigger_manager.list_all() + current_ids = {t.id for t in triggers} + + # Remove rows for deleted triggers. + removed = self._trigger_table_keys - current_ids + for tid in removed: + table.remove_row(tid) + self._trigger_table_keys -= removed + + # Add or update rows. + for t in triggers: + cond = t.condition + op = cond.operator.value + if op == "changed": + condition_text = "changed" + elif op in ("enter", "leave"): + condition_text = f"{op} geofence" + else: + symbols = { + "lt": "<", "gt": ">", "lte": "\u2264", + "gte": "\u2265", "eq": "=", "neq": "\u2260", + } + condition_text = f"{symbols.get(op, op)} {cond.value}" + fire_type = "once" if t.once else "persist" + + if t.id in self._trigger_table_keys: + table.update_cell(t.id, "condition", condition_text) + table.update_cell(t.id, "type", fire_type) + else: + table.add_row(cond.field, condition_text, fire_type, key=t.id) + self._trigger_table_keys.add(t.id) + # -- Command execution (keybinding actions) -------------------------------- def _show_command_status(self, text: str) -> None: diff --git a/src/tescmd/triggers/manager.py b/src/tescmd/triggers/manager.py index ab77b29..daf45f5 100644 --- a/src/tescmd/triggers/manager.py +++ b/src/tescmd/triggers/manager.py @@ -48,6 +48,7 @@ def __init__(self, vin: str) -> None: self._last_fire_times: dict[str, float] = {} self._pending: deque[TriggerNotification] = deque(maxlen=MAX_PENDING) self._on_fire_callbacks: list[Callable[[TriggerNotification], Awaitable[None]]] = [] + self._fired_once_ids: set[str] = set() def create(self, trigger: TriggerDefinition) -> TriggerDefinition: """Register a trigger. Returns the trigger with its assigned ID. @@ -84,6 +85,7 @@ def delete(self, trigger_id: str) -> bool: if not ids: del self._field_index[field] self._last_fire_times.pop(trigger_id, None) + self._fired_once_ids.discard(trigger_id) logger.info("Deleted trigger %s", trigger_id) return True @@ -97,33 +99,121 @@ def drain_pending(self) -> list[TriggerNotification]: self._pending.clear() return result + @property + def vin(self) -> str: + """Vehicle Identification Number included in notifications.""" + return self._vin + + def queue_notification(self, notification: TriggerNotification) -> None: + """Queue a notification for MCP polling via :meth:`drain_pending`.""" + self._pending.append(notification) + + def mark_fired_once(self, trigger_id: str) -> None: + """Mark a one-shot trigger as fired (pending delivery). + + The trigger stays in ``list_all()`` but is skipped in + ``evaluate()`` so it won't fire again. Call :meth:`delete` + after confirming delivery to clean up. + """ + if trigger_id in self._triggers: + self._fired_once_ids.add(trigger_id) + def add_on_fire(self, callback: Callable[[TriggerNotification], Awaitable[None]]) -> None: """Register an async callback invoked when a trigger fires.""" self._on_fire_callbacks.append(callback) + async def evaluate_single( + self, + trigger_id: str, + value: Any, + previous_value: Any, + timestamp: datetime, + ) -> bool: + """Evaluate a single trigger against *value*. + + Returns ``True`` if the trigger fired. Does **not** auto-delete + one-shot triggers — the caller decides whether to delete. + + Used for immediate evaluation at creation time so the response + can indicate the condition was already satisfied. + """ + trigger = self._triggers.get(trigger_id) + if trigger is None: + return False + + if not _matches(trigger.condition, value, previous_value): + return False + + now = time.monotonic() + self._last_fire_times[trigger_id] = now + notification = TriggerNotification( + trigger_id=trigger_id, + field=trigger.condition.field, + operator=trigger.condition.operator, + threshold=trigger.condition.value, + value=value, + previous_value=previous_value, + fired_at=timestamp, + vin=self._vin, + once=trigger.once, + ) + + logger.info( + "Trigger %s fired (immediate): %s %s %s (value=%s)", + trigger_id, + trigger.condition.field, + trigger.condition.operator.value, + trigger.condition.value, + value, + ) + + if trigger.once: + self._fired_once_ids.add(trigger_id) + + self._pending.append(notification) + + for callback in self._on_fire_callbacks: + try: + await callback(notification) + except Exception: + logger.warning("Trigger fire callback failed for %s", trigger_id, exc_info=True) + + return True + async def evaluate( self, field: str, value: Any, previous_value: Any, timestamp: datetime, - ) -> None: + ) -> bool: """Evaluate all triggers registered for *field*. + Returns ``True`` if at least one trigger fired for this field/value. + + One-shot triggers are **not** deleted here. Instead they are + tracked in ``_fired_once_ids`` and skipped on subsequent + evaluations. The push callback (or caller) is responsible for + calling :meth:`delete` after confirming delivery. + Called by the bridge after capturing the previous value and before updating the telemetry store. """ trigger_ids = self._field_index.get(field) if not trigger_ids: - return + return False + fired = False now = time.monotonic() - # Iterate over a copy — one-shot triggers mutate the set for tid in list(trigger_ids): trigger = self._triggers.get(tid) if trigger is None: continue + # Skip one-shot triggers that already fired (pending delivery) + if tid in self._fired_once_ids: + continue + # Cooldown check for persistent triggers if not trigger.once: last_fire = self._last_fire_times.get(tid) @@ -134,6 +224,7 @@ async def evaluate( continue # Fire! + fired = True self._last_fire_times[tid] = now notification = TriggerNotification( trigger_id=tid, @@ -144,6 +235,7 @@ async def evaluate( previous_value=previous_value, fired_at=timestamp, vin=self._vin, + once=trigger.once, ) logger.info( @@ -156,6 +248,12 @@ async def evaluate( previous_value, ) + # Mark one-shot triggers as fired (pending delivery). + # Deletion is deferred until the push callback confirms + # successful WebSocket delivery. + if trigger.once: + self._fired_once_ids.add(tid) + self._pending.append(notification) for callback in self._on_fire_callbacks: @@ -164,9 +262,7 @@ async def evaluate( except Exception: logger.warning("Trigger fire callback failed for %s", tid, exc_info=True) - # Auto-delete one-shot triggers - if trigger.once: - self.delete(tid) + return fired def _matches(condition: TriggerCondition, value: Any, previous_value: Any) -> bool: diff --git a/src/tescmd/triggers/models.py b/src/tescmd/triggers/models.py index 81753c4..bb7a449 100644 --- a/src/tescmd/triggers/models.py +++ b/src/tescmd/triggers/models.py @@ -91,3 +91,4 @@ class TriggerNotification(BaseModel): previous_value: Any = None fired_at: datetime = Field(default_factory=lambda: datetime.now(UTC)) vin: str = "" + once: bool = False diff --git a/tests/openclaw/test_bridge.py b/tests/openclaw/test_bridge.py index 9ec6997..6329668 100644 --- a/tests/openclaw/test_bridge.py +++ b/tests/openclaw/test_bridge.py @@ -16,7 +16,12 @@ from tescmd.openclaw.telemetry_store import TelemetryStore from tescmd.telemetry.decoder import TelemetryDatum, TelemetryFrame from tescmd.triggers.manager import TriggerManager -from tescmd.triggers.models import TriggerCondition, TriggerDefinition, TriggerOperator +from tescmd.triggers.models import ( + TriggerCondition, + TriggerDefinition, + TriggerNotification, + TriggerOperator, +) def _make_frame( @@ -68,13 +73,14 @@ async def test_emit_mapped_datum( assert sent["params"]["event_type"] == "battery" @pytest.mark.asyncio - async def test_drop_unmapped_datum(self, bridge: TelemetryBridge) -> None: + async def test_unmapped_datum_emits_generic_event(self, bridge: TelemetryBridge) -> None: frame = _make_frame(data=[TelemetryDatum("UnknownField", 999, 42, "int")]) await bridge.on_frame(frame) - # UnknownField is not in the filter config, so it should be dropped - assert bridge.event_count == 0 - assert bridge.drop_count == 1 + # UnknownField has no explicit filter config but the default + # fallback allows it through, producing a generic event. + assert bridge.event_count == 1 + assert bridge.drop_count == 0 @pytest.mark.asyncio async def test_multiple_data_in_frame( @@ -584,7 +590,8 @@ async def test_trigger_callback_invoked_from_bridge(self, gateway: GatewayClient """Trigger fire callback is invoked when frame causes a trigger to fire.""" store = TelemetryStore() mgr = TriggerManager(vin="VIN1") - cond = TriggerCondition(field="InsideTemp", operator=TriggerOperator.GT, value=100) + # Threshold 35°C (~95°F) — realistic cabin temperature in Celsius + cond = TriggerCondition(field="InsideTemp", operator=TriggerOperator.GT, value=35) mgr.create(TriggerDefinition(condition=cond)) fired: list[object] = [] @@ -601,19 +608,23 @@ async def test_trigger_callback_invoked_from_bridge(self, gateway: GatewayClient trigger_manager=mgr, ) - # First frame — establishes baseline - frame1 = _make_frame(data=[TelemetryDatum("InsideTemp", 3, 95.0, "float")]) + # First frame — 30°C (86°F), below threshold + frame1 = _make_frame(data=[TelemetryDatum("InsideTemp", 3, 30.0, "float")]) await bridge.on_frame(frame1) assert len(fired) == 0 - # Second frame — crosses threshold - frame2 = _make_frame(data=[TelemetryDatum("InsideTemp", 3, 105.0, "float")]) + # Second frame — 40°C (104°F), crosses threshold + frame2 = _make_frame(data=[TelemetryDatum("InsideTemp", 3, 40.0, "float")]) await bridge.on_frame(frame2) assert len(fired) == 1 @pytest.mark.asyncio async def test_end_to_end_threshold_crossing(self, gateway: GatewayClient) -> None: - """Full pipeline: frame → store update → trigger fire → notification.""" + """Full pipeline: frame → store update → trigger fire → notification. + + One-shot trigger fires once but stays registered (pending delivery). + It only fires once despite subsequent matching frames. + """ store = TelemetryStore() mgr = TriggerManager(vin="VIN1") @@ -644,6 +655,391 @@ async def test_end_to_end_threshold_crossing(self, gateway: GatewayClient) -> No assert len(pending) == 1 assert pending[0].value == 10.0 assert pending[0].previous_value == 20.0 + assert pending[0].once is True + + # Trigger stays registered (pending delivery confirmation) + assert len(mgr.list_all()) == 1 + + @pytest.mark.asyncio + async def test_trigger_force_emits_filtered_datum(self, gateway: GatewayClient) -> None: + """When a trigger fires on a datum blocked by the filter, force-emit it.""" + store = TelemetryStore() + mgr = TriggerManager(vin="VIN1") + cond = TriggerCondition(field="BatteryLevel", operator=TriggerOperator.LT, value=20) + mgr.create(TriggerDefinition(condition=cond)) + + # High granularity so small changes are blocked by the delta gate + filters = {"BatteryLevel": FieldFilter(granularity=50.0, throttle_seconds=0.0)} + filt = DualGateFilter(filters) + emitter = EventEmitter(client_id="test") + bridge = TelemetryBridge( + gateway, + filt, + emitter, + telemetry_store=store, + trigger_manager=mgr, + vin="VIN1", + client_id="test", + ) + + # First frame: above threshold — emitted (first value always passes) + frame1 = _make_frame(data=[TelemetryDatum("BatteryLevel", 3, 25.0, "float")]) + await bridge.on_frame(frame1) + assert gateway._ws.send.call_count == 1 # filter passed (first value) + + # Second frame: below threshold, small delta (10 < 50 granularity) + # Filter would block it, but trigger fires → force-emit + frame2 = _make_frame(data=[TelemetryDatum("BatteryLevel", 3, 15.0, "float")]) + await bridge.on_frame(frame2) + # 1 (filter-passed) + 1 (force-emitted due to trigger) = 2 + assert gateway._ws.send.call_count == 2 + + @pytest.mark.asyncio + async def test_trigger_no_double_emit(self, gateway: GatewayClient) -> None: + """When a trigger fires on a datum already emitted by the filter, don't double-send.""" + store = TelemetryStore() + mgr = TriggerManager(vin="VIN1") + cond = TriggerCondition(field="BatteryLevel", operator=TriggerOperator.LT, value=20) + mgr.create(TriggerDefinition(condition=cond)) + + # Granularity 0 so everything passes the delta gate + filters = {"BatteryLevel": FieldFilter(granularity=0.0, throttle_seconds=0.0)} + filt = DualGateFilter(filters) + emitter = EventEmitter(client_id="test") + bridge = TelemetryBridge( + gateway, + filt, + emitter, + telemetry_store=store, + trigger_manager=mgr, + vin="VIN1", + client_id="test", + ) + + # First frame: above threshold + frame1 = _make_frame(data=[TelemetryDatum("BatteryLevel", 3, 25.0, "float")]) + await bridge.on_frame(frame1) + assert gateway._ws.send.call_count == 1 + + # Second frame: below threshold — filter passes AND trigger fires + # Should only emit once (no double-send) + frame2 = _make_frame(data=[TelemetryDatum("BatteryLevel", 3, 15.0, "float")]) + await bridge.on_frame(frame2) + assert gateway._ws.send.call_count == 2 # exactly 1 more, not 2 + + +class TestTriggerPushCallback: + """Tests for the WS-based trigger push callback and flush.""" + + def _make_notification( + self, trigger_id: str = "t1", field: str = "BatteryLevel", value: float = 15.0 + ) -> TriggerNotification: + return TriggerNotification( + trigger_id=trigger_id, + field=field, + operator=TriggerOperator.LT, + threshold=20, + value=value, + previous_value=25.0, + fired_at=datetime(2026, 2, 1, 12, 0, 0, tzinfo=UTC), + vin="VIN1", + ) + + @pytest.mark.asyncio + async def test_push_callback_sends_via_ws(self, gateway: GatewayClient) -> None: + """Push callback sends via WS when gateway is connected.""" + mgr = TriggerManager(vin="VIN1") + filters: dict[str, FieldFilter] = {} + filt = DualGateFilter(filters) + emitter = EventEmitter(client_id="test") + bridge = TelemetryBridge( + gateway, filt, emitter, vin="VIN1", client_id="test-client", + trigger_manager=mgr, + ) + + cb = bridge.make_trigger_push_callback() + assert cb is not None + + gateway._ws = AsyncMock() + gateway._ws.send = AsyncMock() + await cb(self._make_notification()) + + gateway._ws.send.assert_awaited_once() + assert len(bridge._pending_push) == 0 + + @pytest.mark.asyncio + async def test_push_callback_queues_when_disconnected( + self, gateway: GatewayClient, + ) -> None: + """Push callback queues when gateway is disconnected.""" + gateway._connected = False + gateway._ws = None + filters: dict[str, FieldFilter] = {} + filt = DualGateFilter(filters) + emitter = EventEmitter(client_id="test") + bridge = TelemetryBridge( + gateway, filt, emitter, vin="VIN1", client_id="test-client", + ) + + cb = bridge.make_trigger_push_callback() + assert cb is not None + + await cb(self._make_notification()) + assert len(bridge._pending_push) == 1 + + @pytest.mark.asyncio + async def test_push_callback_queues_on_ws_failure( + self, gateway: GatewayClient, + ) -> None: + """Push callback queues when gateway is connected but send_event raises.""" + filters: dict[str, FieldFilter] = {} + filt = DualGateFilter(filters) + emitter = EventEmitter(client_id="test") + bridge = TelemetryBridge( + gateway, filt, emitter, vin="VIN1", client_id="test-client", + ) + + cb = bridge.make_trigger_push_callback() + assert cb is not None + + gateway._ws = AsyncMock() + gateway._ws.send = AsyncMock(side_effect=ConnectionError("broken")) + await cb(self._make_notification()) + + assert len(bridge._pending_push) == 1 + + @pytest.mark.asyncio + async def test_flush_sends_queued_via_ws( + self, gateway: GatewayClient, + ) -> None: + """flush_pending_push sends via WS when gateway is connected.""" + filters: dict[str, FieldFilter] = {} + filt = DualGateFilter(filters) + emitter = EventEmitter(client_id="test") + bridge = TelemetryBridge( + gateway, filt, emitter, vin="VIN1", client_id="test-client", + ) + + bridge._pending_push.append(self._make_notification("t1")) + bridge._pending_push.append(self._make_notification("t2")) + + gateway._ws = AsyncMock() + gateway._ws.send = AsyncMock() - # Trigger should be auto-deleted (one-shot) + sent = await bridge.flush_pending_push() + assert sent == 2 + assert len(bridge._pending_push) == 0 + assert gateway._ws.send.await_count == 2 + + @pytest.mark.asyncio + async def test_flush_stops_on_ws_failure( + self, gateway: GatewayClient, + ) -> None: + """flush_pending_push stops when WS send fails mid-flush.""" + filters: dict[str, FieldFilter] = {} + filt = DualGateFilter(filters) + emitter = EventEmitter(client_id="test") + bridge = TelemetryBridge( + gateway, filt, emitter, vin="VIN1", client_id="test-client", + ) + + bridge._pending_push.append(self._make_notification("t1")) + bridge._pending_push.append(self._make_notification("t2")) + + gateway._ws = AsyncMock() + call_count = 0 + + async def _send_side_effect(data: str) -> None: + nonlocal call_count + call_count += 1 + if call_count >= 2: + raise ConnectionError("broken") + + gateway._ws.send = AsyncMock(side_effect=_send_side_effect) + + sent = await bridge.flush_pending_push() + assert sent == 1 + assert len(bridge._pending_push) == 1 + + @pytest.mark.asyncio + async def test_flush_noop_when_empty(self, gateway: GatewayClient) -> None: + """flush_pending_push returns 0 when queue is empty.""" + filters: dict[str, FieldFilter] = {} + filt = DualGateFilter(filters) + emitter = EventEmitter(client_id="test") + bridge = TelemetryBridge( + gateway, filt, emitter, vin="VIN1", client_id="test-client", + ) + + sent = await bridge.flush_pending_push() + assert sent == 0 + + @pytest.mark.asyncio + async def test_flush_noop_when_disconnected(self, gateway: GatewayClient) -> None: + """flush_pending_push returns 0 when gateway is disconnected.""" + gateway._connected = False + filters: dict[str, FieldFilter] = {} + filt = DualGateFilter(filters) + emitter = EventEmitter(client_id="test") + bridge = TelemetryBridge( + gateway, filt, emitter, vin="VIN1", client_id="test-client", + ) + + bridge._pending_push.append(self._make_notification("t1")) + + sent = await bridge.flush_pending_push() + assert sent == 0 + assert len(bridge._pending_push) == 1 + + @pytest.mark.asyncio + async def test_push_callback_deletes_once_trigger_on_delivery( + self, gateway: GatewayClient, + ) -> None: + """Push callback deletes one-shot trigger after confirmed WS delivery.""" + mgr = TriggerManager(vin="VIN1") + cond = TriggerCondition(field="BatteryLevel", operator=TriggerOperator.LT, value=20) + t = mgr.create(TriggerDefinition(condition=cond, once=True)) + + filters: dict[str, FieldFilter] = {} + filt = DualGateFilter(filters) + emitter = EventEmitter(client_id="test") + bridge = TelemetryBridge( + gateway, filt, emitter, vin="VIN1", client_id="test-client", + trigger_manager=mgr, + ) + + cb = bridge.make_trigger_push_callback() + assert cb is not None + + gateway._ws = AsyncMock() + gateway._ws.send = AsyncMock() + + n = self._make_notification(trigger_id=t.id) + # Simulate once=True on notification (as set by evaluate) + n = TriggerNotification( + trigger_id=t.id, field="BatteryLevel", operator=TriggerOperator.LT, + threshold=20, value=15.0, previous_value=25.0, + fired_at=datetime(2026, 2, 1, 12, 0, 0, tzinfo=UTC), + vin="VIN1", once=True, + ) + await cb(n) + + # Trigger should be deleted after confirmed delivery assert len(mgr.list_all()) == 0 + assert len(bridge._pending_push) == 0 + + @pytest.mark.asyncio + async def test_push_callback_keeps_once_trigger_on_failure( + self, gateway: GatewayClient, + ) -> None: + """Push callback queues notification and keeps trigger when WS fails.""" + mgr = TriggerManager(vin="VIN1") + cond = TriggerCondition(field="BatteryLevel", operator=TriggerOperator.LT, value=20) + t = mgr.create(TriggerDefinition(condition=cond, once=True)) + + gateway._connected = False + gateway._ws = None + filters: dict[str, FieldFilter] = {} + filt = DualGateFilter(filters) + emitter = EventEmitter(client_id="test") + bridge = TelemetryBridge( + gateway, filt, emitter, vin="VIN1", client_id="test-client", + trigger_manager=mgr, + ) + + cb = bridge.make_trigger_push_callback() + assert cb is not None + + n = TriggerNotification( + trigger_id=t.id, field="BatteryLevel", operator=TriggerOperator.LT, + threshold=20, value=15.0, previous_value=25.0, + fired_at=datetime(2026, 2, 1, 12, 0, 0, tzinfo=UTC), + vin="VIN1", once=True, + ) + await cb(n) + + # Trigger stays — delivery not confirmed + assert len(mgr.list_all()) == 1 + assert len(bridge._pending_push) == 1 + + @pytest.mark.asyncio + async def test_flush_deletes_once_trigger_on_delivery( + self, gateway: GatewayClient, + ) -> None: + """flush_pending_push deletes one-shot trigger after confirmed delivery.""" + mgr = TriggerManager(vin="VIN1") + cond = TriggerCondition(field="BatteryLevel", operator=TriggerOperator.LT, value=20) + t = mgr.create(TriggerDefinition(condition=cond, once=True)) + + filters: dict[str, FieldFilter] = {} + filt = DualGateFilter(filters) + emitter = EventEmitter(client_id="test") + bridge = TelemetryBridge( + gateway, filt, emitter, vin="VIN1", client_id="test-client", + trigger_manager=mgr, + ) + + n = TriggerNotification( + trigger_id=t.id, field="BatteryLevel", operator=TriggerOperator.LT, + threshold=20, value=15.0, previous_value=25.0, + fired_at=datetime(2026, 2, 1, 12, 0, 0, tzinfo=UTC), + vin="VIN1", once=True, + ) + bridge._pending_push.append(n) + + gateway._ws = AsyncMock() + gateway._ws.send = AsyncMock() + + sent = await bridge.flush_pending_push() + assert sent == 1 + assert len(bridge._pending_push) == 0 + # Trigger deleted after flush delivery + assert len(mgr.list_all()) == 0 + + @pytest.mark.asyncio + async def test_flush_keeps_persistent_trigger( + self, gateway: GatewayClient, + ) -> None: + """flush_pending_push does NOT delete persistent triggers.""" + mgr = TriggerManager(vin="VIN1") + cond = TriggerCondition(field="BatteryLevel", operator=TriggerOperator.LT, value=20) + t = mgr.create(TriggerDefinition(condition=cond, once=False)) + + filters: dict[str, FieldFilter] = {} + filt = DualGateFilter(filters) + emitter = EventEmitter(client_id="test") + bridge = TelemetryBridge( + gateway, filt, emitter, vin="VIN1", client_id="test-client", + trigger_manager=mgr, + ) + + n = TriggerNotification( + trigger_id=t.id, field="BatteryLevel", operator=TriggerOperator.LT, + threshold=20, value=15.0, previous_value=25.0, + fired_at=datetime(2026, 2, 1, 12, 0, 0, tzinfo=UTC), + vin="VIN1", once=False, + ) + bridge._pending_push.append(n) + + gateway._ws = AsyncMock() + gateway._ws.send = AsyncMock() + + sent = await bridge.flush_pending_push() + assert sent == 1 + # Persistent trigger stays + assert len(mgr.list_all()) == 1 + + @pytest.mark.asyncio + async def test_push_callback_none_in_dry_run( + self, gateway: GatewayClient, + ) -> None: + """make_trigger_push_callback returns None in dry-run mode.""" + filters: dict[str, FieldFilter] = {} + filt = DualGateFilter(filters) + emitter = EventEmitter(client_id="test") + bridge = TelemetryBridge( + gateway, filt, emitter, dry_run=True, vin="VIN1", client_id="test", + ) + + cb = bridge.make_trigger_push_callback() + assert cb is None diff --git a/tests/openclaw/test_config.py b/tests/openclaw/test_config.py index aa0e6a0..fcc27bc 100644 --- a/tests/openclaw/test_config.py +++ b/tests/openclaw/test_config.py @@ -42,7 +42,7 @@ def test_default_filters_populated(self) -> None: cfg = BridgeConfig() assert "Location" in cfg.telemetry assert "Soc" in cfg.telemetry - assert cfg.telemetry["Location"].granularity == 50.0 + assert cfg.telemetry["Location"].granularity == 5.0 assert cfg.telemetry["Location"].throttle_seconds == 1.0 def test_default_charge_state_filter(self) -> None: @@ -116,8 +116,13 @@ def test_merge_none_keeps_original(self) -> None: class TestNodeCapabilities: def test_defaults(self) -> None: caps = NodeCapabilities() - assert caps.reads == ["location.get"] - assert caps.writes == ["system.run"] + assert "location.get" in caps.reads + assert "telemetry.get" in caps.reads + assert "trigger.list" in caps.reads + assert "trigger.poll" in caps.reads + assert "system.run" in caps.writes + assert "trigger.create" in caps.writes + assert "trigger.delete" in caps.writes def test_custom_reads(self) -> None: caps = NodeCapabilities(reads=["custom.read"]) @@ -169,8 +174,10 @@ class TestBridgeConfigCapabilities: def test_default_capabilities(self) -> None: cfg = BridgeConfig() assert isinstance(cfg.capabilities, NodeCapabilities) - assert len(cfg.capabilities.reads) == 1 - assert len(cfg.capabilities.writes) == 1 + assert len(cfg.capabilities.reads) == 4 + assert len(cfg.capabilities.writes) == 3 + assert "telemetry.get" in cfg.capabilities.reads + assert "trigger.create" in cfg.capabilities.writes def test_custom_capabilities_from_json(self, tmp_path: Path) -> None: config_file = tmp_path / "bridge.json" diff --git a/tests/openclaw/test_dispatcher.py b/tests/openclaw/test_dispatcher.py index 537ff04..e913d97 100644 --- a/tests/openclaw/test_dispatcher.py +++ b/tests/openclaw/test_dispatcher.py @@ -133,6 +133,43 @@ async def test_security_get_from_store(self) -> None: assert result["sentry_mode"] is False +class TestTelemetryGetHandler: + """Tests for the generic telemetry.get read handler.""" + + @pytest.mark.asyncio + async def test_telemetry_get_from_store(self) -> None: + ctx = _mock_app_ctx() + store = _store_with(PackVoltage=398.2) + d = CommandDispatcher(vin="V", app_ctx=ctx, telemetry_store=store) + result = await d.dispatch({"method": "telemetry.get", "params": {"field": "PackVoltage"}}) + assert result["field"] == "PackVoltage" + assert result["value"] == 398.2 + + @pytest.mark.asyncio + async def test_telemetry_get_pending(self) -> None: + ctx = _mock_app_ctx() + store = TelemetryStore() + d = CommandDispatcher(vin="V", app_ctx=ctx, telemetry_store=store) + result = await d.dispatch({"method": "telemetry.get", "params": {"field": "PackVoltage"}}) + assert result["field"] == "PackVoltage" + assert result["pending"] is True + + @pytest.mark.asyncio + async def test_telemetry_get_missing_field_raises(self) -> None: + ctx = _mock_app_ctx() + d = CommandDispatcher(vin="V", app_ctx=ctx, telemetry_store=TelemetryStore()) + with pytest.raises(ValueError, match="requires 'field'"): + await d.dispatch({"method": "telemetry.get", "params": {}}) + + @pytest.mark.asyncio + async def test_telemetry_get_no_store_returns_pending(self) -> None: + ctx = _mock_app_ctx() + d = CommandDispatcher(vin="V", app_ctx=ctx, telemetry_store=None) + result = await d.dispatch({"method": "telemetry.get", "params": {"field": "PackVoltage"}}) + assert result["field"] == "PackVoltage" + assert result["pending"] is True + + class TestReadHandlersColdStart: """Read handlers return pending when store and cache are both empty.""" @@ -282,6 +319,79 @@ async def test_climate_set_temp_missing_param(self) -> None: with pytest.raises(ValueError, match="requires 'temp'"): await d.dispatch({"method": "climate.set_temp", "params": {}}) + @pytest.mark.asyncio + async def test_climate_defrost(self) -> None: + ctx = _mock_app_ctx() + d = CommandDispatcher(vin="VIN1", app_ctx=ctx) + cmd_result = _make_command_result() + mock_client = AsyncMock() + mock_client.close = AsyncMock() + mock_cmd_api = AsyncMock() + mock_cmd_api.set_preconditioning_max = AsyncMock(return_value=cmd_result) + + with ( + patch( + "tescmd.openclaw.dispatcher.get_command_api", + return_value=(mock_client, AsyncMock(), mock_cmd_api), + ), + patch("tescmd.openclaw.dispatcher.invalidate_cache_for_vin"), + patch("tescmd.openclaw.dispatcher.check_command_guards"), + ): + result = await d.dispatch({"method": "climate.defrost", "params": {"on": True}}) + assert result["result"] is True + mock_cmd_api.set_preconditioning_max.assert_awaited_once_with( + "VIN1", on=True, manual_override=True + ) + + @pytest.mark.asyncio + async def test_climate_defrost_off(self) -> None: + ctx = _mock_app_ctx() + d = CommandDispatcher(vin="VIN1", app_ctx=ctx) + cmd_result = _make_command_result() + mock_client = AsyncMock() + mock_client.close = AsyncMock() + mock_cmd_api = AsyncMock() + mock_cmd_api.set_preconditioning_max = AsyncMock(return_value=cmd_result) + + with ( + patch( + "tescmd.openclaw.dispatcher.get_command_api", + return_value=(mock_client, AsyncMock(), mock_cmd_api), + ), + patch("tescmd.openclaw.dispatcher.invalidate_cache_for_vin"), + patch("tescmd.openclaw.dispatcher.check_command_guards"), + ): + result = await d.dispatch({"method": "climate.defrost", "params": {"on": False}}) + assert result["result"] is True + mock_cmd_api.set_preconditioning_max.assert_awaited_once_with( + "VIN1", on=False, manual_override=True + ) + + @pytest.mark.asyncio + async def test_climate_defrost_defaults_on(self) -> None: + ctx = _mock_app_ctx() + d = CommandDispatcher(vin="VIN1", app_ctx=ctx) + cmd_result = _make_command_result() + mock_client = AsyncMock() + mock_client.close = AsyncMock() + mock_cmd_api = AsyncMock() + mock_cmd_api.set_preconditioning_max = AsyncMock(return_value=cmd_result) + + with ( + patch( + "tescmd.openclaw.dispatcher.get_command_api", + return_value=(mock_client, AsyncMock(), mock_cmd_api), + ), + patch("tescmd.openclaw.dispatcher.invalidate_cache_for_vin"), + patch("tescmd.openclaw.dispatcher.check_command_guards"), + ): + # No 'on' param — defaults to True + result = await d.dispatch({"method": "climate.defrost", "params": {}}) + assert result["result"] is True + mock_cmd_api.set_preconditioning_max.assert_awaited_once_with( + "VIN1", on=True, manual_override=True + ) + @pytest.mark.asyncio async def test_charge_set_limit(self) -> None: ctx = _mock_app_ctx() @@ -844,14 +954,14 @@ class TestTriggerHandlers: """Tests for trigger.* command handlers.""" @pytest.mark.asyncio - async def test_trigger_create(self) -> None: + async def test_trigger_create_via_alias(self) -> None: ctx = _mock_app_ctx() mgr = TriggerManager(vin="VIN1") d = CommandDispatcher(vin="VIN1", app_ctx=ctx, trigger_manager=mgr) result = await d.dispatch( { - "method": "trigger.create", - "params": {"field": "BatteryLevel", "operator": "lt", "value": 20}, + "method": "battery.trigger", + "params": {"operator": "lt", "value": 20}, } ) assert result is not None @@ -867,8 +977,8 @@ async def test_trigger_create_once(self) -> None: d = CommandDispatcher(vin="VIN1", app_ctx=ctx, trigger_manager=mgr) await d.dispatch( { - "method": "trigger.create", - "params": {"field": "Soc", "operator": "lt", "value": 10, "once": True}, + "method": "battery.trigger", + "params": {"operator": "lt", "value": 10, "once": True}, } ) trigger = mgr.list_all()[0] @@ -881,59 +991,59 @@ async def test_trigger_create_custom_cooldown(self) -> None: d = CommandDispatcher(vin="VIN1", app_ctx=ctx, trigger_manager=mgr) await d.dispatch( { - "method": "trigger.create", - "params": { - "field": "Soc", - "operator": "lt", - "value": 10, - "cooldown_seconds": 30.0, - }, + "method": "battery.trigger", + "params": {"operator": "lt", "value": 10, "cooldown_seconds": 30.0}, } ) trigger = mgr.list_all()[0] assert trigger.cooldown_seconds == 30.0 @pytest.mark.asyncio - async def test_trigger_create_missing_field_raises(self) -> None: + async def test_trigger_create_missing_operator_raises(self) -> None: ctx = _mock_app_ctx() mgr = TriggerManager(vin="VIN1") d = CommandDispatcher(vin="VIN1", app_ctx=ctx, trigger_manager=mgr) - with pytest.raises(ValueError, match="requires 'field'"): + with pytest.raises(ValueError, match="requires 'operator'"): await d.dispatch( { - "method": "trigger.create", - "params": {"operator": "lt", "value": 20}, + "method": "battery.trigger", + "params": {"value": 20}, } ) @pytest.mark.asyncio - async def test_trigger_create_missing_operator_raises(self) -> None: + async def test_generic_trigger_create_routes(self) -> None: + """trigger.create routes to the generic create handler.""" ctx = _mock_app_ctx() mgr = TriggerManager(vin="VIN1") d = CommandDispatcher(vin="VIN1", app_ctx=ctx, trigger_manager=mgr) - with pytest.raises(ValueError, match="requires 'operator'"): - await d.dispatch( - { - "method": "trigger.create", - "params": {"field": "BatteryLevel", "value": 20}, - } - ) + result = await d.dispatch( + { + "method": "trigger.create", + "params": {"field": "BatteryLevel", "operator": "lt", "value": 20}, + } + ) + assert result is not None + assert result["field"] == "BatteryLevel" + assert result["operator"] == "lt" + assert "id" in result + assert len(mgr.list_all()) == 1 @pytest.mark.asyncio - async def test_trigger_delete(self) -> None: + async def test_trigger_delete_via_domain(self) -> None: ctx = _mock_app_ctx() mgr = TriggerManager(vin="VIN1") d = CommandDispatcher(vin="VIN1", app_ctx=ctx, trigger_manager=mgr) create_result = await d.dispatch( { - "method": "trigger.create", - "params": {"field": "BatteryLevel", "operator": "lt", "value": 20}, + "method": "battery.trigger", + "params": {"operator": "lt", "value": 20}, } ) trigger_id = create_result["id"] delete_result = await d.dispatch( { - "method": "trigger.delete", + "method": "battery.trigger.delete", "params": {"id": trigger_id}, } ) @@ -948,7 +1058,7 @@ async def test_trigger_delete_nonexistent(self) -> None: d = CommandDispatcher(vin="VIN1", app_ctx=ctx, trigger_manager=mgr) result = await d.dispatch( { - "method": "trigger.delete", + "method": "battery.trigger.delete", "params": {"id": "nonexistent123"}, } ) @@ -960,37 +1070,95 @@ async def test_trigger_delete_missing_id_raises(self) -> None: mgr = TriggerManager(vin="VIN1") d = CommandDispatcher(vin="VIN1", app_ctx=ctx, trigger_manager=mgr) with pytest.raises(ValueError, match="requires 'id'"): - await d.dispatch({"method": "trigger.delete", "params": {}}) + await d.dispatch({"method": "battery.trigger.delete", "params": {}}) @pytest.mark.asyncio - async def test_trigger_list_empty(self) -> None: + async def test_generic_trigger_delete_routes(self) -> None: + """trigger.delete is routable as a domain-agnostic delete.""" ctx = _mock_app_ctx() mgr = TriggerManager(vin="VIN1") d = CommandDispatcher(vin="VIN1", app_ctx=ctx, trigger_manager=mgr) + + create_result = await d.dispatch( + {"method": "battery.trigger", "params": {"operator": "lt", "value": 20}} + ) + trigger_id = create_result["id"] + + result = await d.dispatch( + {"method": "trigger.delete", "params": {"id": trigger_id}} + ) + assert result["deleted"] is True + assert len(mgr.list_all()) == 0 + + @pytest.mark.asyncio + async def test_generic_trigger_list_all(self) -> None: + """trigger.list returns ALL triggers across all fields.""" + ctx = _mock_app_ctx() + mgr = TriggerManager(vin="VIN1") + d = CommandDispatcher(vin="VIN1", app_ctx=ctx, trigger_manager=mgr) + await d.dispatch( + {"method": "battery.trigger", "params": {"operator": "lt", "value": 20}} + ) + await d.dispatch( + {"method": "cabin_temp.trigger", "params": {"operator": "gt", "value": 95}} + ) result = await d.dispatch({"method": "trigger.list", "params": {}}) + assert result is not None + assert len(result["triggers"]) == 2 + fields = {t["field"] for t in result["triggers"]} + assert fields == {"BatteryLevel", "InsideTemp"} + + @pytest.mark.asyncio + async def test_domain_list_empty(self) -> None: + ctx = _mock_app_ctx() + mgr = TriggerManager(vin="VIN1") + d = CommandDispatcher(vin="VIN1", app_ctx=ctx, trigger_manager=mgr) + result = await d.dispatch({"method": "battery.trigger.list", "params": {}}) assert result["triggers"] == [] @pytest.mark.asyncio - async def test_trigger_list_returns_all(self) -> None: + async def test_domain_list_filters_by_field(self) -> None: + """Each domain list only shows triggers for its own field.""" ctx = _mock_app_ctx() mgr = TriggerManager(vin="VIN1") d = CommandDispatcher(vin="VIN1", app_ctx=ctx, trigger_manager=mgr) await d.dispatch( { - "method": "trigger.create", - "params": {"field": "BatteryLevel", "operator": "lt", "value": 20}, + "method": "battery.trigger", + "params": {"operator": "lt", "value": 20}, } ) await d.dispatch( { - "method": "trigger.create", - "params": {"field": "InsideTemp", "operator": "gt", "value": 100}, + "method": "cabin_temp.trigger", + "params": {"operator": "gt", "value": 95}, # 95°F → 35°C } ) - result = await d.dispatch({"method": "trigger.list", "params": {}}) - assert len(result["triggers"]) == 2 - fields = {t["field"] for t in result["triggers"]} - assert fields == {"BatteryLevel", "InsideTemp"} + # battery.trigger.list shows only BatteryLevel triggers + bat = await d.dispatch({"method": "battery.trigger.list", "params": {}}) + assert len(bat["triggers"]) == 1 + assert bat["triggers"][0]["field"] == "BatteryLevel" + # cabin_temp.trigger.list shows only InsideTemp triggers + temp = await d.dispatch({"method": "cabin_temp.trigger.list", "params": {}}) + assert len(temp["triggers"]) == 1 + assert temp["triggers"][0]["field"] == "InsideTemp" + + @pytest.mark.asyncio + async def test_temp_list_includes_fahrenheit(self) -> None: + """Temperature trigger list includes value_f for display.""" + ctx = _mock_app_ctx() + mgr = TriggerManager(vin="VIN1") + d = CommandDispatcher(vin="VIN1", app_ctx=ctx, trigger_manager=mgr) + await d.dispatch( + { + "method": "cabin_temp.trigger", + "params": {"operator": "gt", "value": 80}, # 80°F → 26.7°C + } + ) + result = await d.dispatch({"method": "cabin_temp.trigger.list", "params": {}}) + t = result["triggers"][0] + assert t["value"] == 26.7 # stored in Celsius + assert t["value_f"] == 80.1 # converted back to Fahrenheit @pytest.mark.asyncio async def test_trigger_poll_empty(self) -> None: @@ -1007,8 +1175,8 @@ async def test_trigger_poll_returns_notifications(self) -> None: d = CommandDispatcher(vin="VIN1", app_ctx=ctx, trigger_manager=mgr) await d.dispatch( { - "method": "trigger.create", - "params": {"field": "BatteryLevel", "operator": "lt", "value": 20}, + "method": "battery.trigger", + "params": {"operator": "lt", "value": 20}, } ) # Simulate trigger firing via evaluate @@ -1028,8 +1196,8 @@ async def test_trigger_poll_drains(self) -> None: d = CommandDispatcher(vin="VIN1", app_ctx=ctx, trigger_manager=mgr) await d.dispatch( { - "method": "trigger.create", - "params": {"field": "BatteryLevel", "operator": "lt", "value": 20}, + "method": "battery.trigger", + "params": {"operator": "lt", "value": 20}, } ) ts = datetime(2026, 2, 1, 12, 0, 0, tzinfo=UTC) @@ -1046,31 +1214,61 @@ class TestTriggerConvenienceAliases: """Tests for convenience trigger aliases that pre-fill field names.""" @pytest.mark.asyncio - async def test_cabin_temp_trigger(self) -> None: + async def test_cabin_temp_trigger_converts_f_to_c(self) -> None: + """cabin_temp.trigger converts Fahrenheit value to Celsius for storage.""" ctx = _mock_app_ctx() mgr = TriggerManager(vin="VIN1") d = CommandDispatcher(vin="VIN1", app_ctx=ctx, trigger_manager=mgr) result = await d.dispatch( { "method": "cabin_temp.trigger", - "params": {"operator": "gt", "value": 100}, + "params": {"operator": "gt", "value": 72}, # 72°F } ) assert result["field"] == "InsideTemp" assert result["operator"] == "gt" + # 72°F = 22.2°C — stored value should be Celsius + trigger = mgr.list_all()[0] + assert trigger.condition.value == 22.2 @pytest.mark.asyncio - async def test_outside_temp_trigger(self) -> None: + async def test_outside_temp_trigger_converts_f_to_c(self) -> None: + """outside_temp.trigger converts Fahrenheit value to Celsius for storage.""" ctx = _mock_app_ctx() mgr = TriggerManager(vin="VIN1") d = CommandDispatcher(vin="VIN1", app_ctx=ctx, trigger_manager=mgr) result = await d.dispatch( { "method": "outside_temp.trigger", - "params": {"operator": "lt", "value": 32}, + "params": {"operator": "lt", "value": 32}, # 32°F = 0°C } ) assert result["field"] == "OutsideTemp" + trigger = mgr.list_all()[0] + assert trigger.condition.value == 0.0 + + @pytest.mark.asyncio + async def test_cabin_temp_trigger_fires_with_celsius_telemetry(self) -> None: + """End-to-end: convenience alias trigger fires against real Celsius telemetry.""" + ctx = _mock_app_ctx() + mgr = TriggerManager(vin="VIN1") + d = CommandDispatcher(vin="VIN1", app_ctx=ctx, trigger_manager=mgr) + # User sets "alert me when cabin temp > 80°F" (= 26.7°C) + await d.dispatch( + { + "method": "cabin_temp.trigger", + "params": {"operator": "gt", "value": 80, "cooldown_seconds": 0}, + } + ) + ts = datetime(2026, 2, 1, 12, 0, 0, tzinfo=UTC) + # Telemetry reports 25°C (77°F) — below threshold, should NOT fire + await mgr.evaluate("InsideTemp", 25.0, 20.0, ts) + assert mgr.drain_pending() == [] + # Telemetry reports 30°C (86°F) — above threshold, SHOULD fire + await mgr.evaluate("InsideTemp", 30.0, 25.0, ts) + pending = mgr.drain_pending() + assert len(pending) == 1 + assert pending[0].value == 30.0 @pytest.mark.asyncio async def test_battery_trigger(self) -> None: @@ -1084,6 +1282,9 @@ async def test_battery_trigger(self) -> None: } ) assert result["field"] == "BatteryLevel" + # Battery is a percentage — no unit conversion + trigger = mgr.list_all()[0] + assert trigger.condition.value == 20 @pytest.mark.asyncio async def test_location_trigger(self) -> None: @@ -1113,8 +1314,8 @@ async def test_trigger_create_no_manager(self) -> None: with pytest.raises(RuntimeError, match="Triggers not available"): await d.dispatch( { - "method": "trigger.create", - "params": {"field": "BatteryLevel", "operator": "lt", "value": 20}, + "method": "battery.trigger", + "params": {"operator": "lt", "value": 20}, } ) @@ -1123,14 +1324,16 @@ async def test_trigger_delete_no_manager(self) -> None: ctx = _mock_app_ctx() d = CommandDispatcher(vin="VIN1", app_ctx=ctx) with pytest.raises(RuntimeError, match="Triggers not available"): - await d.dispatch({"method": "trigger.delete", "params": {"id": "abc"}}) + await d.dispatch( + {"method": "battery.trigger.delete", "params": {"id": "abc"}} + ) @pytest.mark.asyncio async def test_trigger_list_no_manager(self) -> None: ctx = _mock_app_ctx() d = CommandDispatcher(vin="VIN1", app_ctx=ctx) with pytest.raises(RuntimeError, match="Triggers not available"): - await d.dispatch({"method": "trigger.list", "params": {}}) + await d.dispatch({"method": "battery.trigger.list", "params": {}}) @pytest.mark.asyncio async def test_trigger_poll_no_manager(self) -> None: @@ -1150,3 +1353,128 @@ async def test_convenience_alias_no_manager(self) -> None: "params": {"operator": "lt", "value": 20}, } ) + + +class TestTriggerImmediateEvaluation: + """Triggers that match the current telemetry value fire immediately.""" + + @pytest.mark.asyncio + async def test_trigger_fires_immediately_when_condition_met(self) -> None: + ctx = _mock_app_ctx() + store = TelemetryStore() + mgr = TriggerManager(vin="VIN1") + d = CommandDispatcher( + vin="VIN1", app_ctx=ctx, telemetry_store=store, trigger_manager=mgr + ) + + # Seed the store with a current battery value of 15% + store.update("BatteryLevel", 15.0, datetime(2026, 2, 1, tzinfo=UTC)) + + result = await d.dispatch( + {"method": "battery.trigger", "params": {"operator": "lt", "value": 20}} + ) + + # Trigger fires immediately but stays registered (persistent by default) + assert result["immediate"] is True + assert len(mgr.list_all()) == 1 + # Notification should be queued for polling + pending = mgr.drain_pending() + assert len(pending) == 1 + assert pending[0].field == "BatteryLevel" + assert pending[0].value == 15.0 + + @pytest.mark.asyncio + async def test_once_trigger_stays_on_immediate_fire(self) -> None: + """One-shot trigger fires immediately but stays (pending delivery).""" + ctx = _mock_app_ctx() + store = TelemetryStore() + mgr = TriggerManager(vin="VIN1") + d = CommandDispatcher( + vin="VIN1", app_ctx=ctx, telemetry_store=store, trigger_manager=mgr + ) + + store.update("BatteryLevel", 15.0, datetime(2026, 2, 1, tzinfo=UTC)) + + result = await d.dispatch( + {"method": "battery.trigger", "params": {"operator": "lt", "value": 20, "once": True}} + ) + + # One-shot trigger fires immediately but is NOT deleted — + # the push callback deletes it after confirmed WS delivery. + assert result["immediate"] is True + assert len(mgr.list_all()) == 1 + + @pytest.mark.asyncio + async def test_trigger_not_immediate_when_condition_not_met(self) -> None: + ctx = _mock_app_ctx() + store = TelemetryStore() + mgr = TriggerManager(vin="VIN1") + d = CommandDispatcher( + vin="VIN1", app_ctx=ctx, telemetry_store=store, trigger_manager=mgr + ) + + # Battery is at 50% — trigger for < 20 should NOT fire + store.update("BatteryLevel", 50.0, datetime(2026, 2, 1, tzinfo=UTC)) + + result = await d.dispatch( + {"method": "battery.trigger", "params": {"operator": "lt", "value": 20}} + ) + + assert "immediate" not in result + assert len(mgr.list_all()) == 1 # trigger stays registered + + @pytest.mark.asyncio + async def test_trigger_not_immediate_when_no_telemetry(self) -> None: + ctx = _mock_app_ctx() + store = TelemetryStore() + mgr = TriggerManager(vin="VIN1") + d = CommandDispatcher( + vin="VIN1", app_ctx=ctx, telemetry_store=store, trigger_manager=mgr + ) + + # No telemetry data yet — trigger should register but not fire + result = await d.dispatch( + {"method": "battery.trigger", "params": {"operator": "lt", "value": 20}} + ) + + assert "immediate" not in result + assert len(mgr.list_all()) == 1 + + @pytest.mark.asyncio + async def test_trigger_not_immediate_without_store(self) -> None: + ctx = _mock_app_ctx() + mgr = TriggerManager(vin="VIN1") + d = CommandDispatcher( + vin="VIN1", app_ctx=ctx, telemetry_store=None, trigger_manager=mgr + ) + + result = await d.dispatch( + {"method": "battery.trigger", "params": {"operator": "lt", "value": 20}} + ) + + assert "immediate" not in result + assert len(mgr.list_all()) == 1 + + @pytest.mark.asyncio + async def test_immediate_fires_push_callback(self) -> None: + """Immediate evaluation calls registered fire callbacks.""" + ctx = _mock_app_ctx() + store = TelemetryStore() + mgr = TriggerManager(vin="VIN1") + d = CommandDispatcher( + vin="VIN1", app_ctx=ctx, telemetry_store=store, trigger_manager=mgr + ) + + cb = AsyncMock() + mgr.add_on_fire(cb) + + store.update("BatteryLevel", 15.0, datetime(2026, 2, 1, tzinfo=UTC)) + + await d.dispatch( + {"method": "battery.trigger", "params": {"operator": "lt", "value": 20}} + ) + + cb.assert_awaited_once() + notification = cb.call_args[0][0] + assert notification.field == "BatteryLevel" + assert notification.value == 15.0 diff --git a/tests/openclaw/test_emitter.py b/tests/openclaw/test_emitter.py index 33d2e4d..49da576 100644 --- a/tests/openclaw/test_emitter.py +++ b/tests/openclaw/test_emitter.py @@ -147,9 +147,43 @@ def test_gear(self, emitter: EventEmitter) -> None: assert event["params"]["data"]["gear"] == "D" -class TestUnmappedField: - def test_unknown_field_returns_none(self, emitter: EventEmitter) -> None: - assert emitter.to_event("UnknownField", 42, vin="VIN1") is None +class TestGenericFallback: + def test_unknown_field_produces_generic_event(self, emitter: EventEmitter) -> None: + event = emitter.to_event("PackVoltage", 398.2, vin="VIN1") + assert event is not None + assert event["params"]["event_type"] == "pack_voltage" + data = event["params"]["data"] + assert data["field"] == "PackVoltage" + assert data["value"] == 398.2 + + def test_generic_event_snake_case_naming(self, emitter: EventEmitter) -> None: + cases = { + "HvacFanSpeed": "hvac_fan_speed", + "TpmsPressureFl": "tpms_pressure_fl", + "ChargeLimitSoc": "charge_limit_soc", + "Odometer": "odometer", + "BMSState": "bms_state", + } + for field_name, expected_type in cases.items(): + event = emitter.to_event(field_name, 1, vin="VIN1") + assert event is not None, f"No event for {field_name}" + assert event["params"]["event_type"] == expected_type, ( + f"{field_name} → {event['params']['event_type']}, expected {expected_type}" + ) + + def test_generic_event_with_dict_value(self, emitter: EventEmitter) -> None: + val = {"a": 1, "b": "two"} + event = emitter.to_event("CustomSensor", val, vin="VIN1") + assert event is not None + data = event["params"]["data"] + assert data["value"] == val + + def test_generic_event_preserves_envelope(self, emitter: EventEmitter) -> None: + event = emitter.to_event("PackVoltage", 398.2, vin="VIN1") + assert event is not None + assert event["method"] == "req:agent" + assert event["params"]["source"] == "test-bridge" + assert event["params"]["vin"] == "VIN1" class TestTimestamp: diff --git a/tests/openclaw/test_filters.py b/tests/openclaw/test_filters.py index 2a215bc..66b2228 100644 --- a/tests/openclaw/test_filters.py +++ b/tests/openclaw/test_filters.py @@ -39,9 +39,27 @@ def test_first_value_always_emits(self) -> None: filt = self._make_filter(Soc=(5.0, 10.0)) assert filt.should_emit("Soc", 72, 0.0) is True - def test_unknown_field_rejected(self) -> None: + def test_unknown_field_uses_default_filter(self) -> None: + """Unconfigured fields pass through with the module-level default fallback.""" filt = self._make_filter(Soc=(5.0, 10.0)) - assert filt.should_emit("Unknown", 42, 0.0) is False + # First value for an unconfigured field — always emits + assert filt.should_emit("Unknown", 42, 0.0) is True + + def test_default_filter_throttles(self) -> None: + """Default fallback enforces its 5-second throttle on unconfigured fields.""" + filt = self._make_filter() + assert filt.should_emit("NewField", 1, 0.0) is True + filt.record_emit("NewField", 1, 0.0) + # Changed value but within 5s throttle window + assert filt.should_emit("NewField", 2, 3.0) is False + # After 5s throttle + assert filt.should_emit("NewField", 2, 6.0) is True + + def test_disabled_field_still_rejected(self) -> None: + """Explicitly disabled fields are blocked even if unconfigured ones pass.""" + filters = {"Blocked": FieldFilter(enabled=False, granularity=0.0, throttle_seconds=0.0)} + filt = DualGateFilter(filters) + assert filt.should_emit("Blocked", 42, 0.0) is False def test_disabled_field_rejected(self) -> None: filters = {"Soc": FieldFilter(enabled=False, granularity=0.0, throttle_seconds=0.0)} @@ -129,3 +147,72 @@ def test_non_numeric_value_treated_as_changed(self) -> None: filt.record_emit("Gear", "D", 0.0) # Non-numeric delta → infinity → always passes assert filt.should_emit("Gear", "R", 1.0) is True + + +class TestStalenessGate: + """Tests for max_seconds (staleness override).""" + + def test_max_seconds_forces_emit_when_stale(self) -> None: + """Value barely changes but max_seconds elapsed → force emit.""" + filters = {"Soc": FieldFilter(granularity=5.0, throttle_seconds=1.0, max_seconds=60.0)} + filt = DualGateFilter(filters) + assert filt.should_emit("Soc", 72, 0.0) is True + filt.record_emit("Soc", 72, 0.0) + # Delta too small (1), throttle passed, but max_seconds not reached + assert filt.should_emit("Soc", 73, 30.0) is False + # max_seconds reached (61s > 60s) → force through despite small delta + assert filt.should_emit("Soc", 73, 61.0) is True + + def test_max_seconds_zero_disables_staleness(self) -> None: + """max_seconds=0 means staleness gate is inactive.""" + filters = {"Soc": FieldFilter(granularity=5.0, throttle_seconds=1.0, max_seconds=0.0)} + filt = DualGateFilter(filters) + assert filt.should_emit("Soc", 72, 0.0) is True + filt.record_emit("Soc", 72, 0.0) + # Even after a long time, small delta still blocked + assert filt.should_emit("Soc", 73, 999.0) is False + + def test_max_seconds_respects_throttle(self) -> None: + """Staleness gate doesn't bypass the throttle gate.""" + filters = {"Soc": FieldFilter(granularity=5.0, throttle_seconds=10.0, max_seconds=5.0)} + filt = DualGateFilter(filters) + assert filt.should_emit("Soc", 72, 0.0) is True + filt.record_emit("Soc", 72, 0.0) + # max_seconds elapsed (6s > 5s) but throttle blocks (6s < 10s) + assert filt.should_emit("Soc", 73, 6.0) is False + # Both staleness and throttle satisfied (11s > 10s > 5s) + assert filt.should_emit("Soc", 73, 11.0) is True + + def test_max_seconds_on_first_value(self) -> None: + """First value always emits regardless of max_seconds config.""" + filters = {"Soc": FieldFilter(granularity=5.0, throttle_seconds=1.0, max_seconds=60.0)} + filt = DualGateFilter(filters) + assert filt.should_emit("Soc", 72, 0.0) is True + + def test_max_seconds_resets_after_forced_emit(self) -> None: + """After staleness forces an emit, timer resets normally.""" + filters = {"Soc": FieldFilter(granularity=5.0, throttle_seconds=1.0, max_seconds=10.0)} + filt = DualGateFilter(filters) + assert filt.should_emit("Soc", 72, 0.0) is True + filt.record_emit("Soc", 72, 0.0) + # Force emit at 11s via staleness + assert filt.should_emit("Soc", 73, 11.0) is True + filt.record_emit("Soc", 73, 11.0) + # Small delta again, staleness timer reset — not stale yet + assert filt.should_emit("Soc", 74, 15.0) is False + # Stale again at 22s (11s since last emit at 11s) + assert filt.should_emit("Soc", 74, 22.0) is True + + def test_max_seconds_with_location(self) -> None: + """Staleness gate works for location fields too.""" + filters = { + "Location": FieldFilter(granularity=50.0, throttle_seconds=1.0, max_seconds=30.0), + } + filt = DualGateFilter(filters) + loc = {"latitude": 40.7128, "longitude": -74.0060} + assert filt.should_emit("Location", loc, 0.0) is True + filt.record_emit("Location", loc, 0.0) + # Same location, not stale yet + assert filt.should_emit("Location", loc, 15.0) is False + # Same location, but stale (31s > 30s) + assert filt.should_emit("Location", loc, 31.0) is True diff --git a/tests/openclaw/test_gateway.py b/tests/openclaw/test_gateway.py index de15f30..7ca5995 100644 --- a/tests/openclaw/test_gateway.py +++ b/tests/openclaw/test_gateway.py @@ -543,8 +543,8 @@ async def test_capabilities_in_connect_params(self) -> None: @pytest.mark.asyncio async def test_default_capabilities_sends_all_commands(self) -> None: - """Default NodeCapabilities sends location.get + system.run.""" - caps = NodeCapabilities() # defaults: 1 read + 1 write + """Default NodeCapabilities sends all advertised commands.""" + caps = NodeCapabilities() mock_ws = AsyncMock() mock_ws.recv = AsyncMock(side_effect=[_challenge(), _hello_ok()]) mock_ws.send = AsyncMock() @@ -558,18 +558,25 @@ async def test_default_capabilities_sends_all_commands(self) -> None: permissions = sent["params"]["permissions"] caps_list = sent["params"]["caps"] - # 2 commands: location.get (read) + system.run (write) - assert len(commands) == 2 + # 7 commands: 4 reads + 3 writes + assert len(commands) == 7 assert "location.get" in commands + assert "telemetry.get" in commands + assert "trigger.list" in commands + assert "trigger.poll" in commands assert "system.run" in commands + assert "trigger.create" in commands + assert "trigger.delete" in commands # permissions must match commands 1:1 - assert len(permissions) == 2 + assert len(permissions) == 7 assert all(permissions[cmd] is True for cmd in commands) - # caps: location, system - assert len(caps_list) == 2 + # caps: location, telemetry, trigger, system + assert len(caps_list) == 4 assert "location" in caps_list + assert "telemetry" in caps_list + assert "trigger" in caps_list assert "system" in caps_list @pytest.mark.asyncio diff --git a/tests/telemetry/test_tui.py b/tests/telemetry/test_tui.py index 20522e4..0f1970a 100644 --- a/tests/telemetry/test_tui.py +++ b/tests/telemetry/test_tui.py @@ -267,7 +267,7 @@ async def test_battery_field_in_battery_table(self) -> None: async with app.run_test(): app._state["BatteryLevel"] = 80 app._timestamps["BatteryLevel"] = datetime.now(UTC) - app._update_panels() + app._update_panel_cell("BatteryLevel", 80) table = app.query_one("#battery-table", DataTable) assert table.row_count == 1 @@ -279,7 +279,7 @@ async def test_climate_field_in_climate_table(self) -> None: async with app.run_test(): app._state["InsideTemp"] = 22.5 app._timestamps["InsideTemp"] = datetime.now(UTC) - app._update_panels() + app._update_panel_cell("InsideTemp", 22.5) table = app.query_one("#climate-table", DataTable) assert table.row_count == 1 @@ -291,7 +291,7 @@ async def test_security_field_in_security_table(self) -> None: async with app.run_test(): app._state["Locked"] = True app._timestamps["Locked"] = datetime.now(UTC) - app._update_panels() + app._update_panel_cell("Locked", True) table = app.query_one("#security-table", DataTable) assert table.row_count == 1 @@ -303,7 +303,7 @@ async def test_unknown_field_goes_to_diagnostics(self) -> None: async with app.run_test(): app._state["UnknownField123"] = 42 app._timestamps["UnknownField123"] = datetime.now(UTC) - app._update_panels() + app._update_panel_cell("UnknownField123", 42) table = app.query_one("#diagnostics-table", DataTable) assert table.row_count == 1 @@ -355,7 +355,7 @@ async def test_new_field_adds_table_row(self) -> None: async with app.run_test(): app._state["BatteryLevel"] = 80 app._timestamps["BatteryLevel"] = datetime.now(UTC) - app._update_panels() + app._update_panel_cell("BatteryLevel", 80) table = app.query_one("#battery-table", DataTable) assert table.row_count == 1 @@ -363,7 +363,7 @@ async def test_new_field_adds_table_row(self) -> None: # Second field in same panel. app._state["Soc"] = 80 app._timestamps["Soc"] = datetime.now(UTC) - app._update_panels() + app._update_panel_cell("Soc", 80) assert table.row_count == 2 @@ -373,14 +373,14 @@ async def test_existing_field_updates_in_place(self) -> None: async with app.run_test(): app._state["BatteryLevel"] = 80 app._timestamps["BatteryLevel"] = datetime.now(UTC) - app._update_panels() + app._update_panel_cell("BatteryLevel", 80) table = app.query_one("#battery-table", DataTable) assert table.row_count == 1 # Update same field. app._state["BatteryLevel"] = 75 - app._update_panels() + app._update_panel_cell("BatteryLevel", 75) # Still one row, not two. assert table.row_count == 1 @@ -599,3 +599,85 @@ async def test_no_frame_summary_when_no_new_frames(self) -> None: app._maybe_log_frame_summary() # No message should have been enqueued. assert app._activity_queue.empty() + + +# --------------------------------------------------------------------------- +# Triggers widget +# --------------------------------------------------------------------------- + + +class TestTriggersWidget: + @pytest.mark.asyncio + async def test_triggers_table_exists(self) -> None: + """The triggers DataTable should be present in the sidebar.""" + app = TelemetryTUI(vin=VIN) + async with app.run_test(): + table = app.query_one("#triggers-table", DataTable) + assert table is not None + assert table.row_count == 0 + + @pytest.mark.asyncio + async def test_triggers_table_populated(self) -> None: + """Triggers are shown after set_trigger_manager and _update_triggers.""" + from tescmd.triggers.manager import TriggerManager + from tescmd.triggers.models import TriggerCondition, TriggerDefinition, TriggerOperator + + app = TelemetryTUI(vin=VIN) + mgr = TriggerManager(vin=VIN) + mgr.create( + TriggerDefinition( + condition=TriggerCondition( + field="BatteryLevel", operator=TriggerOperator.LT, value=20, + ), + ) + ) + mgr.create( + TriggerDefinition( + condition=TriggerCondition( + field="InsideTemp", operator=TriggerOperator.GT, value=35, + ), + once=True, + ) + ) + app.set_trigger_manager(mgr) + + async with app.run_test(): + app._update_triggers() + table = app.query_one("#triggers-table", DataTable) + assert table.row_count == 2 + + @pytest.mark.asyncio + async def test_triggers_table_removes_deleted(self) -> None: + """Deleted triggers are removed from the table on refresh.""" + from tescmd.triggers.manager import TriggerManager + from tescmd.triggers.models import TriggerCondition, TriggerDefinition, TriggerOperator + + app = TelemetryTUI(vin=VIN) + mgr = TriggerManager(vin=VIN) + t = mgr.create( + TriggerDefinition( + condition=TriggerCondition( + field="BatteryLevel", operator=TriggerOperator.LT, value=20, + ), + ) + ) + app.set_trigger_manager(mgr) + + async with app.run_test(): + app._update_triggers() + table = app.query_one("#triggers-table", DataTable) + assert table.row_count == 1 + + mgr.delete(t.id) + app._update_triggers() + assert table.row_count == 0 + + @pytest.mark.asyncio + async def test_triggers_noop_without_manager(self) -> None: + """_update_triggers is a no-op when no manager is set.""" + app = TelemetryTUI(vin=VIN) + async with app.run_test(): + # Should not raise + app._update_triggers() + table = app.query_one("#triggers-table", DataTable) + assert table.row_count == 0 diff --git a/tests/triggers/test_manager.py b/tests/triggers/test_manager.py index 6146c46..f053115 100644 --- a/tests/triggers/test_manager.py +++ b/tests/triggers/test_manager.py @@ -92,7 +92,7 @@ def test_empty(self) -> None: def test_returns_all(self) -> None: mgr = TriggerManager(vin="V") t1 = mgr.create(_trig("Soc", TriggerOperator.LT, 20)) - t2 = mgr.create(_trig("InsideTemp", TriggerOperator.GT, 100)) + t2 = mgr.create(_trig("InsideTemp", TriggerOperator.GT, 35)) assert set(t.id for t in mgr.list_all()) == {t1.id, t2.id} @@ -130,8 +130,8 @@ async def test_lte_fires_at_boundary(self) -> None: @pytest.mark.asyncio async def test_gte_fires_at_boundary(self) -> None: mgr = TriggerManager(vin="V") - mgr.create(_trig("InsideTemp", TriggerOperator.GTE, 100, cooldown=0)) - await mgr.evaluate("InsideTemp", 100.0, 95.0, TS) + mgr.create(_trig("InsideTemp", TriggerOperator.GTE, 35, cooldown=0)) + await mgr.evaluate("InsideTemp", 35.0, 30.0, TS) assert len(mgr.drain_pending()) == 1 @pytest.mark.asyncio @@ -208,37 +208,61 @@ async def test_fires_after_cooldown_expires(self) -> None: class TestOnce: @pytest.mark.asyncio - async def test_one_shot_auto_deletes(self) -> None: + async def test_one_shot_stays_until_delivery(self) -> None: + """One-shot triggers remain registered after firing (pending delivery).""" mgr = TriggerManager(vin="V") mgr.create(_trig("Soc", TriggerOperator.LT, 20, once=True, cooldown=0)) await mgr.evaluate("Soc", 15.0, 25.0, TS) assert len(mgr.drain_pending()) == 1 - assert mgr.list_all() == [] + # Trigger still exists — waiting for push callback to confirm delivery + assert len(mgr.list_all()) == 1 @pytest.mark.asyncio async def test_one_shot_fires_only_once(self) -> None: + """One-shot trigger fires once, then is skipped on subsequent evaluations.""" mgr = TriggerManager(vin="V") mgr.create(_trig("Soc", TriggerOperator.LT, 20, once=True, cooldown=0)) await mgr.evaluate("Soc", 15.0, 25.0, TS) await mgr.evaluate("Soc", 10.0, 15.0, TS) - # Second fire should not happen (trigger was deleted) - mgr.drain_pending() - # drain_pending was called after both evaluates, so only 1 - # notification should be there from the first evaluate - # (the second was cleared by the first drain_pending above) - # Actually both evaluates happen before drain, so let me adjust: + pending = mgr.drain_pending() + # Only one notification — second evaluate skipped the fired trigger + assert len(pending) == 1 + + @pytest.mark.asyncio + async def test_one_shot_deleted_after_explicit_delete(self) -> None: + """One-shot trigger is removed when delete() is called (simulating delivery).""" + mgr = TriggerManager(vin="V") + t = mgr.create(_trig("Soc", TriggerOperator.LT, 20, once=True, cooldown=0)) + + await mgr.evaluate("Soc", 15.0, 25.0, TS) + assert len(mgr.list_all()) == 1 + + mgr.delete(t.id) + assert mgr.list_all() == [] @pytest.mark.asyncio - async def test_one_shot_only_one_notification(self) -> None: + async def test_one_shot_notification_carries_once_flag(self) -> None: + """Notification from a one-shot trigger has once=True.""" mgr = TriggerManager(vin="V") mgr.create(_trig("Soc", TriggerOperator.LT, 20, once=True, cooldown=0)) await mgr.evaluate("Soc", 15.0, 25.0, TS) - await mgr.evaluate("Soc", 10.0, 15.0, TS) pending = mgr.drain_pending() assert len(pending) == 1 + assert pending[0].once is True + + @pytest.mark.asyncio + async def test_persistent_notification_once_is_false(self) -> None: + """Notification from a persistent trigger has once=False.""" + mgr = TriggerManager(vin="V") + mgr.create(_trig("Soc", TriggerOperator.LT, 20, once=False, cooldown=0)) + + await mgr.evaluate("Soc", 15.0, 25.0, TS) + pending = mgr.drain_pending() + assert len(pending) == 1 + assert pending[0].once is False class TestDelivery: @@ -315,7 +339,7 @@ class TestFieldIndex: async def test_only_matching_field_evaluated(self) -> None: mgr = TriggerManager(vin="V") mgr.create(_trig("Soc", TriggerOperator.LT, 20, cooldown=0)) - mgr.create(_trig("InsideTemp", TriggerOperator.GT, 100, cooldown=0)) + mgr.create(_trig("InsideTemp", TriggerOperator.GT, 35, cooldown=0)) await mgr.evaluate("Soc", 15.0, 25.0, TS) pending = mgr.drain_pending() @@ -472,3 +496,81 @@ def test_non_numeric_returns_false(self) -> None: def test_string_numeric_coercion(self) -> None: c = _cond("f", TriggerOperator.GT, "80") assert _matches(c, "85", None) is True + + +class TestEvaluateSingle: + """Tests for evaluate_single — one-trigger evaluation without side-effects.""" + + @pytest.mark.asyncio + async def test_returns_true_on_match(self) -> None: + mgr = TriggerManager(vin="V") + t = mgr.create(_trig("BatteryLevel", TriggerOperator.LT, 20)) + + fired = await mgr.evaluate_single(t.id, 15.0, None, TS) + + assert fired is True + pending = mgr.drain_pending() + assert len(pending) == 1 + assert pending[0].value == 15.0 + + @pytest.mark.asyncio + async def test_returns_false_on_no_match(self) -> None: + mgr = TriggerManager(vin="V") + t = mgr.create(_trig("BatteryLevel", TriggerOperator.LT, 20)) + + fired = await mgr.evaluate_single(t.id, 50.0, None, TS) + + assert fired is False + assert len(mgr.drain_pending()) == 0 + + @pytest.mark.asyncio + async def test_returns_false_for_missing_id(self) -> None: + mgr = TriggerManager(vin="V") + + fired = await mgr.evaluate_single("nonexistent", 15.0, None, TS) + + assert fired is False + + @pytest.mark.asyncio + async def test_does_not_auto_delete(self) -> None: + """evaluate_single does NOT auto-delete one-shot triggers.""" + mgr = TriggerManager(vin="V") + t = mgr.create(_trig("BatteryLevel", TriggerOperator.LT, 20, once=True)) + + fired = await mgr.evaluate_single(t.id, 15.0, None, TS) + + assert fired is True + # Trigger should still exist — caller handles deletion + assert len(mgr.list_all()) == 1 + + @pytest.mark.asyncio + async def test_calls_fire_callbacks(self) -> None: + mgr = TriggerManager(vin="V") + t = mgr.create(_trig("BatteryLevel", TriggerOperator.LT, 20)) + + cb = AsyncMock() + mgr.add_on_fire(cb) + + await mgr.evaluate_single(t.id, 15.0, None, TS) + + cb.assert_awaited_once() + assert cb.call_args[0][0].field == "BatteryLevel" + + +class TestQueueNotification: + """Tests for queue_notification and vin property.""" + + def test_queue_notification(self) -> None: + mgr = TriggerManager(vin="V") + n = TriggerNotification( + trigger_id="t1", field="f", operator=TriggerOperator.LT, + threshold=20, value=15, vin="V", + ) + mgr.queue_notification(n) + pending = mgr.drain_pending() + assert len(pending) == 1 + assert pending[0].trigger_id == "t1" + + def test_vin_property(self) -> None: + mgr = TriggerManager(vin="MYVIN") + assert mgr.vin == "MYVIN" From d90a8813c9a528b530e5e2b0398849681209fb91 Mon Sep 17 00:00:00 2001 From: Sean McLellan Date: Wed, 4 Feb 2026 19:29:51 -0500 Subject: [PATCH 2/3] =?UTF-8?q?refactor:=20remove=20trigger.poll=20?= =?UTF-8?q?=E2=80=94=20notifications=20are=20push-only?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Trigger notifications are delivered exclusively via push callbacks over the OpenClaw WebSocket. Removed the polling endpoint, MCP trigger_poll tool, pending notification queue, drain_pending(), and queue_notification() from TriggerManager. Tests updated to use fire callbacks instead of drain_pending() assertions. Co-Authored-By: Claude Opus 4.5 --- CHANGELOG.md | 6 +- src/tescmd/cli/serve.py | 15 +- src/tescmd/openclaw/config.py | 1 - src/tescmd/openclaw/dispatcher.py | 7 - src/tescmd/triggers/manager.py | 18 +-- tests/openclaw/test_bridge.py | 50 ++++--- tests/openclaw/test_config.py | 4 +- tests/openclaw/test_dispatcher.py | 68 +-------- tests/openclaw/test_gateway.py | 7 +- tests/triggers/test_manager.py | 227 +++++++++++------------------- 10 files changed, 130 insertions(+), 273 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c604d9d..a31abed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Default filter for unconfigured fields** — `DualGateFilter` now applies a sensible default (any-change granularity, 5s throttle, 2min staleness) to unconfigured fields instead of silently dropping them; explicitly disabled fields are still blocked - **`telemetry.get` handler** — generic read handler in `CommandDispatcher` allows agents to read the latest value of any telemetry field from the in-memory store - **`telemetry_get` MCP tool** — exposes `telemetry.get` as an MCP tool so agent frameworks can read arbitrary telemetry fields -- **Trigger commands in capabilities** — `NodeCapabilities` now advertises `telemetry.get`, `trigger.list`, `trigger.poll`, `trigger.create`, and `trigger.delete` to the gateway +- **Trigger commands in capabilities** — `NodeCapabilities` now advertises `telemetry.get`, `trigger.list`, `trigger.create`, and `trigger.delete` to the gateway + +### Removed + +- **`trigger.poll` endpoint and pending queue** — trigger notifications are delivered exclusively via push callbacks over the OpenClaw WebSocket; the MCP `trigger_poll` tool, `drain_pending()`, and the in-memory pending notification queue have been removed ### Fixed diff --git a/src/tescmd/cli/serve.py b/src/tescmd/cli/serve.py index fe5b7a1..c73c0e6 100644 --- a/src/tescmd/cli/serve.py +++ b/src/tescmd/cli/serve.py @@ -663,7 +663,7 @@ def _register_trigger_tools( Each trigger domain (cabin_temp, outside_temp, battery, location) gets its own create, list, and delete tools. Temperature triggers accept values in °F and convert to °C internally (matching the dispatcher's - convenience aliases). ``trigger_poll`` is shared across all domains. + convenience aliases). When *telemetry_store* is provided, newly created triggers are immediately evaluated against the current value. If the condition @@ -995,19 +995,6 @@ def _delete_trigger(params: dict[str, Any]) -> dict[str, Any]: is_write=True, ) - # -- Shared: poll -------------------------------------------------------- - - def _handle_poll(params: dict[str, Any]) -> dict[str, Any]: - notifications = trigger_manager.drain_pending() - return {"notifications": [n.model_dump(mode="json") for n in notifications]} - - mcp_server.register_custom_tool( - "trigger_poll", - _handle_poll, - "Poll for fired trigger notifications across all domains", - {"type": "object", "properties": {}}, - ) - # -- Shared: list all triggers ------------------------------------------- def _handle_trigger_list(params: dict[str, Any]) -> dict[str, Any]: diff --git a/src/tescmd/openclaw/config.py b/src/tescmd/openclaw/config.py index 1a25a6f..7b03dbf 100644 --- a/src/tescmd/openclaw/config.py +++ b/src/tescmd/openclaw/config.py @@ -33,7 +33,6 @@ class NodeCapabilities(BaseModel): "location.get", "telemetry.get", "trigger.list", - "trigger.poll", ] writes: list[str] = [ "system.run", diff --git a/src/tescmd/openclaw/dispatcher.py b/src/tescmd/openclaw/dispatcher.py index f5701dd..10a31c0 100644 --- a/src/tescmd/openclaw/dispatcher.py +++ b/src/tescmd/openclaw/dispatcher.py @@ -124,7 +124,6 @@ def __init__( # Shared trigger operations "trigger.create": self._handle_trigger_create, "trigger.list": self._handle_trigger_list_all, - "trigger.poll": self._handle_trigger_poll, "trigger.delete": self._handle_trigger_delete, # Domain-specific trigger CRUD "cabin_temp.trigger": self._handle_cabin_temp_trigger, @@ -616,12 +615,6 @@ async def _handle_trigger_list_all(self, params: dict[str, Any]) -> dict[str, An result.append(entry) return {"triggers": result} - async def _handle_trigger_poll(self, params: dict[str, Any]) -> dict[str, Any]: - """Drain and return pending trigger notifications.""" - mgr = self._require_trigger_manager() - notifications = mgr.drain_pending() - return {"notifications": [n.model_dump(mode="json") for n in notifications]} - # -- Convenience trigger aliases ----------------------------------------- async def _handle_cabin_temp_trigger(self, params: dict[str, Any]) -> dict[str, Any]: diff --git a/src/tescmd/triggers/manager.py b/src/tescmd/triggers/manager.py index daf45f5..6bf40ea 100644 --- a/src/tescmd/triggers/manager.py +++ b/src/tescmd/triggers/manager.py @@ -8,7 +8,7 @@ import logging import time -from collections import defaultdict, deque +from collections import defaultdict from typing import TYPE_CHECKING, Any from tescmd.triggers.models import ( @@ -25,7 +25,6 @@ logger = logging.getLogger(__name__) MAX_TRIGGERS = 100 -MAX_PENDING = 500 class TriggerLimitError(Exception): @@ -46,7 +45,6 @@ def __init__(self, vin: str) -> None: self._triggers: dict[str, TriggerDefinition] = {} self._field_index: dict[str, set[str]] = defaultdict(set) self._last_fire_times: dict[str, float] = {} - self._pending: deque[TriggerNotification] = deque(maxlen=MAX_PENDING) self._on_fire_callbacks: list[Callable[[TriggerNotification], Awaitable[None]]] = [] self._fired_once_ids: set[str] = set() @@ -93,21 +91,11 @@ def list_all(self) -> list[TriggerDefinition]: """Return all registered triggers.""" return list(self._triggers.values()) - def drain_pending(self) -> list[TriggerNotification]: - """Return and clear all pending notifications (for MCP polling).""" - result = list(self._pending) - self._pending.clear() - return result - @property def vin(self) -> str: """Vehicle Identification Number included in notifications.""" return self._vin - def queue_notification(self, notification: TriggerNotification) -> None: - """Queue a notification for MCP polling via :meth:`drain_pending`.""" - self._pending.append(notification) - def mark_fired_once(self, trigger_id: str) -> None: """Mark a one-shot trigger as fired (pending delivery). @@ -170,8 +158,6 @@ async def evaluate_single( if trigger.once: self._fired_once_ids.add(trigger_id) - self._pending.append(notification) - for callback in self._on_fire_callbacks: try: await callback(notification) @@ -254,8 +240,6 @@ async def evaluate( if trigger.once: self._fired_once_ids.add(tid) - self._pending.append(notification) - for callback in self._on_fire_callbacks: try: await callback(notification) diff --git a/tests/openclaw/test_bridge.py b/tests/openclaw/test_bridge.py index 6329668..fd364df 100644 --- a/tests/openclaw/test_bridge.py +++ b/tests/openclaw/test_bridge.py @@ -503,19 +503,21 @@ async def test_trigger_evaluated_on_frame(self, gateway: GatewayClient) -> None: client_id="test", ) + fired: list[TriggerNotification] = [] + mgr.add_on_fire(AsyncMock(side_effect=lambda n: fired.append(n))) + # First frame: value above threshold — no fire frame1 = _make_frame(data=[TelemetryDatum("BatteryLevel", 3, 25.0, "float")]) await bridge.on_frame(frame1) - assert len(mgr.drain_pending()) == 0 + assert len(fired) == 0 # Second frame: value below threshold — trigger fires frame2 = _make_frame(data=[TelemetryDatum("BatteryLevel", 3, 15.0, "float")]) await bridge.on_frame(frame2) - pending = mgr.drain_pending() - assert len(pending) == 1 - assert pending[0].field == "BatteryLevel" - assert pending[0].value == 15.0 - assert pending[0].previous_value == 25.0 + assert len(fired) == 1 + assert fired[0].field == "BatteryLevel" + assert fired[0].value == 15.0 + assert fired[0].previous_value == 25.0 @pytest.mark.asyncio async def test_previous_value_captured_before_store_update( @@ -538,12 +540,14 @@ async def test_previous_value_captured_before_store_update( trigger_manager=mgr, ) + fired: list[TriggerNotification] = [] + mgr.add_on_fire(AsyncMock(side_effect=lambda n: fired.append(n))) + # First frame: no previous value → CHANGED fires (None != 72.0) frame1 = _make_frame(data=[TelemetryDatum("Soc", 3, 72.0, "float")]) await bridge.on_frame(frame1) - n1 = mgr.drain_pending() - assert len(n1) == 1 - assert n1[0].previous_value is None + assert len(fired) == 1 + assert fired[0].previous_value is None # After frame1, store should have Soc=72.0 assert store.get("Soc").value == 72.0 # type: ignore[union-attr] @@ -551,16 +555,14 @@ async def test_previous_value_captured_before_store_update( # Second frame with same value — CHANGED does not fire frame2 = _make_frame(data=[TelemetryDatum("Soc", 3, 72.0, "float")]) await bridge.on_frame(frame2) - n2 = mgr.drain_pending() - assert len(n2) == 0 + assert len(fired) == 1 # still 1 # Third frame with different value — fires with previous=72.0 frame3 = _make_frame(data=[TelemetryDatum("Soc", 3, 80.0, "float")]) await bridge.on_frame(frame3) - n3 = mgr.drain_pending() - assert len(n3) == 1 - assert n3[0].previous_value == 72.0 - assert n3[0].value == 80.0 + assert len(fired) == 2 + assert fired[1].previous_value == 72.0 + assert fired[1].value == 80.0 @pytest.mark.asyncio async def test_trigger_works_without_store(self, gateway: GatewayClient) -> None: @@ -579,11 +581,13 @@ async def test_trigger_works_without_store(self, gateway: GatewayClient) -> None trigger_manager=mgr, ) + fired: list[TriggerNotification] = [] + mgr.add_on_fire(AsyncMock(side_effect=lambda n: fired.append(n))) + # Value below threshold — fires (previous is None, which is fine for numeric ops) frame = _make_frame(data=[TelemetryDatum("BatteryLevel", 3, 15.0, "float")]) await bridge.on_frame(frame) - pending = mgr.drain_pending() - assert len(pending) == 1 + assert len(fired) == 1 @pytest.mark.asyncio async def test_trigger_callback_invoked_from_bridge(self, gateway: GatewayClient) -> None: @@ -635,6 +639,9 @@ async def test_end_to_end_threshold_crossing(self, gateway: GatewayClient) -> No filters = {"BatteryLevel": FieldFilter(granularity=0.0, throttle_seconds=0.0)} filt = DualGateFilter(filters) emitter = EventEmitter(client_id="test") + fired: list[TriggerNotification] = [] + mgr.add_on_fire(AsyncMock(side_effect=lambda n: fired.append(n))) + bridge = TelemetryBridge( gateway, filt, @@ -651,11 +658,10 @@ async def test_end_to_end_threshold_crossing(self, gateway: GatewayClient) -> No await bridge.on_frame(frame) # One-shot trigger should have fired once (at level 10.0 crossing) - pending = mgr.drain_pending() - assert len(pending) == 1 - assert pending[0].value == 10.0 - assert pending[0].previous_value == 20.0 - assert pending[0].once is True + assert len(fired) == 1 + assert fired[0].value == 10.0 + assert fired[0].previous_value == 20.0 + assert fired[0].once is True # Trigger stays registered (pending delivery confirmation) assert len(mgr.list_all()) == 1 diff --git a/tests/openclaw/test_config.py b/tests/openclaw/test_config.py index fcc27bc..6247fd0 100644 --- a/tests/openclaw/test_config.py +++ b/tests/openclaw/test_config.py @@ -119,7 +119,7 @@ def test_defaults(self) -> None: assert "location.get" in caps.reads assert "telemetry.get" in caps.reads assert "trigger.list" in caps.reads - assert "trigger.poll" in caps.reads + assert "trigger.poll" not in caps.reads assert "system.run" in caps.writes assert "trigger.create" in caps.writes assert "trigger.delete" in caps.writes @@ -174,7 +174,7 @@ class TestBridgeConfigCapabilities: def test_default_capabilities(self) -> None: cfg = BridgeConfig() assert isinstance(cfg.capabilities, NodeCapabilities) - assert len(cfg.capabilities.reads) == 4 + assert len(cfg.capabilities.reads) == 3 assert len(cfg.capabilities.writes) == 3 assert "telemetry.get" in cfg.capabilities.reads assert "trigger.create" in cfg.capabilities.writes diff --git a/tests/openclaw/test_dispatcher.py b/tests/openclaw/test_dispatcher.py index e913d97..7d72962 100644 --- a/tests/openclaw/test_dispatcher.py +++ b/tests/openclaw/test_dispatcher.py @@ -1161,53 +1161,13 @@ async def test_temp_list_includes_fahrenheit(self) -> None: assert t["value_f"] == 80.1 # converted back to Fahrenheit @pytest.mark.asyncio - async def test_trigger_poll_empty(self) -> None: + async def test_trigger_poll_returns_unknown_command(self) -> None: + """trigger.poll is no longer a recognized command.""" ctx = _mock_app_ctx() mgr = TriggerManager(vin="VIN1") d = CommandDispatcher(vin="VIN1", app_ctx=ctx, trigger_manager=mgr) result = await d.dispatch({"method": "trigger.poll", "params": {}}) - assert result["notifications"] == [] - - @pytest.mark.asyncio - async def test_trigger_poll_returns_notifications(self) -> None: - ctx = _mock_app_ctx() - mgr = TriggerManager(vin="VIN1") - d = CommandDispatcher(vin="VIN1", app_ctx=ctx, trigger_manager=mgr) - await d.dispatch( - { - "method": "battery.trigger", - "params": {"operator": "lt", "value": 20}, - } - ) - # Simulate trigger firing via evaluate - ts = datetime(2026, 2, 1, 12, 0, 0, tzinfo=UTC) - await mgr.evaluate("BatteryLevel", 15.0, 25.0, ts) - result = await d.dispatch({"method": "trigger.poll", "params": {}}) - assert len(result["notifications"]) == 1 - n = result["notifications"][0] - assert n["field"] == "BatteryLevel" - assert n["value"] == 15.0 - - @pytest.mark.asyncio - async def test_trigger_poll_drains(self) -> None: - """Polling should clear the pending queue.""" - ctx = _mock_app_ctx() - mgr = TriggerManager(vin="VIN1") - d = CommandDispatcher(vin="VIN1", app_ctx=ctx, trigger_manager=mgr) - await d.dispatch( - { - "method": "battery.trigger", - "params": {"operator": "lt", "value": 20}, - } - ) - ts = datetime(2026, 2, 1, 12, 0, 0, tzinfo=UTC) - await mgr.evaluate("BatteryLevel", 15.0, 25.0, ts) - # First poll returns notification - r1 = await d.dispatch({"method": "trigger.poll", "params": {}}) - assert len(r1["notifications"]) == 1 - # Second poll is empty - r2 = await d.dispatch({"method": "trigger.poll", "params": {}}) - assert len(r2["notifications"]) == 0 + assert result is None class TestTriggerConvenienceAliases: @@ -1262,13 +1222,11 @@ async def test_cabin_temp_trigger_fires_with_celsius_telemetry(self) -> None: ) ts = datetime(2026, 2, 1, 12, 0, 0, tzinfo=UTC) # Telemetry reports 25°C (77°F) — below threshold, should NOT fire - await mgr.evaluate("InsideTemp", 25.0, 20.0, ts) - assert mgr.drain_pending() == [] + r1 = await mgr.evaluate("InsideTemp", 25.0, 20.0, ts) + assert r1 is False # Telemetry reports 30°C (86°F) — above threshold, SHOULD fire - await mgr.evaluate("InsideTemp", 30.0, 25.0, ts) - pending = mgr.drain_pending() - assert len(pending) == 1 - assert pending[0].value == 30.0 + r2 = await mgr.evaluate("InsideTemp", 30.0, 25.0, ts) + assert r2 is True @pytest.mark.asyncio async def test_battery_trigger(self) -> None: @@ -1335,13 +1293,6 @@ async def test_trigger_list_no_manager(self) -> None: with pytest.raises(RuntimeError, match="Triggers not available"): await d.dispatch({"method": "battery.trigger.list", "params": {}}) - @pytest.mark.asyncio - async def test_trigger_poll_no_manager(self) -> None: - ctx = _mock_app_ctx() - d = CommandDispatcher(vin="VIN1", app_ctx=ctx) - with pytest.raises(RuntimeError, match="Triggers not available"): - await d.dispatch({"method": "trigger.poll", "params": {}}) - @pytest.mark.asyncio async def test_convenience_alias_no_manager(self) -> None: ctx = _mock_app_ctx() @@ -1377,11 +1328,6 @@ async def test_trigger_fires_immediately_when_condition_met(self) -> None: # Trigger fires immediately but stays registered (persistent by default) assert result["immediate"] is True assert len(mgr.list_all()) == 1 - # Notification should be queued for polling - pending = mgr.drain_pending() - assert len(pending) == 1 - assert pending[0].field == "BatteryLevel" - assert pending[0].value == 15.0 @pytest.mark.asyncio async def test_once_trigger_stays_on_immediate_fire(self) -> None: diff --git a/tests/openclaw/test_gateway.py b/tests/openclaw/test_gateway.py index 7ca5995..bbf8deb 100644 --- a/tests/openclaw/test_gateway.py +++ b/tests/openclaw/test_gateway.py @@ -558,18 +558,17 @@ async def test_default_capabilities_sends_all_commands(self) -> None: permissions = sent["params"]["permissions"] caps_list = sent["params"]["caps"] - # 7 commands: 4 reads + 3 writes - assert len(commands) == 7 + # 6 commands: 3 reads + 3 writes + assert len(commands) == 6 assert "location.get" in commands assert "telemetry.get" in commands assert "trigger.list" in commands - assert "trigger.poll" in commands assert "system.run" in commands assert "trigger.create" in commands assert "trigger.delete" in commands # permissions must match commands 1:1 - assert len(permissions) == 7 + assert len(permissions) == 6 assert all(permissions[cmd] is True for cmd in commands) # caps: location, telemetry, trigger, system diff --git a/tests/triggers/test_manager.py b/tests/triggers/test_manager.py index f053115..a01a236 100644 --- a/tests/triggers/test_manager.py +++ b/tests/triggers/test_manager.py @@ -39,6 +39,17 @@ def _trig( ) +def _collector(mgr: TriggerManager) -> list[TriggerNotification]: + """Register a fire callback that collects notifications into a list.""" + fired: list[TriggerNotification] = [] + + async def _collect(n: TriggerNotification) -> None: + fired.append(n) + + mgr.add_on_fire(_collect) + return fired + + class TestCreate: def test_returns_trigger_with_id(self) -> None: mgr = TriggerManager(vin="V") @@ -48,43 +59,25 @@ def test_returns_trigger_with_id(self) -> None: def test_enforces_max_limit(self) -> None: mgr = TriggerManager(vin="V") - for _ in range(100): - mgr.create(_trig("Soc", TriggerOperator.LT, 20)) + for i in range(100): + mgr.create(_trig("Soc", TriggerOperator.LT, i)) with pytest.raises(TriggerLimitError): - mgr.create(_trig("Soc", TriggerOperator.LT, 20)) - - def test_rejects_missing_value_for_non_changed(self) -> None: - from pydantic import ValidationError - - mgr = TriggerManager(vin="V") - with pytest.raises(ValidationError, match="requires a 'value'"): - mgr.create(_trig("Soc", TriggerOperator.LT, None)) - - def test_allows_none_value_for_changed(self) -> None: - mgr = TriggerManager(vin="V") - t = mgr.create(_trig("Locked", TriggerOperator.CHANGED, None)) - assert t.condition.value is None + mgr.create(_trig("Soc", TriggerOperator.LT, 999)) class TestDelete: - def test_returns_true_on_existing(self) -> None: + def test_delete_existing(self) -> None: mgr = TriggerManager(vin="V") t = mgr.create(_trig("Soc", TriggerOperator.LT, 20)) assert mgr.delete(t.id) is True assert mgr.list_all() == [] - def test_returns_false_on_missing(self) -> None: + def test_delete_nonexistent(self) -> None: mgr = TriggerManager(vin="V") assert mgr.delete("nonexistent") is False - def test_removes_from_field_index(self) -> None: - mgr = TriggerManager(vin="V") - t = mgr.create(_trig("Soc", TriggerOperator.LT, 20)) - mgr.delete(t.id) - assert "Soc" not in mgr._field_index - -class TestListAll: +class TestList: def test_empty(self) -> None: mgr = TriggerManager(vin="V") assert mgr.list_all() == [] @@ -101,80 +94,81 @@ class TestEvaluate: async def test_lt_fires(self) -> None: mgr = TriggerManager(vin="V") mgr.create(_trig("Soc", TriggerOperator.LT, 20, cooldown=0)) - await mgr.evaluate("Soc", 15.0, 25.0, TS) - pending = mgr.drain_pending() - assert len(pending) == 1 - assert pending[0].value == 15.0 + fired = _collector(mgr) + result = await mgr.evaluate("Soc", 15.0, 25.0, TS) + assert result is True + assert len(fired) == 1 + assert fired[0].value == 15.0 @pytest.mark.asyncio async def test_lt_does_not_fire_above(self) -> None: mgr = TriggerManager(vin="V") mgr.create(_trig("Soc", TriggerOperator.LT, 20, cooldown=0)) - await mgr.evaluate("Soc", 25.0, 30.0, TS) - assert mgr.drain_pending() == [] + result = await mgr.evaluate("Soc", 25.0, 30.0, TS) + assert result is False @pytest.mark.asyncio async def test_gt_fires(self) -> None: mgr = TriggerManager(vin="V") mgr.create(_trig("VehicleSpeed", TriggerOperator.GT, 80, cooldown=0)) - await mgr.evaluate("VehicleSpeed", 85.0, 70.0, TS) - assert len(mgr.drain_pending()) == 1 + result = await mgr.evaluate("VehicleSpeed", 85.0, 70.0, TS) + assert result is True @pytest.mark.asyncio async def test_lte_fires_at_boundary(self) -> None: mgr = TriggerManager(vin="V") mgr.create(_trig("OutsideTemp", TriggerOperator.LTE, 32, cooldown=0)) - await mgr.evaluate("OutsideTemp", 32.0, 35.0, TS) - assert len(mgr.drain_pending()) == 1 + result = await mgr.evaluate("OutsideTemp", 32.0, 35.0, TS) + assert result is True @pytest.mark.asyncio async def test_gte_fires_at_boundary(self) -> None: mgr = TriggerManager(vin="V") mgr.create(_trig("InsideTemp", TriggerOperator.GTE, 35, cooldown=0)) - await mgr.evaluate("InsideTemp", 35.0, 30.0, TS) - assert len(mgr.drain_pending()) == 1 + result = await mgr.evaluate("InsideTemp", 35.0, 30.0, TS) + assert result is True @pytest.mark.asyncio async def test_eq_fires(self) -> None: mgr = TriggerManager(vin="V") mgr.create(_trig("ChargeState", TriggerOperator.EQ, "Charging", cooldown=0)) - await mgr.evaluate("ChargeState", "Charging", "Stopped", TS) - assert len(mgr.drain_pending()) == 1 + result = await mgr.evaluate("ChargeState", "Charging", "Stopped", TS) + assert result is True @pytest.mark.asyncio async def test_eq_does_not_fire(self) -> None: mgr = TriggerManager(vin="V") mgr.create(_trig("ChargeState", TriggerOperator.EQ, "Charging", cooldown=0)) - await mgr.evaluate("ChargeState", "Stopped", "Charging", TS) - assert mgr.drain_pending() == [] + result = await mgr.evaluate("ChargeState", "Stopped", "Charging", TS) + assert result is False @pytest.mark.asyncio async def test_neq_fires(self) -> None: mgr = TriggerManager(vin="V") mgr.create(_trig("Gear", TriggerOperator.NEQ, "P", cooldown=0)) - await mgr.evaluate("Gear", "D", "P", TS) - assert len(mgr.drain_pending()) == 1 + result = await mgr.evaluate("Gear", "D", "P", TS) + assert result is True @pytest.mark.asyncio async def test_changed_fires(self) -> None: mgr = TriggerManager(vin="V") mgr.create(_trig("Locked", TriggerOperator.CHANGED, cooldown=0)) - await mgr.evaluate("Locked", False, True, TS) - assert len(mgr.drain_pending()) == 1 + result = await mgr.evaluate("Locked", False, True, TS) + assert result is True @pytest.mark.asyncio async def test_changed_does_not_fire_same_value(self) -> None: mgr = TriggerManager(vin="V") mgr.create(_trig("Locked", TriggerOperator.CHANGED, cooldown=0)) - await mgr.evaluate("Locked", True, True, TS) - assert mgr.drain_pending() == [] + result = await mgr.evaluate("Locked", True, True, TS) + assert result is False @pytest.mark.asyncio async def test_non_numeric_lt_returns_false(self) -> None: mgr = TriggerManager(vin="V") mgr.create(_trig("Soc", TriggerOperator.LT, 20, cooldown=0)) - await mgr.evaluate("Soc", "not-a-number", 25.0, TS) - assert mgr.drain_pending() == [] + result = await mgr.evaluate("Soc", "not-a-number", 25.0, TS) + assert result is False class TestCooldown: @@ -183,27 +177,26 @@ async def test_persistent_respects_cooldown(self) -> None: mgr = TriggerManager(vin="V") mgr.create(_trig("Soc", TriggerOperator.LT, 20, cooldown=60.0)) - await mgr.evaluate("Soc", 15.0, 25.0, TS) - assert len(mgr.drain_pending()) == 1 + r1 = await mgr.evaluate("Soc", 15.0, 25.0, TS) + assert r1 is True # Second fire within cooldown — should NOT fire - await mgr.evaluate("Soc", 10.0, 15.0, TS) - assert mgr.drain_pending() == [] + r2 = await mgr.evaluate("Soc", 10.0, 15.0, TS) + assert r2 is False @pytest.mark.asyncio async def test_fires_after_cooldown_expires(self) -> None: mgr = TriggerManager(vin="V") mgr.create(_trig("Soc", TriggerOperator.LT, 20, cooldown=0.01)) - await mgr.evaluate("Soc", 15.0, 25.0, TS) - assert len(mgr.drain_pending()) == 1 + r1 = await mgr.evaluate("Soc", 15.0, 25.0, TS) + assert r1 is True # Simulate time passing past cooldown - time.sleep(0.02) - await mgr.evaluate("Soc", 10.0, 15.0, TS) - assert len(mgr.drain_pending()) == 1 + r2 = await mgr.evaluate("Soc", 10.0, 15.0, TS) + assert r2 is True class TestOnce: @@ -213,8 +206,8 @@ async def test_one_shot_stays_until_delivery(self) -> None: mgr = TriggerManager(vin="V") mgr.create(_trig("Soc", TriggerOperator.LT, 20, once=True, cooldown=0)) - await mgr.evaluate("Soc", 15.0, 25.0, TS) - assert len(mgr.drain_pending()) == 1 + result = await mgr.evaluate("Soc", 15.0, 25.0, TS) + assert result is True # Trigger still exists — waiting for push callback to confirm delivery assert len(mgr.list_all()) == 1 @@ -223,12 +216,12 @@ async def test_one_shot_fires_only_once(self) -> None: """One-shot trigger fires once, then is skipped on subsequent evaluations.""" mgr = TriggerManager(vin="V") mgr.create(_trig("Soc", TriggerOperator.LT, 20, once=True, cooldown=0)) + fired = _collector(mgr) await mgr.evaluate("Soc", 15.0, 25.0, TS) await mgr.evaluate("Soc", 10.0, 15.0, TS) - pending = mgr.drain_pending() # Only one notification — second evaluate skipped the fired trigger - assert len(pending) == 1 + assert len(fired) == 1 @pytest.mark.asyncio async def test_one_shot_deleted_after_explicit_delete(self) -> None: @@ -247,22 +240,22 @@ async def test_one_shot_notification_carries_once_flag(self) -> None: """Notification from a one-shot trigger has once=True.""" mgr = TriggerManager(vin="V") mgr.create(_trig("Soc", TriggerOperator.LT, 20, once=True, cooldown=0)) + fired = _collector(mgr) await mgr.evaluate("Soc", 15.0, 25.0, TS) - pending = mgr.drain_pending() - assert len(pending) == 1 - assert pending[0].once is True + assert len(fired) == 1 + assert fired[0].once is True @pytest.mark.asyncio async def test_persistent_notification_once_is_false(self) -> None: """Notification from a persistent trigger has once=False.""" mgr = TriggerManager(vin="V") mgr.create(_trig("Soc", TriggerOperator.LT, 20, once=False, cooldown=0)) + fired = _collector(mgr) await mgr.evaluate("Soc", 15.0, 25.0, TS) - pending = mgr.drain_pending() - assert len(pending) == 1 - assert pending[0].once is False + assert len(fired) == 1 + assert fired[0].once is False class TestDelivery: @@ -279,25 +272,6 @@ async def test_callback_invoked(self) -> None: assert isinstance(notification, TriggerNotification) assert notification.value == 15.0 - @pytest.mark.asyncio - async def test_pending_populated(self) -> None: - mgr = TriggerManager(vin="V") - mgr.create(_trig("Soc", TriggerOperator.LT, 20, cooldown=0)) - - await mgr.evaluate("Soc", 15.0, 25.0, TS) - pending = mgr.drain_pending() - assert len(pending) == 1 - assert pending[0].trigger_id - - @pytest.mark.asyncio - async def test_drain_clears(self) -> None: - mgr = TriggerManager(vin="V") - mgr.create(_trig("Soc", TriggerOperator.LT, 20, cooldown=0)) - - await mgr.evaluate("Soc", 15.0, 25.0, TS) - mgr.drain_pending() - assert mgr.drain_pending() == [] - @pytest.mark.asyncio async def test_callback_failure_does_not_block(self) -> None: mgr = TriggerManager(vin="V") @@ -311,27 +285,6 @@ async def test_callback_failure_does_not_block(self) -> None: # Both callbacks called; failure in first doesn't block second bad_cb.assert_awaited_once() good_cb.assert_awaited_once() - # Notification still in pending despite callback failure - assert len(mgr.drain_pending()) == 1 - - -class TestPendingOverflow: - @pytest.mark.asyncio - async def test_oldest_notifications_dropped_on_overflow(self) -> None: - from tescmd.triggers.manager import MAX_PENDING - - mgr = TriggerManager(vin="V") - mgr.create(_trig("Soc", TriggerOperator.CHANGED, None, cooldown=0)) - - # Fill beyond MAX_PENDING - for i in range(MAX_PENDING + 10): - await mgr.evaluate("Soc", float(i), float(i - 1) if i > 0 else None, TS) - - pending = mgr.drain_pending() - assert len(pending) == MAX_PENDING - # Oldest should have been dropped — first pending value should be 10.0 - assert pending[0].value == 10.0 - assert pending[-1].value == float(MAX_PENDING + 9) class TestFieldIndex: @@ -340,30 +293,30 @@ async def test_only_matching_field_evaluated(self) -> None: mgr = TriggerManager(vin="V") mgr.create(_trig("Soc", TriggerOperator.LT, 20, cooldown=0)) mgr.create(_trig("InsideTemp", TriggerOperator.GT, 35, cooldown=0)) + fired = _collector(mgr) await mgr.evaluate("Soc", 15.0, 25.0, TS) - pending = mgr.drain_pending() - assert len(pending) == 1 - assert pending[0].field == "Soc" + assert len(fired) == 1 + assert fired[0].field == "Soc" @pytest.mark.asyncio async def test_multiple_triggers_same_field(self) -> None: mgr = TriggerManager(vin="V") mgr.create(_trig("Soc", TriggerOperator.LT, 20, cooldown=0)) mgr.create(_trig("Soc", TriggerOperator.LT, 10, cooldown=0)) + fired = _collector(mgr) await mgr.evaluate("Soc", 5.0, 25.0, TS) - pending = mgr.drain_pending() # Both triggers should fire (value 5 is < 20 and < 10) - assert len(pending) == 2 + assert len(fired) == 2 @pytest.mark.asyncio async def test_unregistered_field_is_noop(self) -> None: mgr = TriggerManager(vin="V") mgr.create(_trig("Soc", TriggerOperator.LT, 20, cooldown=0)) - await mgr.evaluate("UnknownField", 42, None, TS) - assert mgr.drain_pending() == [] + result = await mgr.evaluate("UnknownField", 42, None, TS) + assert result is False class TestGeofence: @@ -381,8 +334,8 @@ async def test_enter_fires_on_crossing_inward(self) -> None: prev = {"latitude": 37.79, "longitude": -122.4194} # ~1.7km away curr = {"latitude": 37.7749, "longitude": -122.4194} # at center - await mgr.evaluate("Location", curr, prev, TS) - assert len(mgr.drain_pending()) == 1 + result = await mgr.evaluate("Location", curr, prev, TS) + assert result is True @pytest.mark.asyncio async def test_leave_fires_on_crossing_outward(self) -> None: @@ -393,8 +346,8 @@ async def test_leave_fires_on_crossing_outward(self) -> None: prev = {"latitude": 37.7749, "longitude": -122.4194} # at center curr = {"latitude": 37.79, "longitude": -122.4194} # ~1.7km away - await mgr.evaluate("Location", curr, prev, TS) - assert len(mgr.drain_pending()) == 1 + result = await mgr.evaluate("Location", curr, prev, TS) + assert result is True @pytest.mark.asyncio async def test_no_fire_without_previous(self) -> None: @@ -404,8 +357,8 @@ async def test_no_fire_without_previous(self) -> None: curr = {"latitude": 37.7749, "longitude": -122.4194} - await mgr.evaluate("Location", curr, None, TS) - assert mgr.drain_pending() == [] + result = await mgr.evaluate("Location", curr, None, TS) + assert result is False @pytest.mark.asyncio async def test_no_fire_when_already_inside(self) -> None: @@ -417,8 +370,8 @@ async def test_no_fire_when_already_inside(self) -> None: prev = {"latitude": 37.7750, "longitude": -122.4194} curr = {"latitude": 37.7749, "longitude": -122.4194} - await mgr.evaluate("Location", curr, prev, TS) - assert mgr.drain_pending() == [] + result = await mgr.evaluate("Location", curr, prev, TS) + assert result is False @pytest.mark.asyncio async def test_no_fire_when_already_outside(self) -> None: @@ -430,8 +383,8 @@ async def test_no_fire_when_already_outside(self) -> None: prev = {"latitude": 37.80, "longitude": -122.4194} # ~2.8km away curr = {"latitude": 37.79, "longitude": -122.4194} # ~1.7km away - await mgr.evaluate("Location", curr, prev, TS) - assert mgr.drain_pending() == [] + result = await mgr.evaluate("Location", curr, prev, TS) + assert result is False @pytest.mark.asyncio async def test_haversine_accuracy(self) -> None: @@ -505,13 +458,13 @@ class TestEvaluateSingle: async def test_returns_true_on_match(self) -> None: mgr = TriggerManager(vin="V") t = mgr.create(_trig("BatteryLevel", TriggerOperator.LT, 20)) + fired = _collector(mgr) - fired = await mgr.evaluate_single(t.id, 15.0, None, TS) + result = await mgr.evaluate_single(t.id, 15.0, None, TS) - assert fired is True - pending = mgr.drain_pending() - assert len(pending) == 1 - assert pending[0].value == 15.0 + assert result is True + assert len(fired) == 1 + assert fired[0].value == 15.0 @pytest.mark.asyncio async def test_returns_false_on_no_match(self) -> None: @@ -521,7 +474,6 @@ async def test_returns_false_on_no_match(self) -> None: fired = await mgr.evaluate_single(t.id, 50.0, None, TS) assert fired is False - assert len(mgr.drain_pending()) == 0 @pytest.mark.asyncio async def test_returns_false_for_missing_id(self) -> None: @@ -557,20 +509,7 @@ async def test_calls_fire_callbacks(self) -> None: assert cb.call_args[0][0].field == "BatteryLevel" -class TestQueueNotification: - """Tests for queue_notification and vin property.""" - - def test_queue_notification(self) -> None: - mgr = TriggerManager(vin="V") - n = TriggerNotification( - trigger_id="t1", field="f", operator=TriggerOperator.LT, - threshold=20, value=15, vin="V", - ) - mgr.queue_notification(n) - pending = mgr.drain_pending() - assert len(pending) == 1 - assert pending[0].trigger_id == "t1" - +class TestVinProperty: def test_vin_property(self) -> None: mgr = TriggerManager(vin="MYVIN") assert mgr.vin == "MYVIN" From 2decf373209e52d24baeedb45dcf0da2cf11e90d Mon Sep 17 00:00:00 2001 From: Sean McLellan Date: Wed, 4 Feb 2026 23:32:36 -0500 Subject: [PATCH 3/3] fix: harden error handling, port conflicts, and API consistency - Port-in-use crash: pre-check with _resolve_port(), auto-select free port when default is occupied, clear UsageError for explicit --port - SystemExit from uvicorn caught and converted to OSError; clean shutdown (code 0) logged at debug level and passed through - Environment-sourced --port treated as explicit (not auto-select) - Notification serialization guards narrowed from except Exception to (AttributeError, TypeError, ValueError) in push callback and flush - Pending push queue overflow logs dropped notification before eviction - flush_pending_push guarded after reconnect so one bad notification cannot break recovery - system.run unknown method bumped to warning with resolved name - contextlib.suppress replaced with explicit try/except + debug log - p.pop("field") mutation fixed with p.get() + filtered dict copy - _matches() renamed to matches() as public API - Shared _internal/units.py replaces 3 duplicate temp conversions - telemetry_get distinguishes store-unavailable from pending - Empty exception message shows "Unexpected {Type}" + --verbose hint Co-Authored-By: Claude Opus 4.5 --- CHANGELOG.md | 21 +++++ src/tescmd/_internal/units.py | 13 +++ src/tescmd/cli/main.py | 13 ++- src/tescmd/cli/serve.py | 119 +++++++++++++++++++-------- src/tescmd/mcp/server.py | 8 +- src/tescmd/openclaw/bridge.py | 44 ++++++++-- src/tescmd/openclaw/dispatcher.py | 33 ++++---- src/tescmd/openclaw/emitter.py | 4 +- src/tescmd/triggers/manager.py | 6 +- tests/cli/test_serve_port.py | 130 ++++++++++++++++++++++++++++++ tests/openclaw/test_dispatcher.py | 11 +-- tests/triggers/test_manager.py | 34 ++++---- 12 files changed, 349 insertions(+), 87 deletions(-) create mode 100644 src/tescmd/_internal/units.py create mode 100644 tests/cli/test_serve_port.py diff --git a/CHANGELOG.md b/CHANGELOG.md index a31abed..c6f8e57 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,9 +19,30 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **`trigger.poll` endpoint and pending queue** — trigger notifications are delivered exclusively via push callbacks over the OpenClaw WebSocket; the MCP `trigger_poll` tool, `drain_pending()`, and the in-memory pending notification queue have been removed +### Changed + +- **`matches()` is now a public API** — renamed from `_matches()` in `triggers/manager.py` so cross-module consumers (MCP trigger tools) import a stable public symbol +- **Shared temperature conversion utility** — `_internal/units.py` provides `fahrenheit_to_celsius()` and `celsius_to_fahrenheit()` replacing three duplicate implementations across `dispatcher.py`, `serve.py`, and `emitter.py` +- **`telemetry_get` distinguishes store-unavailable from pending** — returns `{"error": "telemetry_store_unavailable", "pending": false}` when no telemetry store is configured instead of the misleading `{"pending": true}` + ### Fixed - **TUI panel widgets not updating** — telemetry panel DataTables now update per-frame as data arrives instead of relying solely on a 1-second timer poll; header, server info, and trigger displays remain on the timer +- **Port-in-use crash** — `tescmd serve` now pre-checks port availability with `_resolve_port()` and auto-selects a free port when the default is occupied; explicit `--port` raises a clear `UsageError` with a suggested alternative +- **`SystemExit` killing the event loop** — uvicorn's `sys.exit(1)` on bind failure is now caught by `_safe_uvicorn_serve()` and converted to `OSError`; clean shutdown (`exit(0)`) passes through without error +- **Empty "Error:" crash** — generic error handler in `main.py` now shows `"Unexpected {ExceptionType}"` when `str(exc)` is empty and suggests `--verbose` for the full traceback +- **`trigger.poll` ValueError** — removed the defunct `trigger.poll` handler that raised `ValueError` when called; `system.run` now returns `None` for unknown inner methods instead of raising +- **Trigger notification queue overflow silent data loss** — `_pending_push` deque now logs a warning when at capacity before evicting the oldest notification +- **`flush_pending_push` crash after reconnect** — flush is now wrapped in try/except so a single malformed notification cannot break the reconnect recovery path +- **Notification serialization crash blocks all deliveries** — event dict construction in both push callback and flush loop is now guarded per-notification; a bad entry is logged and discarded instead of blocking subsequent notifications +- **`p.pop("field")` mutates MCP params** — generic `trigger_create` tool now uses `p.get()` with a filtered dict copy to avoid mutating the framework's params dict on retries +- **Unknown `system.run` method logged at wrong level** — bumped from `info` to `warning` so operators monitoring at warning level see protocol mismatches +- **`contextlib.suppress` hiding conversion bugs** — replaced with explicit try/except + debug-level logging in both dispatcher and MCP trigger list paths +- **Reconnect failure logged without traceback** — added `exc_info=True` to reconnect and Tailscale auto-detection warning logs +- **Notification serialization guard too broad** — narrowed `except Exception` to `(AttributeError, TypeError, ValueError)` in both push callback and flush loop so unexpected errors (e.g. `ConnectionError`) propagate instead of being silently discarded +- **`system.run` unknown method log missing resolved name** — warning now includes both the raw and resolved method names for easier debugging +- **`SystemExit(0)` silent return** — clean uvicorn shutdowns now log at debug level in both `serve.py` and `server.py` for traceability +- **Environment-sourced `--port` treated as auto-select** — ports set via `TESLA_MCP_PORT` env var are now treated as explicit (error on conflict) instead of silently picking an alternative port ## [0.5.0] - 2026-02-03 diff --git a/src/tescmd/_internal/units.py b/src/tescmd/_internal/units.py new file mode 100644 index 0000000..a562886 --- /dev/null +++ b/src/tescmd/_internal/units.py @@ -0,0 +1,13 @@ +"""Unit conversion helpers shared across the codebase.""" + +from __future__ import annotations + + +def fahrenheit_to_celsius(f: float) -> float: + """Convert Fahrenheit to Celsius, rounded to 1 decimal place.""" + return round((f - 32.0) * 5.0 / 9.0, 1) + + +def celsius_to_fahrenheit(c: float) -> float: + """Convert Celsius to Fahrenheit, rounded to 1 decimal place.""" + return round(c * 9.0 / 5.0 + 32.0, 1) diff --git a/src/tescmd/cli/main.py b/src/tescmd/cli/main.py index 8585b81..cda72eb 100644 --- a/src/tescmd/cli/main.py +++ b/src/tescmd/cli/main.py @@ -237,11 +237,20 @@ def main(argv: list[str] | None = None) -> None: if _handle_known_error(exc, app_ctx, formatter, cmd_name): raise SystemExit(1) from exc + exc_type = type(exc).__name__ + exc_msg = str(exc) + # Never show an empty error — always include the exception class. + message = exc_msg if exc_msg else f"Unexpected {exc_type}" formatter.output_error( - code=type(exc).__name__, - message=str(exc), + code=exc_type, + message=message, command=cmd_name, ) + if not exc_msg and formatter.format != "json": + # The message was empty — nudge toward --verbose for the traceback. + formatter.rich.info( + "[dim]Run with --verbose to see the full traceback.[/dim]" + ) raise SystemExit(1) from exc diff --git a/src/tescmd/cli/serve.py b/src/tescmd/cli/serve.py index c73c0e6..a3dea7f 100644 --- a/src/tescmd/cli/serve.py +++ b/src/tescmd/cli/serve.py @@ -2,7 +2,6 @@ from __future__ import annotations -import contextlib import logging import random import signal @@ -20,6 +19,56 @@ logger = logging.getLogger(__name__) +def _resolve_port(host: str, preferred: int, *, auto_select: bool = True) -> int: + """Return *preferred* if available, or find a free port. + + When *auto_select* is ``True`` (default port in use), the OS picks + a free port. When ``False`` (user explicitly chose a port), raise + ``click.UsageError`` with an actionable message. + """ + import socket + + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s: + s.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) + try: + s.bind((host, preferred)) + return preferred + except OSError: + pass + + if not auto_select: + raise click.UsageError( + f"Port {preferred} is already in use.\n" + f"Use --port to specify a different port, e.g.:\n" + f" tescmd serve --port {preferred + 1}" + ) + + # OS picks a free port + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s: + s.bind((host, 0)) + free_port = s.getsockname()[1] + + logger.info("Port %d in use — using port %d instead", preferred, free_port) + return free_port + + +async def _safe_uvicorn_serve(server: Any, port: int) -> None: + """Run uvicorn.Server.serve() with SystemExit protection. + + Uvicorn calls ``sys.exit(1)`` when it cannot bind the port. + ``SystemExit`` is a ``BaseException`` that kills the asyncio event + loop before the owning task can retrieve the exception. This + wrapper converts it to a regular ``OSError``. + """ + try: + await server.serve() + except SystemExit as exc: + if exc.code == 0: + logger.debug("Uvicorn exited cleanly (code 0) on port %d", port) + return + raise OSError(f"MCP server failed to start on port {port}") from exc + + @click.command("serve") @click.argument("vin_positional", required=False, default=None, metavar="VIN") @click.option( @@ -201,6 +250,12 @@ def serve_cmd( if openclaw_config_path and not openclaw_url: raise click.UsageError("--openclaw-config requires --openclaw.") + port_source = click.get_current_context().get_parameter_source("port") + port_explicit = port_source in ( + click.core.ParameterSource.COMMANDLINE, + click.core.ParameterSource.ENVIRONMENT, + ) + run_async( _cmd_serve( app_ctx, @@ -208,6 +263,7 @@ def serve_cmd( transport=transport, mcp_port=port, mcp_host=host, + port_explicit=port_explicit, telemetry_port=telemetry_port, fields_spec=fields, interval_override=interval, @@ -233,6 +289,7 @@ async def _cmd_serve( transport: str, mcp_port: int, mcp_host: str = "127.0.0.1", + port_explicit: bool = False, telemetry_port: int | None, fields_spec: str, interval_override: int | None, @@ -272,6 +329,10 @@ async def _cmd_serve( mcp_server = create_mcp_server(client_id=client_id, client_secret=client_secret) tool_count = len(mcp_server.list_tools()) + # -- Resolve MCP port (pre-check for port conflicts) -- + if not no_mcp and transport != "stdio": + mcp_port = _resolve_port(mcp_host, mcp_port, auto_select=not port_explicit) + # -- stdio mode: no telemetry, no fanout -- if transport == "stdio" and mcp_server is not None: print(f"tescmd serve starting (stdio, {tool_count} tools)", file=sys.stderr) @@ -486,7 +547,7 @@ async def _trigger_sink(frame: object) -> None: _pre_hostname = await _ts_pre.get_hostname() public_url = f"https://{_pre_hostname}" except Exception: - logger.warning("Tailscale auto-detection failed — using localhost") + logger.warning("Tailscale auto-detection failed — using localhost", exc_info=True) # -- Populate TUI with server info -- if tui is not None: @@ -566,7 +627,9 @@ def _handle_sigterm() -> None: combined_app, host=mcp_host, port=mcp_port, log_level="warning" ) _uvi_server = uvicorn.Server(_uvi_cfg) - combined_task = asyncio.create_task(_uvi_server.serve()) + combined_task = asyncio.create_task( + _safe_uvicorn_serve(_uvi_server, mcp_port) + ) # Give uvicorn a moment to bind the port. await asyncio.sleep(0.5) if combined_task.done(): @@ -671,20 +734,14 @@ def _register_trigger_tools( One-shot triggers are **not** deleted immediately — the push callback handles deletion after confirmed WebSocket delivery. """ - from tescmd.triggers.manager import _matches + from tescmd._internal.units import celsius_to_fahrenheit, fahrenheit_to_celsius + from tescmd.triggers.manager import matches from tescmd.triggers.models import ( TriggerCondition, TriggerDefinition, - TriggerNotification, TriggerOperator, ) - def _f_to_c(f: float) -> float: - return round((f - 32.0) * 5.0 / 9.0, 1) - - def _c_to_f(c: float) -> float: - return round(c * 9.0 / 5.0 + 32.0, 1) - def _create_trigger( field: str, params: dict[str, Any], *, convert_temp: bool = False ) -> dict[str, Any]: @@ -693,7 +750,7 @@ def _create_trigger( raise ValueError("Trigger requires 'operator' parameter") value = params.get("value") if convert_temp and value is not None: - value = _f_to_c(float(value)) + value = fahrenheit_to_celsius(float(value)) condition = TriggerCondition( field=field, operator=TriggerOperator(op_str), @@ -708,26 +765,12 @@ def _create_trigger( result = dict(created.model_dump(mode="json")) # Immediate evaluation: if the telemetry store already has a - # value that satisfies the condition, fire now. - # One-shot triggers are NOT deleted here — the push callback - # handles deletion after confirmed WebSocket delivery. + # value that satisfies the condition, report it. One-shot + # triggers are marked as fired; the push callback handles + # deletion after confirmed WebSocket delivery. if telemetry_store is not None: snap = telemetry_store.get(field) - if snap is not None and _matches(condition, snap.value, None): - from datetime import UTC, datetime - - notification = TriggerNotification( - trigger_id=created.id, - field=field, - operator=condition.operator, - threshold=condition.value, - value=snap.value, - previous_value=None, - fired_at=datetime.now(UTC), - vin=trigger_manager.vin, - once=trigger.once, - ) - trigger_manager.queue_notification(notification) + if snap is not None and matches(condition, snap.value, None): result["immediate"] = True if trigger.once: trigger_manager.mark_fired_once(created.id) @@ -751,8 +794,14 @@ def _list_triggers( "cooldown_seconds": t.cooldown_seconds, } if show_fahrenheit and t.condition.value is not None: - with contextlib.suppress(TypeError, ValueError): - entry["value_f"] = _c_to_f(float(t.condition.value)) + try: + entry["value_f"] = celsius_to_fahrenheit(float(t.condition.value)) + except (TypeError, ValueError): + logger.debug( + "Could not convert trigger %s value %r to Fahrenheit", + t.id, + t.condition.value, + ) result.append(entry) return {"triggers": result} @@ -989,7 +1038,9 @@ def _delete_trigger(params: dict[str, Any]) -> dict[str, Any]: mcp_server.register_custom_tool( "trigger_create", - lambda p: _create_trigger(p.pop("field", ""), p), + lambda p: _create_trigger( + p.get("field", ""), {k: v for k, v in p.items() if k != "field"} + ), "Create a trigger on any telemetry field", _generic_trigger_schema, is_write=True, @@ -1026,7 +1077,7 @@ def _handle_telemetry_get(params: dict[str, Any]) -> dict[str, Any]: if not field: raise ValueError("telemetry_get requires 'field' parameter") if telemetry_store is None: - return {"field": field, "pending": True} + return {"field": field, "error": "telemetry_store_unavailable", "pending": False} snap = telemetry_store.get(field) if snap is None: return {"field": field, "pending": True} diff --git a/src/tescmd/mcp/server.py b/src/tescmd/mcp/server.py index e2c415d..29cd845 100644 --- a/src/tescmd/mcp/server.py +++ b/src/tescmd/mcp/server.py @@ -388,7 +388,13 @@ async def run_http( app = self.create_http_app(host=host, port=port, public_url=public_url) config = uvicorn.Config(app, host=host, port=port, log_level="warning") server = uvicorn.Server(config) - await server.serve() + try: + await server.serve() + except SystemExit as exc: + if exc.code == 0: + logger.debug("Uvicorn exited cleanly (code 0) on port %d", port) + return + raise OSError(f"MCP server failed to start on port {port}") from exc def _register_fastmcp_tool( self, diff --git a/src/tescmd/openclaw/bridge.py b/src/tescmd/openclaw/bridge.py index c82038e..f9c8423 100644 --- a/src/tescmd/openclaw/bridge.py +++ b/src/tescmd/openclaw/bridge.py @@ -96,6 +96,7 @@ async def _maybe_reconnect(self) -> None: logger.warning( "Reconnection failed — next attempt in %.0fs", self._reconnect_backoff, + exc_info=True, ) self._reconnect_backoff = min(self._reconnect_backoff * 2, _RECONNECT_MAX) return @@ -107,7 +108,14 @@ async def _maybe_reconnect(self) -> None: logger.warning("Failed to send connected event after reconnect", exc_info=True) # Flush any queued trigger notifications now that WS is available. if self._pending_push: - await self.flush_pending_push() + try: + await self.flush_pending_push() + except Exception: + logger.warning( + "Failed to flush %d pending push notification(s) after reconnect", + len(self._pending_push), + exc_info=True, + ) def _build_lifecycle_event(self, event_type: str) -> dict[str, Any]: """Build a ``req:agent`` lifecycle event (connecting/disconnecting).""" @@ -145,8 +153,8 @@ def make_trigger_push_callback(self) -> Any: async def _push_trigger_notification(n: Any) -> None: if gateway.is_connected: - await gateway.send_event( - { + try: + event = { "method": "tescmd.trigger.fired", "params": { "trigger_id": n.trigger_id, @@ -157,7 +165,14 @@ async def _push_trigger_notification(n: Any) -> None: "fired_at": n.fired_at.isoformat(), }, } - ) + except (AttributeError, TypeError, ValueError): + logger.error( + "Malformed trigger notification — discarding: %r", + n, + exc_info=True, + ) + return + await gateway.send_event(event) # send_event() never raises — check is_connected to detect failure. if gateway.is_connected: logger.info( @@ -176,6 +191,13 @@ async def _push_trigger_notification(n: Any) -> None: n.trigger_id, ) + if len(pending_push) == pending_push.maxlen: + dropped = pending_push[0] + logger.warning( + "Pending push queue full (%d) — dropping oldest: trigger=%s", + len(pending_push), + dropped.trigger_id, + ) pending_push.append(n) logger.warning( "Trigger notification queued: trigger=%s", n.trigger_id, @@ -201,8 +223,8 @@ async def flush_pending_push(self) -> int: while self._pending_push: n = self._pending_push[0] # peek without removing - await self._gateway.send_event( - { + try: + event = { "method": "tescmd.trigger.fired", "params": { "trigger_id": n.trigger_id, @@ -213,7 +235,15 @@ async def flush_pending_push(self) -> int: "fired_at": n.fired_at.isoformat(), }, } - ) + except (AttributeError, TypeError, ValueError): + logger.error( + "Malformed notification in push queue — discarding: %r", + n, + exc_info=True, + ) + self._pending_push.popleft() + continue + await self._gateway.send_event(event) # send_event() never raises — check is_connected to detect failure. if not self._gateway.is_connected: logger.warning("Flush stopped at %d/%d", sent, total) diff --git a/src/tescmd/openclaw/dispatcher.py b/src/tescmd/openclaw/dispatcher.py index 10a31c0..651f408 100644 --- a/src/tescmd/openclaw/dispatcher.py +++ b/src/tescmd/openclaw/dispatcher.py @@ -9,10 +9,10 @@ from __future__ import annotations import asyncio -import contextlib import logging from typing import TYPE_CHECKING, Any +from tescmd._internal.units import celsius_to_fahrenheit, fahrenheit_to_celsius from tescmd.api.errors import VehicleAsleepError from tescmd.cli._client import ( check_command_guards, @@ -30,16 +30,6 @@ logger = logging.getLogger(__name__) -def _fahrenheit_to_celsius(f: float) -> float: - """Convert Fahrenheit to Celsius, rounded to 1 decimal place.""" - return round((f - 32.0) * 5.0 / 9.0, 1) - - -def _celsius_to_fahrenheit(c: float) -> float: - """Convert Celsius to Fahrenheit, rounded to 1 decimal place.""" - return round(c * 9.0 / 5.0 + 32.0, 1) - - # API snake_case → OpenClaw dot notation aliases for system.run _METHOD_ALIASES: dict[str, str] = { "door_lock": "door.lock", @@ -458,7 +448,7 @@ async def _handle_homelink(self, params: dict[str, Any]) -> dict[str, Any]: # -- Meta-dispatch handler ----------------------------------------------- - async def _handle_system_run(self, params: dict[str, Any]) -> dict[str, Any]: + async def _handle_system_run(self, params: dict[str, Any]) -> dict[str, Any] | None: """Invoke any registered handler by name. Accepts both OpenClaw-style (``door.lock``) and API-style @@ -466,6 +456,9 @@ async def _handle_system_run(self, params: dict[str, Any]) -> dict[str, Any]: The target method can be specified as ``method`` or ``command`` (the latter mirrors the gateway protocol's field name). + + Returns ``None`` for unknown inner methods so the gateway can + send a clean error response without a traceback. """ raw = params.get("method", "") or params.get("command", "") # Normalize: bots may send a list like ["door.lock"] instead of a string @@ -481,7 +474,7 @@ async def _handle_system_run(self, params: dict[str, Any]) -> dict[str, Any]: inner_params = params.get("params", {}) result = await self.dispatch({"method": resolved, "params": inner_params}) if result is None: - raise ValueError(f"Unknown method: {method}") + logger.warning("system.run: unknown inner method %s (resolved: %s)", method, resolved) return result # -- Trigger handlers ---------------------------------------------------- @@ -581,8 +574,14 @@ def _list_triggers_for_field( "cooldown_seconds": t.cooldown_seconds, } if show_fahrenheit and t.condition.value is not None: - with contextlib.suppress(TypeError, ValueError): - entry["value_f"] = _celsius_to_fahrenheit(float(t.condition.value)) + try: + entry["value_f"] = celsius_to_fahrenheit(float(t.condition.value)) + except (TypeError, ValueError): + logger.debug( + "Could not convert trigger %s value %r to Fahrenheit", + t.id, + t.condition.value, + ) result.append(entry) return {"triggers": result} @@ -621,7 +620,7 @@ async def _handle_cabin_temp_trigger(self, params: dict[str, Any]) -> dict[str, converted = {**params, "field": "InsideTemp"} if "value" in converted and converted["value"] is not None: f_val = float(converted["value"]) - converted["value"] = _fahrenheit_to_celsius(f_val) + converted["value"] = fahrenheit_to_celsius(f_val) logger.debug( "cabin_temp.trigger: converted %.1f°F → %.1f°C", f_val, converted["value"] ) @@ -631,7 +630,7 @@ async def _handle_outside_temp_trigger(self, params: dict[str, Any]) -> dict[str converted = {**params, "field": "OutsideTemp"} if "value" in converted and converted["value"] is not None: f_val = float(converted["value"]) - converted["value"] = _fahrenheit_to_celsius(f_val) + converted["value"] = fahrenheit_to_celsius(f_val) logger.debug( "outside_temp.trigger: converted %.1f°F → %.1f°C", f_val, converted["value"] ) diff --git a/src/tescmd/openclaw/emitter.py b/src/tescmd/openclaw/emitter.py index 9081155..9dce6b1 100644 --- a/src/tescmd/openclaw/emitter.py +++ b/src/tescmd/openclaw/emitter.py @@ -38,7 +38,9 @@ def _to_snake_case(name: str) -> str: def _celsius_to_fahrenheit(c: float) -> float: - return c * 9.0 / 5.0 + 32.0 + from tescmd._internal.units import celsius_to_fahrenheit + + return celsius_to_fahrenheit(c) class EventEmitter: diff --git a/src/tescmd/triggers/manager.py b/src/tescmd/triggers/manager.py index 6bf40ea..b746954 100644 --- a/src/tescmd/triggers/manager.py +++ b/src/tescmd/triggers/manager.py @@ -129,7 +129,7 @@ async def evaluate_single( if trigger is None: return False - if not _matches(trigger.condition, value, previous_value): + if not matches(trigger.condition, value, previous_value): return False now = time.monotonic() @@ -206,7 +206,7 @@ async def evaluate( if last_fire is not None and (now - last_fire) < trigger.cooldown_seconds: continue - if not _matches(trigger.condition, value, previous_value): + if not matches(trigger.condition, value, previous_value): continue # Fire! @@ -249,7 +249,7 @@ async def evaluate( return fired -def _matches(condition: TriggerCondition, value: Any, previous_value: Any) -> bool: +def matches(condition: TriggerCondition, value: Any, previous_value: Any) -> bool: """Check whether a value satisfies a trigger condition.""" op = condition.operator diff --git a/tests/cli/test_serve_port.py b/tests/cli/test_serve_port.py new file mode 100644 index 0000000..52491f4 --- /dev/null +++ b/tests/cli/test_serve_port.py @@ -0,0 +1,130 @@ +"""Tests for port resolution and conflict handling in ``tescmd serve``.""" + +from __future__ import annotations + +import socket +from unittest.mock import AsyncMock, MagicMock + +import click +import pytest + + +class TestResolvePort: + """Unit tests for ``_resolve_port()``.""" + + def test_available_port_returned(self) -> None: + """When the preferred port is free, return it unchanged.""" + from tescmd.cli.serve import _resolve_port + + # Use port 0 to let the OS pick a free port, then verify + # _resolve_port returns it when it's actually free. + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s: + s.bind(("127.0.0.1", 0)) + free_port = s.getsockname()[1] + # Port is now released — _resolve_port should succeed. + result = _resolve_port("127.0.0.1", free_port, auto_select=True) + assert result == free_port + + def test_occupied_port_auto_selects(self) -> None: + """When the port is in use and auto_select=True, return a different port.""" + from tescmd.cli.serve import _resolve_port + + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s: + s.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) + s.bind(("127.0.0.1", 0)) + occupied_port = s.getsockname()[1] + s.listen(1) + + result = _resolve_port("127.0.0.1", occupied_port, auto_select=True) + assert result != occupied_port + assert 1 <= result <= 65535 + + def test_occupied_port_raises_when_explicit(self) -> None: + """When the port is in use and auto_select=False, raise UsageError.""" + from tescmd.cli.serve import _resolve_port + + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s: + s.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) + s.bind(("127.0.0.1", 0)) + occupied_port = s.getsockname()[1] + s.listen(1) + + with pytest.raises(click.UsageError, match="already in use"): + _resolve_port("127.0.0.1", occupied_port, auto_select=False) + + def test_usage_error_suggests_next_port(self) -> None: + """The error message should suggest the next port number.""" + from tescmd.cli.serve import _resolve_port + + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s: + s.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) + s.bind(("127.0.0.1", 0)) + occupied_port = s.getsockname()[1] + s.listen(1) + + with pytest.raises(click.UsageError, match=str(occupied_port + 1)): + _resolve_port("127.0.0.1", occupied_port, auto_select=False) + + +class TestSafeUvicornServe: + """Unit tests for ``_safe_uvicorn_serve()``.""" + + @pytest.mark.asyncio + async def test_system_exit_becomes_os_error(self) -> None: + """SystemExit from uvicorn should be caught and re-raised as OSError.""" + from tescmd.cli.serve import _safe_uvicorn_serve + + mock_server = MagicMock() + mock_server.serve = AsyncMock(side_effect=SystemExit(1)) + + with pytest.raises(OSError, match="MCP server failed to start on port 8080"): + await _safe_uvicorn_serve(mock_server, 8080) + + @pytest.mark.asyncio + async def test_normal_completion_passes_through(self) -> None: + """When serve() completes normally, no error is raised.""" + from tescmd.cli.serve import _safe_uvicorn_serve + + mock_server = MagicMock() + mock_server.serve = AsyncMock(return_value=None) + + # Should not raise + await _safe_uvicorn_serve(mock_server, 8080) + mock_server.serve.assert_awaited_once() + + @pytest.mark.asyncio + async def test_other_exceptions_propagate(self) -> None: + """Non-SystemExit exceptions should propagate unchanged.""" + from tescmd.cli.serve import _safe_uvicorn_serve + + mock_server = MagicMock() + mock_server.serve = AsyncMock(side_effect=RuntimeError("something else")) + + with pytest.raises(RuntimeError, match="something else"): + await _safe_uvicorn_serve(mock_server, 8080) + + +class TestMCPServerRunHttpSystemExit: + """Verify MCPServer.run_http() catches SystemExit.""" + + @pytest.mark.asyncio + async def test_system_exit_becomes_os_error(self) -> None: + """SystemExit from uvicorn in run_http() should become OSError.""" + from unittest.mock import patch + + from tescmd.mcp.server import MCPServer + + server = MCPServer(client_id="test-id", client_secret="test-secret") + + mock_uvi_server = MagicMock() + mock_uvi_server.serve = AsyncMock(side_effect=SystemExit(1)) + + mock_uvicorn = MagicMock() + mock_uvicorn.Config.return_value = MagicMock() + mock_uvicorn.Server.return_value = mock_uvi_server + + with ( + patch.dict("sys.modules", {"uvicorn": mock_uvicorn}), + pytest.raises(OSError, match="MCP server failed to start on port 9999"), + ): + await server.run_http(host="127.0.0.1", port=9999) diff --git a/tests/openclaw/test_dispatcher.py b/tests/openclaw/test_dispatcher.py index 7d72962..a559c8d 100644 --- a/tests/openclaw/test_dispatcher.py +++ b/tests/openclaw/test_dispatcher.py @@ -894,13 +894,14 @@ async def test_system_run_resolves_api_style_alias(self) -> None: assert result["result"] is True @pytest.mark.asyncio - async def test_system_run_unknown_method_raises(self) -> None: + async def test_system_run_unknown_method_returns_none(self) -> None: + """Unknown inner methods return None (no traceback) for clean gateway handling.""" ctx = _mock_app_ctx() d = CommandDispatcher(vin="VIN1", app_ctx=ctx) - with pytest.raises(ValueError, match="Unknown method"): - await d.dispatch( - {"method": "system.run", "params": {"method": "nonexistent.cmd", "params": {}}} - ) + result = await d.dispatch( + {"method": "system.run", "params": {"method": "nonexistent.cmd", "params": {}}} + ) + assert result is None @pytest.mark.asyncio async def test_system_run_self_dispatch_rejected(self) -> None: diff --git a/tests/triggers/test_manager.py b/tests/triggers/test_manager.py index a01a236..cc06391 100644 --- a/tests/triggers/test_manager.py +++ b/tests/triggers/test_manager.py @@ -9,7 +9,7 @@ import pytest -from tescmd.triggers.manager import TriggerLimitError, TriggerManager, _matches +from tescmd.triggers.manager import TriggerLimitError, TriggerManager, matches from tescmd.triggers.models import ( TriggerCondition, TriggerDefinition, @@ -400,55 +400,55 @@ async def test_haversine_accuracy(self) -> None: class TestMatches: - """Direct tests for the _matches() helper.""" + """Direct tests for the matches() helper.""" def test_changed_different(self) -> None: c = _cond("f", TriggerOperator.CHANGED) - assert _matches(c, "a", "b") is True + assert matches(c, "a", "b") is True def test_changed_same(self) -> None: c = _cond("f", TriggerOperator.CHANGED) - assert _matches(c, "a", "a") is False + assert matches(c, "a", "a") is False def test_eq_match(self) -> None: c = _cond("f", TriggerOperator.EQ, "x") - assert _matches(c, "x", None) is True + assert matches(c, "x", None) is True def test_eq_no_match(self) -> None: c = _cond("f", TriggerOperator.EQ, "x") - assert _matches(c, "y", None) is False + assert matches(c, "y", None) is False def test_neq_match(self) -> None: c = _cond("f", TriggerOperator.NEQ, "x") - assert _matches(c, "y", None) is True + assert matches(c, "y", None) is True def test_lt_numeric(self) -> None: c = _cond("f", TriggerOperator.LT, 20) - assert _matches(c, 15, None) is True - assert _matches(c, 25, None) is False + assert matches(c, 15, None) is True + assert matches(c, 25, None) is False def test_gt_numeric(self) -> None: c = _cond("f", TriggerOperator.GT, 80) - assert _matches(c, 85, None) is True - assert _matches(c, 75, None) is False + assert matches(c, 85, None) is True + assert matches(c, 75, None) is False def test_lte_boundary(self) -> None: c = _cond("f", TriggerOperator.LTE, 20) - assert _matches(c, 20, None) is True - assert _matches(c, 21, None) is False + assert matches(c, 20, None) is True + assert matches(c, 21, None) is False def test_gte_boundary(self) -> None: c = _cond("f", TriggerOperator.GTE, 80) - assert _matches(c, 80, None) is True - assert _matches(c, 79, None) is False + assert matches(c, 80, None) is True + assert matches(c, 79, None) is False def test_non_numeric_returns_false(self) -> None: c = _cond("f", TriggerOperator.LT, 20) - assert _matches(c, "abc", None) is False + assert matches(c, "abc", None) is False def test_string_numeric_coercion(self) -> None: c = _cond("f", TriggerOperator.GT, "80") - assert _matches(c, "85", None) is True + assert matches(c, "85", None) is True class TestEvaluateSingle: