diff --git a/app/fanout/mqtt_ha.py b/app/fanout/mqtt_ha.py index dcbc359..6a6469d 100644 --- a/app/fanout/mqtt_ha.py +++ b/app/fanout/mqtt_ha.py @@ -115,6 +115,22 @@ def _lpp_sensor_key(type_name: str, channel: int) -> str: return f"lpp_{type_name}_ch{channel}" +def _assign_lpp_keys(lpp_sensors: list[dict]) -> list[tuple[dict, str, int]]: + """Pair each LPP sensor dict with a disambiguated flat key and occurrence. + + First occurrence keeps the base key (``lpp_temperature_ch1``), occurrence=1; + subsequent duplicates of the same (type_name, channel) get ``_2``, ``_3``, etc. + """ + counts: dict[str, int] = {} + result: list[tuple[dict, str, int]] = [] + for sensor in lpp_sensors: + base = _lpp_sensor_key(sensor.get("type_name", "unknown"), sensor.get("channel", 0)) + n = counts.get(base, 0) + 1 + counts[base] = n + result.append((sensor, base if n == 1 else f"{base}_{n}", n)) + return result + + def _repeater_telemetry_payload(data: dict[str, Any]) -> dict[str, Any]: """Build the flat HA state payload for a repeater telemetry snapshot.""" payload: dict[str, Any] = {} @@ -123,8 +139,7 @@ def _repeater_telemetry_payload(data: dict[str, Any]) -> dict[str, Any]: if field is not None: payload[field] = data.get(field) - for sensor in data.get("lpp_sensors", []) or []: - key = _lpp_sensor_key(sensor.get("type_name", "unknown"), sensor.get("channel", 0)) + for sensor, key, _ in _assign_lpp_keys(data.get("lpp_sensors", []) or []): payload[key] = sensor.get("value") return payload @@ -139,16 +154,19 @@ def _lpp_discovery_configs( ) -> list[tuple[str, dict]]: """Build HA discovery configs for a repeater's LPP sensors.""" configs: list[tuple[str, dict]] = [] - for sensor in lpp_sensors: + for sensor, field, occurrence in _assign_lpp_keys(lpp_sensors): type_name = sensor.get("type_name", "unknown") channel = sensor.get("channel", 0) - field = _lpp_sensor_key(type_name, channel) meta = _LPP_HA_META.get(type_name, {}) nid = _node_id(pub_key) object_id = field display = type_name.replace("_", " ").title() - name = f"{display} (Ch {channel})" + name = ( + f"{display} (Ch {channel})" + if occurrence == 1 + else f"{display} (Ch {channel}) #{occurrence}" + ) cfg: dict[str, Any] = { "name": name, @@ -731,9 +749,7 @@ class MqttHaModule(FanoutModule): payload = _repeater_telemetry_payload(data) lpp_sensors: list[dict] = data.get("lpp_sensors", []) rediscover = False - for sensor in lpp_sensors: - # Check if discovery for this sensor has been published yet - key = _lpp_sensor_key(sensor.get("type_name", "unknown"), sensor.get("channel", 0)) + for _, key, _ in _assign_lpp_keys(lpp_sensors): expected_topic = f"homeassistant/sensor/meshcore_{nid}/{key}/config" if expected_topic not in self._discovery_topics: rediscover = True diff --git a/frontend/src/components/repeater/RepeaterLppTelemetryPane.tsx b/frontend/src/components/repeater/RepeaterLppTelemetryPane.tsx index d78a460..3d61aa5 100644 --- a/frontend/src/components/repeater/RepeaterLppTelemetryPane.tsx +++ b/frontend/src/components/repeater/RepeaterLppTelemetryPane.tsx @@ -1,4 +1,5 @@ -import { RepeaterPane, NotFetched, LppSensorRow } from './repeaterPaneShared'; +import { useMemo } from 'react'; +import { RepeaterPane, NotFetched, LppSensorRow, formatLppLabel } from './repeaterPaneShared'; import { useDistanceUnit } from '../../contexts/DistanceUnitContext'; import type { RepeaterLppTelemetryResponse, PaneState } from '../../types'; @@ -14,6 +15,19 @@ export function LppTelemetryPane({ disabled?: boolean; }) { const { distanceUnit } = useDistanceUnit(); + + // Build disambiguated labels matching the telemetry history chart names + const labels = useMemo(() => { + if (!data) return []; + const counts = new Map(); + return data.sensors.map((s) => { + const base = `${s.type_name}_${s.channel}`; + const n = (counts.get(base) ?? 0) + 1; + counts.set(base, n); + return formatLppLabel(s.type_name) + ` Ch${s.channel}` + (n > 1 ? ` (${n})` : ''); + }); + }, [data]); + return ( {!data ? ( @@ -23,7 +37,7 @@ export function LppTelemetryPane({ ) : (
{data.sensors.map((sensor, i) => ( - + ))}
)} diff --git a/frontend/src/components/repeater/RepeaterTelemetryHistoryPane.tsx b/frontend/src/components/repeater/RepeaterTelemetryHistoryPane.tsx index 8286018..2a55f85 100644 --- a/frontend/src/components/repeater/RepeaterTelemetryHistoryPane.tsx +++ b/frontend/src/components/repeater/RepeaterTelemetryHistoryPane.tsx @@ -37,9 +37,18 @@ const BUILTIN_METRICS: BuiltinMetric[] = Object.keys(BUILTIN_METRIC_CONFIG) as B // Stable color rotation for dynamic LPP sensors const LPP_COLORS = ['#ec4899', '#14b8a6', '#f97316', '#6366f1', '#84cc16', '#e11d48']; -/** Build a flat data key for an LPP sensor: lpp_{type_name}_ch{channel} */ -function lppKey(s: TelemetryLppSensor): string { - return `lpp_${s.type_name}_ch${s.channel}`; +/** Assign disambiguated flat keys to an array of LPP sensors. + * First occurrence keeps the base key; duplicates of the same (type, channel) get _2, _3, etc. */ +function assignLppKeys( + sensors: TelemetryLppSensor[] +): { sensor: TelemetryLppSensor; key: string; occurrence: number }[] { + const counts = new Map(); + return sensors.map((s) => { + const base = `lpp_${s.type_name}_ch${s.channel}`; + const n = (counts.get(base) ?? 0) + 1; + counts.set(base, n); + return { sensor: s, key: n === 1 ? base : `${base}_${n}`, occurrence: n }; + }); } const TOOLTIP_STYLE = { @@ -93,11 +102,10 @@ export function TelemetryHistoryPane({ // Discover unique LPP sensors across all history entries const lppMetrics = useMemo(() => { - const seen = new Map(); + const seen = new Map(); for (const e of entries) { - for (const s of e.data.lpp_sensors ?? []) { - const k = lppKey(s); - if (!seen.has(k)) seen.set(k, { type_name: s.type_name, channel: s.channel }); + for (const { sensor: s, key: k, occurrence } of assignLppKeys(e.data.lpp_sensors ?? [])) { + if (!seen.has(k)) seen.set(k, { type_name: s.type_name, channel: s.channel, occurrence }); } } const result: { key: string; config: MetricConfig; type_name: string; channel: number }[] = []; @@ -106,7 +114,8 @@ export function TelemetryHistoryPane({ const label = info.type_name.charAt(0).toUpperCase() + info.type_name.slice(1).replace(/_/g, ' ') + - ` Ch${info.channel}`; + ` Ch${info.channel}` + + (info.occurrence > 1 ? ` (${info.occurrence})` : ''); const { unit } = lppDisplayUnit(info.type_name, 0, distanceUnit); result.push({ key: k, @@ -148,9 +157,9 @@ export function TelemetryHistoryPane({ uptime_seconds: d.uptime_seconds, }; // Flatten LPP sensors into the point, converting units as needed - for (const s of d.lpp_sensors ?? []) { + for (const { sensor: s, key } of assignLppKeys(d.lpp_sensors ?? [])) { if (typeof s.value === 'number') { - point[lppKey(s)] = lppDisplayUnit(s.type_name, s.value, distanceUnit).value; + point[key] = lppDisplayUnit(s.type_name, s.value, distanceUnit).value; } } return point; diff --git a/frontend/src/components/repeater/repeaterPaneShared.tsx b/frontend/src/components/repeater/repeaterPaneShared.tsx index a3d3720..5ec04d4 100644 --- a/frontend/src/components/repeater/repeaterPaneShared.tsx +++ b/frontend/src/components/repeater/repeaterPaneShared.tsx @@ -242,8 +242,16 @@ export function formatLppLabel(typeName: string): string { return typeName.charAt(0).toUpperCase() + typeName.slice(1).replace(/_/g, ' '); } -export function LppSensorRow({ sensor, unitPref }: { sensor: LppSensor; unitPref?: string }) { - const label = formatLppLabel(sensor.type_name); +export function LppSensorRow({ + sensor, + unitPref, + label: labelOverride, +}: { + sensor: LppSensor; + unitPref?: string; + label?: string; +}) { + const label = labelOverride ?? formatLppLabel(sensor.type_name); if (typeof sensor.value === 'object' && sensor.value !== null) { // Multi-value sensor (GPS, accelerometer, etc.) diff --git a/tests/test_mqtt_ha.py b/tests/test_mqtt_ha.py index 94fa5d9..e34dfce 100644 --- a/tests/test_mqtt_ha.py +++ b/tests/test_mqtt_ha.py @@ -7,6 +7,7 @@ import pytest from app.fanout.mqtt_ha import ( MqttHaModule, + _assign_lpp_keys, _contact_tracker_discovery_config, _device_payload, _lpp_discovery_configs, @@ -552,6 +553,45 @@ class TestLppSensorKey: assert _lpp_sensor_key("humidity", 0) == "lpp_humidity_ch0" +class TestAssignLppKeys: + def test_no_duplicates(self): + sensors = [ + {"type_name": "temperature", "channel": 1, "value": 20}, + {"type_name": "humidity", "channel": 2, "value": 45}, + ] + result = _assign_lpp_keys(sensors) + assert [(k, n) for _, k, n in result] == [ + ("lpp_temperature_ch1", 1), + ("lpp_humidity_ch2", 1), + ] + + def test_duplicate_type_and_channel(self): + sensors = [ + {"type_name": "temperature", "channel": 1, "value": 20}, + {"type_name": "humidity", "channel": 2, "value": 45}, + {"type_name": "temperature", "channel": 1, "value": 53}, + ] + result = _assign_lpp_keys(sensors) + assert [(k, n) for _, k, n in result] == [ + ("lpp_temperature_ch1", 1), + ("lpp_humidity_ch2", 1), + ("lpp_temperature_ch1_2", 2), + ] + + def test_triple_duplicate(self): + sensors = [ + {"type_name": "voltage", "channel": 0, "value": 3.3}, + {"type_name": "voltage", "channel": 0, "value": 5.0}, + {"type_name": "voltage", "channel": 0, "value": 12.0}, + ] + result = _assign_lpp_keys(sensors) + keys = [k for _, k, _ in result] + assert keys == ["lpp_voltage_ch0", "lpp_voltage_ch0_2", "lpp_voltage_ch0_3"] + + def test_empty_list(self): + assert _assign_lpp_keys([]) == [] + + class TestLppDiscoveryConfigs: def test_produces_config_per_sensor(self): nid = "ccdd11223344" @@ -583,6 +623,27 @@ class TestLppDiscoveryConfigs: assert cfg["suggested_display_precision"] == 1 assert "lpp_temperature_ch1" in cfg["value_template"] + def test_duplicate_type_channel_gets_indexed_keys(self): + nid = "ccdd11223344" + device = _device_payload(nid, "Rep1", "Repeater") + sensors = [ + {"channel": 1, "type_name": "temperature", "value": 20.0}, + {"channel": 2, "type_name": "humidity", "value": 45.0}, + {"channel": 1, "type_name": "temperature", "value": 53.0}, + ] + configs = _lpp_discovery_configs("mc", nid, device, sensors, f"mc/{nid}/telemetry") + + assert len(configs) == 3 + topics = [t for t, _ in configs] + assert f"homeassistant/sensor/meshcore_{nid}/lpp_temperature_ch1/config" in topics + assert f"homeassistant/sensor/meshcore_{nid}/lpp_humidity_ch2/config" in topics + assert f"homeassistant/sensor/meshcore_{nid}/lpp_temperature_ch1_2/config" in topics + + # First temperature keeps base name, second gets #2 suffix + names = {cfg["unique_id"]: cfg["name"] for _, cfg in configs} + assert names[f"meshcore_{nid}_lpp_temperature_ch1"] == "Temperature (Ch 1)" + assert names[f"meshcore_{nid}_lpp_temperature_ch1_2"] == "Temperature (Ch 1) #2" + def test_unknown_sensor_type_no_device_class(self): nid = "ccdd11223344" device = _device_payload(nid, "Rep1", "Repeater") @@ -712,6 +773,35 @@ class TestMqttHaTelemetryWithLpp: mod._publish_discovery.assert_not_awaited() + @pytest.mark.asyncio + async def test_on_telemetry_duplicate_lpp_sensors_not_overwritten(self): + """Two sensors with same (type_name, channel) get distinct keys.""" + key = "ccdd11223344" + nid = _node_id(key) + mod = MqttHaModule("test", _base_config(tracked_repeaters=[key])) + mod._publisher = MagicMock() + mod._publisher.connected = True + mod._publisher.publish = AsyncMock() + mod._discovery_topics = [ + f"homeassistant/sensor/meshcore_{nid}/lpp_temperature_ch1/config", + f"homeassistant/sensor/meshcore_{nid}/lpp_temperature_ch1_2/config", + ] + + await mod.on_telemetry( + { + "public_key": key, + "battery_volts": 4.1, + "lpp_sensors": [ + {"channel": 1, "type_name": "temperature", "value": 20.0}, + {"channel": 1, "type_name": "temperature", "value": 53.0}, + ], + } + ) + + payload = mod._publisher.publish.call_args[0][1] + assert payload["lpp_temperature_ch1"] == 20.0 + assert payload["lpp_temperature_ch1_2"] == 53.0 + @pytest.mark.asyncio async def test_on_telemetry_without_lpp_sensors(self): """Existing behavior: no lpp_sensors key means no LPP fields in payload."""