Skip to content

Commit 45baf41

Browse files
committed
feat: prefer unified warning about equipment with repeated names, with context about source of equipment
Check for duplicate names in the main refresh loop. Describe that they are due to output from the OmniLogic unit itself. This will help clarify the source in downstream usage, like in Home Assistant (HA). I personally saw messages in HA like the following and thought I had incorrectly set up the HA OmniLogic integration: ``` 2026-04-24 20:56:23.095 WARNING (MainThread) [pyomnilogic_local.collections] Equipment collection contains 4 items with the same name 'SolarSensor'. Name-based lookups will return the first match. Consider using system_id-based lookups for reliability or renaming equipment to avoid duplicates. 2026-04-24 20:56:24.636 WARNING (MainThread) [pyomnilogic_local.collections] Equipment collection contains 2 items with the same name 'Gas'. Name-based lookups will return the first match. Consider using system_id-based lookups for reliability or renaming equipment to avoid duplicates. 2026-04-24 20:56:24.637 WARNING (MainThread) [pyomnilogic_local.collections] Equipment collection contains 2 items with the same name 'Solar'. Name-based lookups will return the first match. Consider using system_id-based lookups for reliability or renaming equipment to avoid duplicates. 2026-04-24 20:56:24.638 WARNING (MainThread) [pyomnilogic_local.collections] Equipment collection contains 2 items with the same name 'Filter Pump'. Name-based lookups will return the first match. Consider using system_id-based lookups for reliability or renaming equipment to avoid duplicates. 2026-04-24 20:56:24.639 WARNING (MainThread) [pyomnilogic_local.collections] Equipment collection contains 2 items with the same name 'Chlorinator'. Name-based lookups will return the first match. Consider using system_id-based lookups for reliability or renaming equipment to avoid duplicates. 2026-04-24 20:56:24.641 WARNING (MainThread) [pyomnilogic_local.collections] Equipment collection contains 2 items with the same name 'WaterSensor'. Name-based lookups will return the first match. Consider using system_id-based lookups for reliability or renaming equipment to avoid duplicates. 2026-04-24 20:56:24.641 WARNING (MainThread) [pyomnilogic_local.collections] Equipment collection contains 2 items with the same name 'FlowSensor'. Name-based lookups will return the first match. Consider using system_id-based lookups for reliability or renaming equipment to avoid duplicates. ```
1 parent 46b19ce commit 45baf41

3 files changed

Lines changed: 119 additions & 62 deletions

File tree

pyomnilogic_local/collections.py

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
from __future__ import annotations
44

55
import logging
6-
from collections import Counter
76
from enum import Enum
87
from typing import TYPE_CHECKING, Any, overload
98

@@ -15,9 +14,6 @@
1514

1615
_LOGGER = logging.getLogger(__name__)
1716

18-
# Track which duplicate names we've already warned about to avoid log spam
19-
_WARNED_DUPLICATE_NAMES: set[str] = set()
20-
2117

