From 6805c75a7a3e1f0c35e6b6df8149fa269d917c1c Mon Sep 17 00:00:00 2001 From: Luke Date: Mon, 6 Jul 2026 07:54:07 -0400 Subject: [PATCH 1/4] feat: accept in-memory PEM cert/key material in HarborCameraConfig HarborCameraConfig gains cert_pem/key_pem fields so consumers can pass certificate material directly instead of persisting private keys to disk. cert_path/key_path/cert_dir are now optional; the config validates that one complete pair is provided. Since ssl.SSLContext.load_cert_chain only accepts file paths, PEM data is staged in a private temp directory (0600 files) that is removed before build_ssl_context returns, so the temp-file lifecycle is fully encapsulated in the library. The ssl_context_cache is now keyed off the certificate material (SHA-256 of the PEM pair, or the path pair for path-based configs) via the new get_ssl_cache_key(), so rotated credentials never reuse a stale context. The test fixture in tests/data/certs.py is a throwaway self-signed cert generated for the suite; it is excluded from detect-private-key. --- .pre-commit-config.yaml | 2 + harbor/config.py | 27 +++++++++++-- harbor/utils.py | 52 ++++++++++++++++++++++--- tests/data/certs.py | 24 ++++++++++++ tests/test_config.py | 84 +++++++++++++++++++++++++++++++++++++++++ 5 files changed, 181 insertions(+), 8 deletions(-) create mode 100644 tests/data/certs.py create mode 100644 tests/test_config.py diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 755f4f9..edbbbfd 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -15,6 +15,8 @@ repos: - id: check-toml - id: check-yaml - id: detect-private-key + # Throwaway self-signed key generated for the test suite only + exclude: ^tests/data/certs\.py$ - id: end-of-file-fixer - id: trailing-whitespace - repo: https://github.com/astral-sh/uv-pre-commit diff --git a/harbor/config.py b/harbor/config.py index 5bf49b8..6eb50b1 100644 --- a/harbor/config.py +++ b/harbor/config.py @@ -3,9 +3,30 @@ @dataclass(frozen=True) class HarborCameraConfig: + """Connection configuration for a Harbor camera. + + Client certificate material can be provided either as in-memory PEM + strings (``cert_pem``/``key_pem``) or as file paths + (``cert_path``/``key_path``). PEM strings are preferred for consumers + that should not persist private keys to disk; the library never writes + them anywhere the caller can observe. When both are provided, the PEM + strings win. + """ + serial: str - cert_path: str - key_path: str - cert_dir: str + cert_path: str | None = None + key_path: str | None = None + cert_dir: str | None = None ip_address: str | None = None + + cert_pem: str | None = None + key_pem: str | None = None + + def __post_init__(self) -> None: + if (self.cert_pem is None) != (self.key_pem is None): + raise ValueError("cert_pem and key_pem must be provided together") + if (self.cert_path is None) != (self.key_path is None): + raise ValueError("cert_path and key_path must be provided together") + if self.cert_pem is None and self.cert_path is None: + raise ValueError("Certificate material is required: provide cert_pem/key_pem or cert_path/key_path") diff --git a/harbor/utils.py b/harbor/utils.py index 7fe04e9..59d324f 100644 --- a/harbor/utils.py +++ b/harbor/utils.py @@ -1,5 +1,8 @@ +import hashlib import logging +import os import ssl +import tempfile from .config import HarborCameraConfig @@ -12,12 +15,51 @@ def get_camera_host(camera_config: HarborCameraConfig) -> str: return f"harborc-{camera_config.serial}.local" +def get_ssl_cache_key(camera_config: HarborCameraConfig) -> str: + """Return the cache key for the SSL context built from this config. + + Keyed off the certificate material itself so a config carrying new + credentials never reuses a stale context. + """ + if camera_config.cert_pem is not None and camera_config.key_pem is not None: + digest = hashlib.sha256() + digest.update(camera_config.cert_pem.encode()) + digest.update(b"\x00") + digest.update(camera_config.key_pem.encode()) + return f"pem:{digest.hexdigest()}" + return f"path:{camera_config.cert_path}:{camera_config.key_path}" + + +def _write_private_file(path: str, data: str) -> None: + fd = os.open(path, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o600) + with os.fdopen(fd, "w") as handle: + handle.write(data) + + def build_ssl_context(camera_config: HarborCameraConfig) -> ssl.SSLContext: - _LOGGER.info( - "Harbor: Building SSL context with cert_path=%s, key_path=%s", camera_config.cert_path, camera_config.key_path - ) ctx = ssl.create_default_context(ssl.Purpose.SERVER_AUTH) - ctx.load_cert_chain(certfile=camera_config.cert_path, keyfile=camera_config.key_path) + + if camera_config.cert_pem is not None and camera_config.key_pem is not None: + _LOGGER.info("Harbor: Building SSL context from in-memory PEM data for camera %s", camera_config.serial) + # ssl.SSLContext.load_cert_chain only accepts file paths, so stage the + # PEM data in a short-lived private temp dir that is removed before + # this function returns. + with tempfile.TemporaryDirectory(prefix="harbor-tls-") as tmp_dir: + cert_file = os.path.join(tmp_dir, "cert.pem") + key_file = os.path.join(tmp_dir, "key.pem") + _write_private_file(cert_file, camera_config.cert_pem) + _write_private_file(key_file, camera_config.key_pem) + ctx.load_cert_chain(certfile=cert_file, keyfile=key_file) + else: + _LOGGER.info( + "Harbor: Building SSL context with cert_path=%s, key_path=%s", + camera_config.cert_path, + camera_config.key_path, + ) + if camera_config.cert_path is None or camera_config.key_path is None: + raise ValueError("HarborCameraConfig has no certificate material") + ctx.load_cert_chain(certfile=camera_config.cert_path, keyfile=camera_config.key_path) + ctx.check_hostname = False ctx.verify_mode = ssl.CERT_NONE return ctx @@ -28,7 +70,7 @@ def get_ssl_context(camera_config: HarborCameraConfig, cache: dict | None = None _LOGGER.info("Harbor: Creating new SSL context (no cache)") return build_ssl_context(camera_config) - key = camera_config.serial + key = get_ssl_cache_key(camera_config) if key in cache: _LOGGER.debug("Harbor: Returning cached SSL context for camera %s", camera_config.serial) return cache[key] diff --git a/tests/data/certs.py b/tests/data/certs.py new file mode 100644 index 0000000..ac784d0 --- /dev/null +++ b/tests/data/certs.py @@ -0,0 +1,24 @@ +"""Throwaway self-signed certificate for tests. + +Generated solely for this test suite (CN=harbor-python-test); it is not a +real secret and grants access to nothing. +""" + +TEST_CERT_PEM = """-----BEGIN CERTIFICATE----- +MIIBVjCB/aADAgECAhQ5ykwr6RhEiJjomgP5kdT5fXoQWTAKBggqhkjOPQQDAjAA +MCAXDTI2MDcwNjExMDgyOVoYDzIxMjYwNjEyMTEwODI5WjAAMFkwEwYHKoZIzj0C +AQYIKoZIzj0DAQcDQgAEQcUSIsA+ThfY80PpAtm762isem0PJrO8wTVkLdz8gEbc +7R84RM0ZxEW9etazgEmckGl92WD3AReeC2KXfkve7aNTMFEwHQYDVR0OBBYEFMpH +q9UJ3gFWMc7SC2hJDyAZ0/pPMB8GA1UdIwQYMBaAFMpHq9UJ3gFWMc7SC2hJDyAZ +0/pPMA8GA1UdEwEB/wQFMAMBAf8wCgYIKoZIzj0EAwIDSAAwRQIhAKKyEgeDNa6g +X242fVc63QAuORCx2RpRLR3irw3NsMSdAiB0KKwhbJEnAtZ+6xHNNT8iThy2SQId +pEBbNrI/R4MYEA== +-----END CERTIFICATE----- +""" + +TEST_KEY_PEM = """-----BEGIN PRIVATE KEY----- +MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQg+Ns9YmTAi2r/3Cod +o3AFirQ3S1foIJ8Y3uqd+NNGHdyhRANCAARBxRIiwD5OF9jzQ+kC2bvraKx6bQ8m +s7zBNWQt3PyARtztHzhEzRnERb161rOASZyQaX3ZYPcBF54LYpd+S97t +-----END PRIVATE KEY----- +""" diff --git a/tests/test_config.py b/tests/test_config.py new file mode 100644 index 0000000..acffb52 --- /dev/null +++ b/tests/test_config.py @@ -0,0 +1,84 @@ +from __future__ import annotations + +import ssl + +import pytest + +from harbor.config import HarborCameraConfig +from harbor.utils import build_ssl_context, get_ssl_cache_key, get_ssl_context + +from .data.certs import TEST_CERT_PEM, TEST_KEY_PEM + + +def test_config_accepts_paths() -> None: + """Path-based configs should keep working unchanged.""" + + config = HarborCameraConfig( + serial="TEST123", + cert_path="/path/to/cert.pem", + key_path="/path/to/key.pem", + cert_dir="/path/to/cert_dir", + ) + + assert config.cert_path == "/path/to/cert.pem" + assert config.cert_pem is None + + +def test_config_accepts_pem_strings() -> None: + """PEM material should be accepted without any file paths.""" + + config = HarborCameraConfig(serial="TEST123", cert_pem=TEST_CERT_PEM, key_pem=TEST_KEY_PEM) + + assert config.cert_pem == TEST_CERT_PEM + assert config.cert_path is None + + +def test_config_requires_some_certificate_material() -> None: + with pytest.raises(ValueError): + HarborCameraConfig(serial="TEST123") + + +def test_config_rejects_partial_pem() -> None: + with pytest.raises(ValueError): + HarborCameraConfig(serial="TEST123", cert_pem=TEST_CERT_PEM) + + +def test_config_rejects_partial_paths() -> None: + with pytest.raises(ValueError): + HarborCameraConfig(serial="TEST123", cert_path="/path/to/cert.pem") + + +def test_build_ssl_context_from_pem() -> None: + """The SSL context should be built entirely from in-memory PEM data.""" + + config = HarborCameraConfig(serial="TEST123", cert_pem=TEST_CERT_PEM, key_pem=TEST_KEY_PEM) + + ctx = build_ssl_context(config) + + assert isinstance(ctx, ssl.SSLContext) + assert ctx.verify_mode == ssl.CERT_NONE + + +def test_ssl_context_cache_keyed_off_pem_material() -> None: + """The cache should hit on identical material and miss on different material.""" + + cache: dict = {} + config = HarborCameraConfig(serial="TEST123", cert_pem=TEST_CERT_PEM, key_pem=TEST_KEY_PEM) + + ctx_first = get_ssl_context(config, cache) + ctx_second = get_ssl_context(config, cache) + assert ctx_first is ctx_second + assert len(cache) == 1 + + same_material_other_serial = HarborCameraConfig(serial="OTHER", cert_pem=TEST_CERT_PEM, key_pem=TEST_KEY_PEM) + assert get_ssl_cache_key(config) == get_ssl_cache_key(same_material_other_serial) + + different_material = HarborCameraConfig(serial="TEST123", cert_pem=TEST_CERT_PEM, key_pem=TEST_KEY_PEM + "\n") + assert get_ssl_cache_key(config) != get_ssl_cache_key(different_material) + + +def test_ssl_cache_key_for_paths() -> None: + pem_config = HarborCameraConfig(serial="TEST123", cert_pem=TEST_CERT_PEM, key_pem=TEST_KEY_PEM) + path_config = HarborCameraConfig(serial="TEST123", cert_path="/a/cert.pem", key_path="/a/key.pem") + + assert get_ssl_cache_key(pem_config) != get_ssl_cache_key(path_config) From bb59013a4df7d23d118ffe99f832617413048a86 Mon Sep 17 00:00:00 2001 From: Luke Date: Mon, 6 Jul 2026 07:54:22 -0400 Subject: [PATCH 2/4] feat: debounce connection-state callbacks with a configurable grace period Harbor cameras frequently cycle their TCP connection, which previously caused on_connection_change to fire on every raw connect/disconnect and forced consumers to run their own grace timers. HarborMQTTClient now takes connection_grace_period (default 90 seconds, exported as DEFAULT_CONNECTION_GRACE_PERIOD) and only reports stable transitions: connects are reported immediately, while a disconnect is held back and silently dropped if the client reconnects within the window. Setting the grace period to 0 restores raw per-transition reporting. An explicit stop() skips the grace window and reports the disconnect immediately. The async callback signature is unchanged, and client.connected still reflects the raw transport state. SSL cache invalidation in the reconnect loop now goes through get_ssl_cache_key() to match the material-keyed cache. --- harbor/mqtt.py | 75 ++++++++++++++++++++++++++++++++------ tests/test_mqtt.py | 91 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 154 insertions(+), 12 deletions(-) diff --git a/harbor/mqtt.py b/harbor/mqtt.py index 5ffbc51..5d7ad83 100644 --- a/harbor/mqtt.py +++ b/harbor/mqtt.py @@ -8,10 +8,12 @@ from aiomqtt import Client, MqttError from .config import HarborCameraConfig -from .utils import get_camera_host, get_ssl_context +from .utils import get_camera_host, get_ssl_cache_key, get_ssl_context _LOGGER = logging.getLogger(__name__) +DEFAULT_CONNECTION_GRACE_PERIOD = 90.0 + class HarborMQTTClient: def __init__( @@ -22,16 +24,29 @@ def __init__( client_id: str | None = None, ssl_context_cache: dict | None = None, on_connection_change: Callable[[bool], Awaitable[None]] | None = None, + connection_grace_period: float = DEFAULT_CONNECTION_GRACE_PERIOD, ) -> None: + """Initialize the MQTT client. + + ``on_connection_change`` fires only on stable connection-state + transitions: a disconnect is reported only if the client stays + disconnected for ``connection_grace_period`` seconds, so routine + TCP flapping never reaches the listener. Set the grace period to 0 + to report every raw transition. ``connected`` always reflects the + raw transport state. + """ self.config = config self.topics = topics self.message_handler = message_handler self.client_id = client_id self.ssl_context_cache = ssl_context_cache or {} self.on_connection_change = on_connection_change + self.connection_grace_period = connection_grace_period self.connected: bool = False self._stop_event = asyncio.Event() self._task: asyncio.Task | None = None + self._reported_connected: bool | None = None + self._disconnect_grace_task: asyncio.Task | None = None async def _handle_message(self, topic: str, payload_raw: str) -> None: try: @@ -42,10 +57,44 @@ async def _handle_message(self, topic: str, payload_raw: str) -> None: await self.message_handler(topic, payload) async def _set_connected(self, connected: bool) -> None: - """Update the connection flag and notify the listener if it changed.""" + """Update the raw connection flag and debounce listener notifications.""" if self.connected == connected: return self.connected = connected + + if connected: + self._cancel_disconnect_grace() + if self._reported_connected is not True: + await self._notify_connection_change(True) + return + + if self._reported_connected is not True: + return + if self.connection_grace_period <= 0: + await self._notify_connection_change(False) + return + if self._disconnect_grace_task is None or self._disconnect_grace_task.done(): + self._disconnect_grace_task = asyncio.create_task(self._disconnect_after_grace()) + + async def _disconnect_after_grace(self) -> None: + """Report a disconnect only if it survives the grace period.""" + await asyncio.sleep(self.connection_grace_period) + self._disconnect_grace_task = None + if not self.connected: + _LOGGER.info( + "Harbor: camera %s still disconnected after %s second grace period", + self.config.serial, + self.connection_grace_period, + ) + await self._notify_connection_change(False) + + def _cancel_disconnect_grace(self) -> None: + if self._disconnect_grace_task is not None: + self._disconnect_grace_task.cancel() + self._disconnect_grace_task = None + + async def _notify_connection_change(self, connected: bool) -> None: + self._reported_connected = connected if self.on_connection_change is None: return try: @@ -56,6 +105,9 @@ async def _set_connected(self, connected: bool) -> None: self.config.serial, ) + def _invalidate_ssl_cache(self) -> None: + self.ssl_context_cache.pop(get_ssl_cache_key(self.config), None) + async def run(self) -> None: try: loop = asyncio.get_running_loop() @@ -63,8 +115,7 @@ async def run(self) -> None: except Exception as e: _LOGGER.error("Harbor: Failed to create SSL context for camera %s: %s", self.config.serial, e) # Ensure we clear any partial state - if self.config.serial in self.ssl_context_cache: - del self.ssl_context_cache[self.config.serial] + self._invalidate_ssl_cache() return reconnect_delay = 2 @@ -128,28 +179,24 @@ async def run(self) -> None: except TimeoutError as e: _LOGGER.warning("Harbor: MQTT connection timeout for %s: %s (reconnecting)", self.config.serial, e) # Clear SSL context on timeout as it might be a stale session - if self.config.serial in self.ssl_context_cache: - del self.ssl_context_cache[self.config.serial] + self._invalidate_ssl_cache() except MqttError as e: _LOGGER.warning("Harbor: MQTT error for %s: %s (reconnecting)", self.config.serial, e) _LOGGER.info("Harbor: MQTT disconnected from camera %s", self.config.serial) # Clear SSL context on MQTT error - if self.config.serial in self.ssl_context_cache: - del self.ssl_context_cache[self.config.serial] + self._invalidate_ssl_cache() except OSError as e: _LOGGER.warning("Harbor: MQTT OS error for %s: %s (reconnecting)", self.config.serial, e) # Critical to clear context here for WinError 10065 cleanup - if self.config.serial in self.ssl_context_cache: - del self.ssl_context_cache[self.config.serial] + self._invalidate_ssl_cache() except asyncio.CancelledError: raise except Exception as e: _LOGGER.error("Harbor: MQTT unexpected error for %s: %s (reconnecting)", self.config.serial, e) # Clear context on unexpected errors too - if self.config.serial in self.ssl_context_cache: - del self.ssl_context_cache[self.config.serial] + self._invalidate_ssl_cache() import traceback _LOGGER.error(traceback.format_exc()) @@ -195,6 +242,10 @@ async def stop(self) -> None: except asyncio.CancelledError: pass _LOGGER.info("Harbor: MQTT client stopped for camera %s", self.config.serial) + # An intentional stop is a stable disconnect: skip the grace period. + self._cancel_disconnect_grace() + if self._reported_connected: + await self._notify_connection_change(False) def __del__(self) -> None: if self._stop_event and not self._stop_event.is_set(): diff --git a/tests/test_mqtt.py b/tests/test_mqtt.py index 254434b..de98979 100644 --- a/tests/test_mqtt.py +++ b/tests/test_mqtt.py @@ -1,5 +1,7 @@ from __future__ import annotations +import asyncio + from harbor.config import HarborCameraConfig from harbor.mqtt import HarborMQTTClient @@ -46,3 +48,92 @@ async def message_handler(topic: str, payload: object) -> None: await client._handle_message("test/topic", '{"test": "data"}') assert messages == [("test/topic", {"test": "data"})] + + +async def _noop_handler(topic: str, payload: object) -> None: + pass + + +def _create_debounce_client(changes: list[bool], grace: float) -> HarborMQTTClient: + """Create a client that records connection-change callbacks.""" + + async def on_change(connected: bool) -> None: + changes.append(connected) + + return HarborMQTTClient( + config=_create_config(), + topics=[], + message_handler=_noop_handler, + on_connection_change=on_change, + connection_grace_period=grace, + ) + + +async def test_connection_change_fires_on_first_connect() -> None: + changes: list[bool] = [] + client = _create_debounce_client(changes, grace=0.1) + + await client._set_connected(True) + + assert changes == [True] + assert client.connected is True + + +async def test_connection_change_suppresses_flapping() -> None: + """A disconnect followed by a reconnect within the grace window is silent.""" + + changes: list[bool] = [] + client = _create_debounce_client(changes, grace=0.1) + + await client._set_connected(True) + await client._set_connected(False) + await asyncio.sleep(0.02) + await client._set_connected(True) + await asyncio.sleep(0.2) + + assert changes == [True] + + +async def test_connection_change_reports_stable_disconnect() -> None: + changes: list[bool] = [] + client = _create_debounce_client(changes, grace=0.05) + + await client._set_connected(True) + await client._set_connected(False) + await asyncio.sleep(0.15) + + assert changes == [True, False] + assert client.connected is False + + +async def test_connection_change_zero_grace_reports_immediately() -> None: + changes: list[bool] = [] + client = _create_debounce_client(changes, grace=0) + + await client._set_connected(True) + await client._set_connected(False) + + assert changes == [True, False] + + +async def test_disconnect_before_first_connect_is_not_reported() -> None: + changes: list[bool] = [] + client = _create_debounce_client(changes, grace=0) + + client.connected = True # raw flag only; never reported as connected + await client._set_connected(False) + + assert changes == [] + + +async def test_stop_flushes_pending_disconnect() -> None: + """An intentional stop should report the disconnect without waiting.""" + + changes: list[bool] = [] + client = _create_debounce_client(changes, grace=60) + + await client._set_connected(True) + await client._set_connected(False) + await client.stop() + + assert changes == [True, False] From ebda38505b8e2c854335e0db95f990fb5179306f Mon Sep 17 00:00:00 2001 From: Luke Date: Mon, 6 Jul 2026 07:54:37 -0400 Subject: [PATCH 3/4] feat: normalize speaker_state and stream_quality to lowercase values The device reports enum-ish fields in mixed/upper case (e.g. PLAYING, GOOD). These are now normalized to stable lowercase strings before landing in HarborDeviceState.values, so consumers no longer need to lowercase them for enum options. The documented value sets are exported from the package root: SPEAKER_STATES (idle, muted, off, paused, playing, unknown) and STREAM_QUALITIES (excellent, fair, good, poor, unknown). Values outside these sets are still stored lowercased and logged as a warning once per value, so the option lists can be extended when new values appear. --- harbor/__init__.py | 4 ++- harbor/devices/camera.py | 43 ++++++++++++++++++++++++++-- tests/test_camera_state.py | 57 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 101 insertions(+), 3 deletions(-) create mode 100644 tests/test_camera_state.py diff --git a/harbor/__init__.py b/harbor/__init__.py index f716614..7be13e1 100644 --- a/harbor/__init__.py +++ b/harbor/__init__.py @@ -9,7 +9,7 @@ ViewerLeftEvent, ) from .device import HarborDevice -from .devices.camera import HarborCamera +from .devices.camera import SPEAKER_STATES, STREAM_QUALITIES, HarborCamera from .devices.monitor import HarborMonitor from .events import ( CameraEventUpdate, @@ -59,4 +59,6 @@ "HarborViewer", "HarborEventState", "HarborDeviceState", + "SPEAKER_STATES", + "STREAM_QUALITIES", ] diff --git a/harbor/devices/camera.py b/harbor/devices/camera.py index 320369f..821d71f 100644 --- a/harbor/devices/camera.py +++ b/harbor/devices/camera.py @@ -20,6 +20,15 @@ DEFAULT_CAMERA_EVENT_KEYS = ("motion_detection", "cry_detection", "noise_detection") DEFAULT_EVENT_ACTIVE_SECONDS = 5.0 +# Known values for enumerated state fields. The device reports these in +# mixed/upper case (e.g. "PLAYING", "GOOD"); they are normalized to the +# lowercase values below before being stored in ``HarborDeviceState.values``. +# A value outside these sets is still stored lowercased (and logged once), +# so consumers that declare options up front should treat these sets as the +# baseline, not a hard guarantee. +SPEAKER_STATES = frozenset({"idle", "muted", "off", "paused", "playing", "unknown"}) +STREAM_QUALITIES = frozenset({"excellent", "fair", "good", "poor", "unknown"}) + class HarborCamera(HarborDevice): """Represents a Harbor camera device.""" @@ -29,6 +38,7 @@ def __init__(self, config: HarborCameraConfig) -> None: super().__init__(config.serial, "camera") self.config = config self._event_reset_handles: dict[str, asyncio.TimerHandle] = {} + self._unexpected_enum_values: set[tuple[str, str]] = set() for event_key in DEFAULT_CAMERA_EVENT_KEYS: self._ensure_camera_event(event_key) @@ -60,8 +70,14 @@ def _apply_local_livekit_heartbeat( self._set_state_value("bitrate", payload.bitrate) self._set_state_value("wifi_strength", payload.network_bars) self._set_state_value("camera_present", payload.camera_present) - self._set_state_value("speaker_state", payload.speaker_state) - self._set_state_value("stream_quality", payload.stream_quality) + self._set_state_value( + "speaker_state", + self._normalize_enum_value("speaker_state", payload.speaker_state, SPEAKER_STATES), + ) + self._set_state_value( + "stream_quality", + self._normalize_enum_value("stream_quality", payload.stream_quality, STREAM_QUALITIES), + ) self._set_state_value("app_start_time", payload.app_start_time) self._set_state_value("stream_start_time", payload.stream_start_time) @@ -77,6 +93,29 @@ def _apply_local_livekit_heartbeat( } self.state.values["num_viewers"] = len(self.state.viewers) + def _normalize_enum_value( + self, + field_name: str, + value: str | None, + known_values: frozenset[str], + ) -> str | None: + """Normalize an enumerated device value to a stable lowercase string.""" + if value is None: + return None + normalized = value.strip().lower() + if not normalized: + return None + if normalized not in known_values and (field_name, normalized) not in self._unexpected_enum_values: + self._unexpected_enum_values.add((field_name, normalized)) + _LOGGER.warning( + "Camera %s reported unexpected %s value %r (known values: %s)", + self.serial, + field_name, + normalized, + sorted(known_values), + ) + return normalized + def _apply_viewer_joined(self, viewer: ViewerInfo | None) -> None: """Apply a viewer joined update.""" if viewer is None: diff --git a/tests/test_camera_state.py b/tests/test_camera_state.py new file mode 100644 index 0000000..0886453 --- /dev/null +++ b/tests/test_camera_state.py @@ -0,0 +1,57 @@ +from __future__ import annotations + +from harbor.config import HarborCameraConfig +from harbor.devices.camera import SPEAKER_STATES, STREAM_QUALITIES, HarborCamera + + +def _create_camera() -> HarborCamera: + config = HarborCameraConfig( + serial="TEST123", + cert_path="/path/to/cert.pem", + key_path="/path/to/key.pem", + ) + return HarborCamera(config) + + +async def test_enum_state_values_are_normalized_to_lowercase() -> None: + """Device-reported enum values arrive upper case and must be lowercased.""" + + camera = _create_camera() + + await camera.handle_message( + "cameras/TEST123/events/local-livekit-heartbeat", + {"speaker_state": "PLAYING", "stream_quality": "GOOD"}, + ) + + assert camera.state.values["speaker_state"] == "playing" + assert camera.state.values["stream_quality"] == "good" + assert camera.state.values["speaker_state"] in SPEAKER_STATES + assert camera.state.values["stream_quality"] in STREAM_QUALITIES + + +async def test_unexpected_enum_value_is_passed_through_lowercased() -> None: + camera = _create_camera() + + await camera.handle_message( + "cameras/TEST123/events/local-livekit-heartbeat", + {"speaker_state": "Buffering", "stream_quality": "EXCELLENT"}, + ) + + assert camera.state.values["speaker_state"] == "buffering" + assert camera.state.values["stream_quality"] == "excellent" + + +async def test_missing_enum_values_do_not_clear_state() -> None: + camera = _create_camera() + + await camera.handle_message( + "cameras/TEST123/events/local-livekit-heartbeat", + {"speaker_state": "IDLE", "stream_quality": "POOR"}, + ) + await camera.handle_message( + "cameras/TEST123/events/local-livekit-heartbeat", + {"bitrate": 1000.0}, + ) + + assert camera.state.values["speaker_state"] == "idle" + assert camera.state.values["stream_quality"] == "poor" From a992186731a69cf44127cefc46f44dbef25e0d55 Mon Sep 17 00:00:00 2001 From: Luke Date: Mon, 6 Jul 2026 08:12:19 -0400 Subject: [PATCH 4/4] fix: address Copilot review feedback - Rebuild the SSL context inside the reconnect loop so cache invalidations in the error handlers actually take effect on the next attempt instead of reusing the initial context forever; unchanged material remains a cheap cache hit. - Write staged PEM temp files with explicit UTF-8 encoding and LF newlines to avoid platform-dependent newline translation on Windows. - Correct the HarborCameraConfig docstring to accurately describe the short-lived temp-file staging instead of claiming PEM data is never written to disk. --- harbor/config.py | 8 +++++--- harbor/mqtt.py | 21 ++++++++++++--------- harbor/utils.py | 3 ++- 3 files changed, 19 insertions(+), 13 deletions(-) diff --git a/harbor/config.py b/harbor/config.py index 6eb50b1..e876d08 100644 --- a/harbor/config.py +++ b/harbor/config.py @@ -8,9 +8,11 @@ class HarborCameraConfig: Client certificate material can be provided either as in-memory PEM strings (``cert_pem``/``key_pem``) or as file paths (``cert_path``/``key_path``). PEM strings are preferred for consumers - that should not persist private keys to disk; the library never writes - them anywhere the caller can observe. When both are provided, the PEM - strings win. + that should not manage certificate files themselves: while building the + SSL context the library briefly stages the PEM data in a private + temporary directory (0600 files) that is deleted before the build + returns, so the caller never handles files. When both are provided, the + PEM strings win. """ serial: str diff --git a/harbor/mqtt.py b/harbor/mqtt.py index 5d7ad83..c9f3e01 100644 --- a/harbor/mqtt.py +++ b/harbor/mqtt.py @@ -109,21 +109,24 @@ def _invalidate_ssl_cache(self) -> None: self.ssl_context_cache.pop(get_ssl_cache_key(self.config), None) async def run(self) -> None: - try: - loop = asyncio.get_running_loop() - ssl_ctx = await loop.run_in_executor(None, get_ssl_context, self.config, self.ssl_context_cache) - except Exception as e: - _LOGGER.error("Harbor: Failed to create SSL context for camera %s: %s", self.config.serial, e) - # Ensure we clear any partial state - self._invalidate_ssl_cache() - return - reconnect_delay = 2 _LOGGER.info("Harbor: MQTT client starting for camera %s", self.config.serial) try: while not self._stop_event.is_set(): + # Fetch the SSL context each attempt so invalidations in the + # error handlers below take effect on the next reconnect; + # unchanged material is a cheap cache hit. + try: + loop = asyncio.get_running_loop() + ssl_ctx = await loop.run_in_executor(None, get_ssl_context, self.config, self.ssl_context_cache) + except Exception as e: + _LOGGER.error("Harbor: Failed to create SSL context for camera %s: %s", self.config.serial, e) + # Ensure we clear any partial state + self._invalidate_ssl_cache() + return + try: host = get_camera_host(self.config) _LOGGER.info( diff --git a/harbor/utils.py b/harbor/utils.py index 59d324f..e8e1456 100644 --- a/harbor/utils.py +++ b/harbor/utils.py @@ -32,7 +32,8 @@ def get_ssl_cache_key(camera_config: HarborCameraConfig) -> str: def _write_private_file(path: str, data: str) -> None: fd = os.open(path, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o600) - with os.fdopen(fd, "w") as handle: + # PEM must stay byte-exact: no platform newline translation or locale encoding. + with os.fdopen(fd, "w", encoding="utf-8", newline="\n") as handle: handle.write(data)