Repository: cgarwood/python-openzwave-mqtt

Issue:
Convert item_id to int
All IDs are ints in OZW. We now extract the ID from the topic and pass them in as strins (`item_id`) to the constructor of `ZWaveBase`. We should convert these to integers as data posted to OZW topics will also refer to nodes as integers. Makes it easier on the consumer of this lib. 

Conversion should happen in `ItemCollection` (base.py) where it instantiates the model: `self.item_class(…)`. 

Discussion:

Patch:
diff --git a/openzwavemqtt/base.py b/openzwavemqtt/base.py
index 350ef4a..1d2f426 100644
--- a/openzwavemqtt/base.py
+++ b/openzwavemqtt/base.py
@@ -31,7 +31,7 @@ class ItemCollection:
 
     def process_message(self, topic: Deque[str], message: dict):
         """Process a new message."""
-        item_id = topic.popleft()
+        item_id = int(topic.popleft())
         item = self.collection.get(item_id)
         added = False
 
@@ -103,7 +103,7 @@ class ZWaveBase(ABC):
         options: OZWOptions,
         parent: Optional["ZWaveBase"],
         topic_part: str,
-        item_id: Optional[str],
+        item_id: Optional[int],
     ):
         """Initialize the model."""
         # Runtime options
diff --git a/openzwavemqtt/models/node.py b/openzwavemqtt/models/node.py
index 3ade28b..4eb7f50 100644
--- a/openzwavemqtt/models/node.py
+++ b/openzwavemqtt/models/node.py
@@ -1,5 +1,5 @@
 """Model for Node."""
-from typing import List
+from typing import Iterable, List, TYPE_CHECKING
 
 from ..base import ZWaveBase, ItemCollection
 from ..const import EVENT_NODE_ADDED, EVENT_NODE_CHANGED, EVENT_NODE_REMOVED
@@ -7,6 +7,9 @@ from ..const import EVENT_NODE_ADDED, EVENT_NODE_CHANGED, EVENT_NODE_REMOVED
 from .node_statistics import OZWNodeStatistics
 from .node_instance import OZWNodeInstance
 
+if TYPE_CHECKING:
+    from .value import OZWValue
+
 
 class OZWNode(ZWaveBase):
 
@@ -199,16 +202,15 @@ class OZWNode(ZWaveBase):
         """Return Neighbors."""
         return self.data.get("Neighbors")
 
-    @property
-    def values(self):
-        """Return list of OZWValue child items."""
-        _values = []
-        for instance in self.collections["instance"]:
-            for cc in instance.collections["commandclass"]:
-                for value in cc.collections["value"]:
-                    _values.append(value)
-
-        return _values
+    def values(self) -> Iterable["OZWValue"]:
+        """Iterate over all OZWValue child items."""
+        # pylint: disable=no-member
+        return (
+            value
+            for instance in self.instances()
+            for cc in instance.commandclasses()
+            for value in cc.values()
+        )
 
     def create_collections(self):
         """Create collections that Node supports."""


Test patch:
diff --git a/test/models/test_instance_statistics.py b/test/models/test_instance_statistics.py
index d2cbffe..e64722f 100644
--- a/test/models/test_instance_statistics.py
+++ b/test/models/test_instance_statistics.py
@@ -26,7 +26,7 @@ def test_statistics(mgr):
 
     mgr.mock_receive_json("OpenZWave/1", {})
     mgr.mock_receive_json("OpenZWave/1/statistics/", RESPONSE_JSON)
-    statistics = mgr.get_instance("1").get_statistics()
+    statistics = mgr.get_instance(1).get_statistics()
 
     assert statistics.sof_count == 148
     assert statistics.read_count == 147
diff --git a/test/models/test_node_statistics.py b/test/models/test_node_statistics.py
index 2ace4ad..1606b1b 100644
--- a/test/models/test_node_statistics.py
+++ b/test/models/test_node_statistics.py
@@ -38,9 +38,9 @@ def test_statistics(mgr):
     mgr.mock_receive_json("OpenZWave/1", {})
     mgr.mock_receive_json("OpenZWave/1/node/2", {})
     mgr.mock_receive_json("OpenZWave/1/node/2/statistics/", RESPONSE_JSON)
-    statistics = mgr.get_instance("1").get_node("2").get_statistics()
+    statistics = mgr.get_instance(1).get_node(2).get_statistics()
     assert statistics.ack_channel == 0
     assert statistics.average_response_rtt == 47
     assert statistics.average_request_rtt == 31
     assert statistics.send_count == 10