2218
class EquipmentDict[OE: OmniEquipment[Any, Any]]:
2319
"""A dictionary-like collection that supports lookup by both name and system_id.
@@ -72,10 +68,6 @@ def __init__(self, items: list[OE] | None = None) -> None:
7268
def _validate(self) -> None:
7369
"""Validate the equipment collection.
7470
75-
Checks for:
76-
1. Items without both system_id and name (raises ValueError)
77-
2. Duplicate names (logs warning once per unique duplicate)
78-
7971
Raises:
8072
ValueError: If any item has neither a system_id nor a name.
8173
"""
@@ -88,23 +80,6 @@ def _validate(self) -> None:
8880
)
8981
raise ValueError(msg)
9082

91-
# Find duplicate names that we haven't warned about yet
92-
name_counts = Counter(item.name for item in self._items if item.name is not None)
93-
duplicate_names = {name for name, count in name_counts.items() if count > 1}
94-
unwarned_duplicates = duplicate_names.difference(_WARNED_DUPLICATE_NAMES)
95-
96-
# Log warnings for new duplicates
97-
for name in unwarned_duplicates:
98-
_LOGGER.warning(
99-
"Equipment collection contains %d items with the same name '%s'. "
100-
"Name-based lookups will return the first match. "
101-
"Consider using system_id-based lookups for reliability "
102-
"or renaming equipment to avoid duplicates.",
103-
name_counts[name],
104-
name,
105-
)
106-
_WARNED_DUPLICATE_NAMES.add(name)
107-
10883
@property
10984
def _by_name(self) -> dict[str, OE]:
11085
"""Dynamically build name-to-equipment mapping."""

pyomnilogic_local/omnilogic.py

Lines changed: 57 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
from pyomnilogic_local.system import System
1717

1818
if TYPE_CHECKING:
19+
from collections.abc import Sequence
20+
1921
from pyomnilogic_local._base import OmniEquipment
2022
from pyomnilogic_local.bow import Bow
2123
from pyomnilogic_local.chlorinator import Chlorinator
@@ -35,6 +37,32 @@
3537
_LOGGER = logging.getLogger(__name__)
3638

3739

40+
def _check_duplicate_item_names(items: Sequence[OmniEquipment[Any, Any]], source_id: str) -> str | None:
41+
"""Returns a warning message if there are items with a duplicate name."""
42+
items_by_name: dict[str, list[OmniEquipment[Any, Any]]] = {}
43+
for item in items:
44+
if item.name is not None:
45+
items_by_name.setdefault(item.name, []).append(item)
46+
47+
duplicate_names = {name for name, name_items in items_by_name.items() if len(name_items) > 1}
48+
if not duplicate_names:
49+
return None
50+
51+
item = items[0] # Guaranteed to exist if we have duplicates
52+
msg_lines = [f"OmniLogic {source_id} contains multiple items with the same name:"]
53+
for name in sorted(duplicate_names):
54+
name_items = items_by_name[name]
55+
ids = [str(i.system_id) for i in name_items if i.system_id is not None]
56+
msg_lines.append(f" - {name}: IDs {', '.join(ids)}")
57+
58+
msg_lines.append(
59+
"Name-based lookups will return the first match. "
60+
"Consider using system_id-based lookups for reliability "
61+
"or renaming equipment on the OmniLogic controller to avoid duplicates."
62+
)
63+
return "\n".join(msg_lines)
64+
65+
3866
class OmniLogic:
3967
mspconfig: MSPConfig
4068
telemetry: Telemetry
@@ -65,6 +93,7 @@ def __init__(self, host: str, port: int = 10444, timeout: float = DEFAULT_RESPON
6593
else:
6694
self._api = OmniLogicAPI(host, port, timeout)
6795
self._refresh_lock = asyncio.Lock()
96+
self._seen_item_names: set[str] = set()
6897

6998
def __repr__(self) -> str:
7099
"""Return a string representation of the OmniLogic instance for debugging.
@@ -182,6 +211,12 @@ def _update_equipment(self) -> None:
182211
else:
183212
self.schedules = EquipmentDict([Schedule(self, schedule_, self.telemetry) for schedule_ in self.mspconfig.schedules])
184213

214+
current_names = {item.name for item in self.all_equipment if item.name is not None}
215+
if not current_names.issubset(self._seen_item_names):
216+
if warning := _check_duplicate_item_names(self.all_equipment, f"{self.host}:{self.port}"):
217+
_LOGGER.warning(warning)
218+
self._seen_item_names.update(current_names)
219+
185220
# Equipment discovery properties
186221
@property
187222
def all_lights(self) -> EquipmentDict[ColorLogicLight]:
@@ -276,6 +311,26 @@ def all_csads(self) -> EquipmentDict[CSAD]:
276311
csads.extend(bow.csads.values())
277312
return EquipmentDict(csads)
278313

