Repository: XKNX/xknx
Pull request #1288 (2023-06-25 18:35:43)

Add state_address, respond_to_read and sync_state to DateTime device
<!--
  You are awesome! Thanks for contributing to our project!
  Please, DO NOT DELETE ANY TEXT from this template!.
-->
## Description
<!--
Please include a summary of the change and which issue is fixed.
Please also include relevant motivation and context.
List any dependencies that are required for this change.
-->
Add 
- group_address_state, 
- respond_to_read 
- sync_state 

to DateTime device so it can support custom times properly.

Fixes # (issue)

## Type of change
<!--
Please tick the applicable options.
NOTE: Ticking multiple options most likely indicates
that your change is to big and it is suggested to split it into several smaller PRs.
-->

- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
- [ ] Code quality improvements to existing code or addition of tests

## Checklist

- [ ] The documentation has been adjusted accordingly
- [x] Tests have been added that prove the fix is effective or that the feature works
- [x] The changes are documented in the changelog (docs/changelog.md)

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/XKNX/xknx/pull/1288?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX) Report
> Merging [#1288](https://app.codecov.io/gh/XKNX/xknx/pull/1288?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX) (9a5ac72) into [main](https://app.codecov.io/gh/XKNX/xknx/commit/3a80117485a63c94e7b20050224b3b6f5ee08ef5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX) (3a80117) will **increase** coverage by `0.01%`.
> The diff coverage is `91.66%`.

> :exclamation: Current head 9a5ac72 differs from pull request most recent head ec5394b. Consider uploading reports for the commit ec5394b to get more accurate results

[![Impacted file tree graph](https://app.codecov.io/gh/XKNX/xknx/pull/1288/graphs/tree.svg?width=650&height=150&src=pr&token=irWbIygS84&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX)](https://app.codecov.io/gh/XKNX/xknx/pull/1288?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX)

```diff
@@            Coverage Diff             @@
##             main    #1288      +/-   ##
==========================================
+ Coverage   96.53%   96.54%   +0.01%     
==========================================
  Files         151      151              
  Lines        9583     9590       +7     
==========================================
+ Hits         9251     9259       +8     
+ Misses        332      331       -1     
```


| [Impacted Files](https://app.codecov.io/gh/XKNX/xknx/pull/1288?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX) | Coverage Δ | |
|---|---|---|
| [xknx/devices/datetime.py](https://app.codecov.io/gh/XKNX/xknx/pull/1288?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX#diff-eGtueC9kZXZpY2VzL2RhdGV0aW1lLnB5) | `97.91% <91.66%> (+2.79%)` | :arrow_up: |


Patch:
diff --git a/docs/changelog.md b/docs/changelog.md
index f6a48322..291a0257 100644
--- a/docs/changelog.md
+++ b/docs/changelog.md
@@ -8,6 +8,10 @@ nav_order: 2
 
 # Unreleased changes
 
+### Devices
+
+- Add group_address_state, respond_to_read and sync_state arguments to DateTime devices.
+
 ### Internals
 
 - Remove pydocstyle and flake8 plugins, add pytest-icdiff to testing requirements.
diff --git a/xknx/devices/datetime.py b/xknx/devices/datetime.py
index 091bf83e..6ee05827 100644
--- a/xknx/devices/datetime.py
+++ b/xknx/devices/datetime.py
@@ -9,6 +9,7 @@ from __future__ import annotations
 import asyncio
 from collections.abc import Iterator
 from functools import partial
+import logging
 import time
 from typing import TYPE_CHECKING
 
@@ -21,6 +22,8 @@ if TYPE_CHECKING:
     from xknx.telegram import Telegram
     from xknx.xknx import XKNX
 
+logger = logging.getLogger("xknx.log")
+
 
 class DateTime(Device):
     """Class for virtual date/time device."""
@@ -32,16 +35,28 @@ class DateTime(Device):
         broadcast_type: str = "TIME",
         localtime: bool = True,
         group_address: GroupAddressesType | None = None,
+        group_address_state: GroupAddressesType | None = None,
+        respond_to_read: bool = False,
+        sync_state: bool | int | float | str = True,
         device_updated_cb: DeviceCallbackType[DateTime] | None = None,
     ):
         """Initialize DateTime class."""
         super().__init__(xknx, name, device_updated_cb)
         self.localtime = localtime
+        if localtime and group_address_state is not None:
+            logger.warning(
+                "State address invalid in DateTime device when using `localtime=True`. Ignoring `group_address_state=%s` argument.",
+                group_address_state,
+            )
+            # state address invalid for localtime - therefore sync_state doesn't apply for localtime
+            group_address_state = None
+        self.respond_to_read = respond_to_read
         self._broadcast_type = broadcast_type.upper()
-        self._remote_value = RemoteValueDateTime(
+        self.remote_value = RemoteValueDateTime(
             xknx,
             group_address=group_address,
-            sync_state=False,
+            group_address_state=group_address_state,
+            sync_state=sync_state,
             value_type=broadcast_type,
             device_name=name,
             after_update_cb=self.after_update,
@@ -50,7 +65,7 @@ class DateTime(Device):
 
     def _iter_remote_values(self) -> Iterator[RemoteValueDateTime]:
         """Iterate the devices RemoteValue classes."""
-        yield self._remote_value
+        yield self.remote_value
 
     def _iter_tasks(self) -> Iterator[Task | None]:
         """Iterate the device tasks."""
@@ -75,28 +90,33 @@ class DateTime(Device):
 
     async def broadcast_localtime(self, response: bool = False) -> None:
         """Broadcast the local time to KNX bus."""
-        await self._remote_value.set(time.localtime(), response=response)
+        await self.remote_value.set(time.localtime(), response=response)
 
     async def set(self, struct_time: time.struct_time) -> None:
         """Set time and send to KNX bus."""
-        await self._remote_value.set(struct_time)
+        await self.remote_value.set(struct_time)
 
     async def process_group_read(self, telegram: Telegram) -> None:
         """Process incoming GROUP READ telegram."""
         if self.localtime:
             await self.broadcast_localtime(True)
-        else:
-            await self._remote_value.respond()
+        elif (
+            self.respond_to_read
+            and telegram.destination_address == self.remote_value.group_address
+        ):
+            await self.remote_value.respond()
 
     async def sync(self, wait_for_result: bool = False) -> None:
         """Read state of device from KNX bus. Used here to broadcast time to KNX bus."""
         if self.localtime:
             await self.broadcast_localtime(response=False)
+        else:
+            await super().sync(wait_for_result)
 
     def __str__(self) -> str:
         """Return object as readable string."""
         return (
             f'<DateTime name="{self.name}" '
-            f"_remote_value={self._remote_value.group_addr_str()} "
+            f"remote_value={self.remote_value.group_addr_str()} "
             f'broadcast_type="{self._broadcast_type}" />'
         )


Test patch:
diff --git a/test/devices_tests/datetime_test.py b/test/devices_tests/datetime_test.py
index a98c4bda..ef1d6847 100644
--- a/test/devices_tests/datetime_test.py
+++ b/test/devices_tests/datetime_test.py
@@ -6,7 +6,7 @@ from xknx import XKNX
 from xknx.devices import DateTime
 from xknx.dpt import DPTArray
 from xknx.telegram import GroupAddress, Telegram
-from xknx.telegram.apci import GroupValueRead, GroupValueResponse
+from xknx.telegram.apci import GroupValueRead, GroupValueResponse, GroupValueWrite
 
 
 class TestDateTime:
@@ -17,6 +17,26 @@ class TestDateTime:
         """Cancel broadcast_task."""
         self.datetime.__del__()
 
+    #
+    # SET Time
+    #
+    async def test_process_set_custom_time(self):
+        """Test setting a new time."""
+        xknx = XKNX()
+        self.datetime = DateTime(
+            xknx,
+            "TestDateTime",
+            group_address="1/2/3",
+            broadcast_type="TIME",
+            localtime=False,
+        )
+        await self.datetime.set(time.struct_time([2017, 1, 7, 9, 13, 14, 6, 0, 0]))
+        telegram = xknx.telegrams.get_nowait()
+        assert telegram == Telegram(
+            destination_address=GroupAddress("1/2/3"),
+            payload=GroupValueWrite(DPTArray((0xE9, 0xD, 0xE))),
+        )
+
     #
     # SYNC DateTime
     #
@@ -68,13 +88,15 @@ class TestDateTime:
     #
     # SYNC Time
     #
-    async def test_sync_time(self):
+    async def test_sync_time_local(self):
         """Test sync function / sending group reads to KNX bus."""
         xknx = XKNX()
         self.datetime = DateTime(
-            xknx, "TestDateTime", group_address="1/2/3", broadcast_type="TIME"
+            xknx,
+            "TestDateTime",
+            group_address="1/2/3",
+            broadcast_type="TIME",
         )
-
         with patch("time.localtime") as mock_time:
             mock_time.return_value = time.struct_time([2017, 1, 7, 9, 13, 14, 6, 0, 0])
             await self.datetime.sync()
@@ -84,13 +106,31 @@ class TestDateTime:
         assert len(telegram.payload.value.value) == 3
         assert telegram.payload.value.value == (0xE9, 0x0D, 0x0E)
 
+    async def test_sync_time_custom(self):
+        """Test sync function / sending group reads to KNX bus."""
+        xknx = XKNX()
+        self.datetime = DateTime(
+            xknx,
+            "TestDateTime",
+            group_address="1/2/3",
+            group_address_state="1/2/4",
+            broadcast_type="TIME",
+            localtime=False,
+        )
+        assert self.datetime.has_group_address(GroupAddress("1/2/4"))
+        await self.datetime.sync()
+
+        telegram = xknx.telegrams.get_nowait()
+        assert telegram.destination_address == GroupAddress("1/2/4")
+        assert isinstance(telegram.payload, GroupValueRead)
+
     #
     # PROCESS
     #
     #
     # TEST PROCESS
     #
-    async def test_process_read(self):
+    async def test_process_read_localtime(self):
         """Test test process a read telegram from KNX bus."""
         xknx = XKNX()
         self.datetime = DateTime(
@@ -110,14 +150,58 @@ class TestDateTime:
             payload=GroupValueResponse(DPTArray((0xE9, 0xD, 0xE))),
         )
 
+    async def test_process_read_custom_time(self):
+        """Test test process a read telegram from KNX bus."""
+        xknx = XKNX()
+        self.datetime = DateTime(
+            xknx,
+            "TestDateTime",
+            group_address="1/2/3",
+            broadcast_type="TIME",
+            localtime=False,
+            respond_to_read=True,
+        )
+
+        self.datetime.remote_value.value = time.struct_time(
+            [2017, 1, 7, 9, 13, 14, 6, 0, 0]
+        )
+        telegram_read = Telegram(
+            destination_address=GroupAddress("1/2/3"), payload=GroupValueRead()
+        )
+        await self.datetime.process(telegram_read)
+
+        telegram = xknx.telegrams.get_nowait()
+        assert telegram == Telegram(
+            destination_address=GroupAddress("1/2/3"),
+            payload=GroupValueResponse(DPTArray((0xE9, 0xD, 0xE))),
+        )
+
     #
     # TEST HAS GROUP ADDRESS
     #
-    def test_has_group_address(self):
+    async def test_has_group_address_localtime(self):
         """Test if has_group_address function works."""
         xknx = XKNX()
         self.datetime = DateTime(
-            xknx, "TestDateTime", group_address="1/2/3", localtime=False
+            xknx,
+            "TestDateTime",
+            group_address="1/2/3",
+            group_address_state="1/2/4",
+            localtime=True,
         )
         assert self.datetime.has_group_address(GroupAddress("1/2/3"))
+        # group_address_state ignored when using localtime
         assert not self.datetime.has_group_address(GroupAddress("1/2/4"))
+
+    async def test_has_group_address_custom_time(self):
+        """Test if has_group_address function works."""
+        xknx = XKNX()
+        self.datetime = DateTime(
+            xknx,
+            "TestDateTime",
+            group_address="1/2/3",
+            group_address_state="1/2/4",
+            localtime=False,
+        )
+        assert self.datetime.has_group_address(GroupAddress("1/2/3"))
+        assert self.datetime.has_group_address(GroupAddress("1/2/4"))
diff --git a/test/str_test.py b/test/str_test.py
index 6b26172c..ee8df198 100644
--- a/test/str_test.py
+++ b/test/str_test.py
@@ -386,7 +386,7 @@ class TestStringRepresentations:
         date_time = DateTime(xknx, name="Zeit", group_address="1/2/3", localtime=False)
         assert (
             str(date_time)
-            == '<DateTime name="Zeit" _remote_value=<1/2/3, None, [], None /> broadcast_type="TIME" />'
+            == '<DateTime name="Zeit" remote_value=<1/2/3, None, [], None /> broadcast_type="TIME" />'
         )
 
     def test_could_not_parse_telegramn_exception(self):