-    assert statistics.parent.id == "2"
+    assert statistics.parent.id == 2
diff --git a/test/models/test_value.py b/test/models/test_value.py
index e6e53d9..51854ab 100644
--- a/test/models/test_value.py
+++ b/test/models/test_value.py
@@ -19,12 +19,12 @@ def test_value_events(mgr):
         "OpenZWave/1/node/2/instance/1/commandclass/4/value/3", {"Value": "yo"}
     )
     assert len(events) == 1
-    assert events[0].id == "3"
+    assert events[0].id == 3
     assert events[0].value == "yo"
-    assert events[0].parent.id == "4"
+    assert events[0].parent.id == 4
 
     # Test OZWNode.values shortcut
-    assert mgr.get_instance("1").get_node("2").values[0].id == "3"
+    assert list(mgr.get_instance(1).get_node(2).values())[0].id == 3
 
     # Listen for value changed
     mgr.options.listen(EVENT_VALUE_CHANGED, events.append)
@@ -32,13 +32,13 @@ def test_value_events(mgr):
         "OpenZWave/1/node/2/instance/1/commandclass/4/value/3", {"Value": "yo2"}
     )
     assert len(events) == 2
-    assert events[0].id == "3"
+    assert events[0].id == 3
     assert events[0].value == "yo2"
 
     # Show how to use collection helpers
     assert (
-        list(mgr.get_instance("1").get_node("2").get_instance("1").commandclasses())[0]
-        .get_value("3")
+        list(mgr.get_instance(1).get_node(2).get_instance(1).commandclasses())[0]
+        .get_value(3)
         .value
         == "yo2"
     )
@@ -47,4 +47,4 @@ def test_value_events(mgr):
     mgr.options.listen(EVENT_VALUE_REMOVED, events.append)
     mgr.receive_message("OpenZWave/1/node/2/instance/1/commandclass/4/value/3", "")
     assert len(events) == 3
-    assert events[0].id == "3"
+    assert events[0].id == 3
diff --git a/test/test_base.py b/test/test_base.py
index a684063..a810dc0 100644
--- a/test/test_base.py
+++ b/test/test_base.py
@@ -45,11 +45,11 @@ def test_direct_collection(level1, caplog):
     level1.process_message(deque(), {"info": 1})
     level1.process_message(deque(["2"]), {"info": 1})
     level1.process_message(deque(["2", "3"]), {"hello": 1})
-    assert level1.get_level2("2").get_level3("3").hello == 1
+    assert level1.get_level2(2).get_level3(3).hello == 1
 
     # Only works on numbers
     level1.process_message(deque(["2", "a"]), {"hello": 1})
-    assert level1.get_level2("2").get_level3("a") is None
+    assert level1.get_level2(2).get_level3("a") is None
     assert "cannot process message" in caplog.text
 
 
@@ -59,17 +59,17 @@ def test_pending_messages(level1, options):
 
     # Only message for level3 has been received, level2 is none
     level1.process_message(deque(["2", "3"]), {"hello": 1})
-    assert level1.get_level2("2") is None
+    assert level1.get_level2(2) is None
     assert events == []
 
     # Message for level2, level3 received, level1 still None
     level1.process_message(deque(["2"]), {"hello": 1})
-    assert level1.get_level2("2") is None
+    assert level1.get_level2(2) is None
     assert events == []
 
     # Level 1 receives data, process all child messages.
     level1.process_message(deque(), {"info": 1})
-    assert level1.get_level2("2").get_level3("3").hello == 1
+    assert level1.get_level2(2).get_level3(3).hello == 1
     assert events == ["level2_added", "level3_added"]
 
 
@@ -111,11 +111,11 @@ def test_topic(options):
     level1.process_message(deque(["2", "3", "statistics"]), {"hello": 1})
 
     assert (
-        level1.get_level2("2").get_level3("3").get_level4("4").topic
+        level1.get_level2(2).get_level3(3).get_level4(4).topic
         == "OpenZWave/2/3/level4/4"
     )
     assert (
-        level1.get_level2("2").get_level3("3").get_statistics().topic
+        level1.get_level2(2).get_level3(3).get_statistics().topic
         == "OpenZWave/2/3/statistics"
     )
 
@@ -126,9 +126,7 @@ def test_automatic_collections(level1):
     level1.process_message(deque(["2", "3"]), {"hello": 1})
 
     # Test overridden using PLURAL_NAME
-    assert list(level1.level_twos()) == [level1.get_level2("2")]
+    assert list(level1.level_twos()) == [level1.get_level2(2)]
 
     # Test default name
-    assert list(level1.get_level2("2").level3s()) == [
-        level1.get_level2("2").get_level3("3")
-    ]
+    assert list(level1.get_level2(2).level3s()) == [level1.get_level2(2).get_level3(3)]