314+
@property
315+
def all_equipment(self) -> list[OmniEquipment[Any, Any]]:
316+
"""Returns a flat list of all equipment instances in the system."""
317+
equipment: list[OmniEquipment[Any, Any]] = [self.backyard]
318+
equipment.extend(self.all_lights.values())
319+
equipment.extend(self.all_relays.values())
320+
equipment.extend(self.all_pumps.values())
321+
equipment.extend(self.all_filters.values())
322+
equipment.extend(self.all_sensors.values())
323+
equipment.extend(self.all_heaters.values())
324+
equipment.extend(self.all_heater_equipment.values())
325+
equipment.extend(self.all_chlorinators.values())
326+
equipment.extend(self.all_chlorinator_equipment.values())
327+
equipment.extend(self.all_csads.values())
328+
equipment.extend(self.all_csad_equipment.values())
329+
equipment.extend(self.all_bows.values())
330+
equipment.extend(self.groups.values())
331+
equipment.extend(self.schedules.values())
332+
return equipment
333+
279334
@property
280335
def all_bows(self) -> EquipmentDict[Bow]:
281336
"""Returns all Bow instances across all bows in the backyard."""
@@ -292,24 +347,7 @@ def get_equipment_by_name(self, name: str) -> OmniEquipment[Any, Any] | None:
292347
Returns:
293348
The first equipment with matching name, or None if not found
294349
"""
295-
# Search all equipment types
296-
all_equipment: list[OmniEquipment[Any, Any]] = []
297-
all_equipment.extend([self.backyard])
298-
all_equipment.extend(self.all_lights.values())
299-
all_equipment.extend(self.all_relays.values())
300-
all_equipment.extend(self.all_pumps.values())
301-
all_equipment.extend(self.all_filters.values())
302-
all_equipment.extend(self.all_sensors.values())
303-
all_equipment.extend(self.all_heaters.values())
304-
all_equipment.extend(self.all_heater_equipment.values())
305-
all_equipment.extend(self.all_chlorinators.values())
306-
all_equipment.extend(self.all_chlorinator_equipment.values())
307-
all_equipment.extend(self.all_csads.values())
308-
all_equipment.extend(self.all_csad_equipment.values())
309-
all_equipment.extend(self.all_bows.values())
310-
all_equipment.extend(self.groups.values())
311-
312-
for equipment in all_equipment:
350+
for equipment in self.all_equipment:
313351
if equipment.name == name:
314352
return equipment
315353

@@ -324,25 +362,7 @@ def get_equipment_by_id(self, system_id: int) -> OmniEquipment[Any, Any] | None:
324362
Returns:
325363
The first equipment with matching system_id, or None if not found
326364
"""
327-
# Search all equipment types
328-
all_equipment: list[OmniEquipment[Any, Any]] = []
329-
all_equipment.extend([self.backyard])
330-
all_equipment.extend(self.all_lights.values())
331-
all_equipment.extend(self.all_relays.values())
332-
all_equipment.extend(self.all_pumps.values())
333-
all_equipment.extend(self.all_filters.values())
334-
all_equipment.extend(self.all_sensors.values())
335-
all_equipment.extend(self.all_heaters.values())
336-
all_equipment.extend(self.all_heater_equipment.values())
337-
all_equipment.extend(self.all_chlorinators.values())
338-
all_equipment.extend(self.all_chlorinator_equipment.values())
339-
all_equipment.extend(self.all_csads.values())
340-
all_equipment.extend(self.all_csad_equipment.values())
341-
all_equipment.extend(self.all_bows.values())
342-
all_equipment.extend(self.groups.values())
343-
all_equipment.extend(self.schedules.values())
344-
345-
for equipment in all_equipment:
365+
for equipment in self.all_equipment:
346366
if equipment.system_id == system_id:
347367
return equipment
348368

tests/test_omnilogic.py

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
from typing import Any
2+
3+
from pyomnilogic_local.omnilogic import _check_duplicate_item_names
4+
5+
6+
class FakeEquipment:
7+
"""A fake equipment class for testing duplicate detection."""
8+
9+
def __init__(self, system_id: int, name: str) -> None:
10+
self.system_id = system_id
11+
self.name = name
12+
13+
14+
def test__check_duplicate_item_names() -> None:
15+
"""Test the duplicate name warning helper directly."""
16+
items: Any = [
17+
FakeEquipment(1, "Solar"),
18+
FakeEquipment(2, "Solar"),
19+
FakeEquipment(3, "Gas"),
20+
FakeEquipment(4, "Gas"),
21+
]
22+
warning = _check_duplicate_item_names(
23+
items,
24+
source_id="127.0.0.1:10444",
25+
)
26+
27+
assert warning == (
28+
"OmniLogic 127.0.0.1:10444 contains multiple items with the same name:\n"
29+
" - Gas: IDs 3, 4\n"
30+
" - Solar: IDs 1, 2\n"
31+
"Name-based lookups will return the first match. "
32+
"Consider using system_id-based lookups for reliability "
33+
"or renaming equipment on the OmniLogic controller to avoid duplicates."
34+
)
35+
36+
37+
def test__check_duplicate_item_names_different_host() -> None:
38+
"""Test the duplicate name warning helper with a different host."""
39+
items: Any = [
40+
FakeEquipment(1, "Solar"),
41+
FakeEquipment(2, "Solar"),
42+
]
43+
warning = _check_duplicate_item_names(
44+
items,
45+
source_id="10.0.0.1:3000",
46+
)
47+
48+
assert warning is not None
49+
assert "OmniLogic 10.0.0.1:3000 contains multiple items with the same name:" in warning
50+
51+
52+
def test__check_duplicate_item_names_no_duplicates() -> None:
53+
"""Test that the helper returns None when there are no duplicates."""
54+
items: Any = [
55+
FakeEquipment(1, "Solar"),
56+
FakeEquipment(2, "Gas"),
57+
]
58+
warning = _check_duplicate_item_names(
59+
items,
60+
source_id="127.0.0.1:10444",
61+
)
62+
assert warning is None

0 commit comments

Comments
 (0)