Return on_since only when its available and the device is on (#48)

* moves on_since property to smartdevice class, as it is not plug only
* returns None if the value is not available (some bulbs), or if the device is off

kasa/smartdevice.py

@@ -15,7 +15,7 @@ import functools
 import inspect
 import logging
 from dataclasses import dataclass
-from datetime import datetime
+from datetime import datetime, timedelta
 from enum import Enum
 from typing import Any, Dict, List, Optional
 
@@ -554,6 +554,23 @@ class SmartDevice:
         """
         raise NotImplementedError("Device subclass needs to implement this.")
 
+    @property  # type: ignore
+    @requires_update
+    def on_since(self) -> Optional[datetime]:
+        """Return pretty-printed on-time, if available.
+
+        Returns None if the device is turned off or does not report it.
+        """
+        if "on_time" not in self.sys_info:
+            return None
+
+        if self.is_off:
+            return None
+
+        on_time = self.sys_info["on_time"]
+
+        return datetime.now() - timedelta(seconds=on_time)
+
     @property  # type: ignore
     @requires_update
     def state_information(self) -> Dict[str, Any]:

kasa/smartplug.py

@@ -1,5 +1,4 @@
 """Module for plugs."""
-import datetime
 import logging
 from typing import Any, Dict
 
@@ -81,18 +80,6 @@ class SmartPlug(SmartDevice):
         await self._query_helper("system", "set_led_off", {"off": int(not state)})
         await self.update()
 
-    @property  # type: ignore
-    @requires_update
-    def on_since(self) -> datetime.datetime:
-        """Return pretty-printed on-time.
-
-        :return: datetime for on since
-        :rtype: datetime
-        """
-        on_time = self.sys_info["on_time"]
-
-        return datetime.datetime.now() - datetime.timedelta(seconds=on_time)
-
     @property  # type: ignore
     @requires_update
     def state_information(self) -> Dict[str, Any]:

kasa/smartstrip.py

@@ -2,9 +2,9 @@
 
 .. todo:: describe how this interfaces with single plugs.
 """
-import datetime
 import logging
 from collections import defaultdict
+from datetime import datetime, timedelta
 from typing import Any, DefaultDict, Dict, List, Optional
 
 from kasa.smartdevice import (
@@ -111,9 +111,12 @@ class SmartStrip(SmartDevice):
 
     @property  # type: ignore
     @requires_update
-    def on_since(self) -> datetime.datetime:
+    def on_since(self) -> Optional[datetime]:
         """Return the maximum on-time of all outlets."""
-        return max(plug.on_since for plug in self.plugs)
+        if self.is_off:
+            return None
+
+        return max(plug.on_since for plug in self.plugs if plug.on_since is not None)
 
     @property  # type: ignore
     @requires_update
@@ -317,16 +320,19 @@ class SmartStripPlug(SmartPlug):
 
     @property  # type: ignore
     @requires_update
-    def on_since(self) -> datetime.datetime:
+    def on_since(self) -> Optional[datetime]:
         """Return pretty-printed on-time.
 
         :return: datetime for on since
         :rtype: datetime
         """
+        if self.is_off:
+            return None
+
         info = self._get_child_info()
         on_time = info["on_time"]
 
-        return datetime.datetime.now() - datetime.timedelta(seconds=on_time)
+        return datetime.now() - timedelta(seconds=on_time)
 
     @property  # type: ignore
     @requires_update

kasa/tests/conftest.py

@@ -98,7 +98,6 @@ def dev(request):
     if ip:
         d = asyncio.run(Discover.discover_single(ip))
         asyncio.run(d.update())
-        print(d.model)
         if d.model in file:
             return d
         return

kasa/tests/test_fixtures.py

@@ -1,5 +1,5 @@
 import asyncio
-import datetime
+from datetime import datetime
 from unittest.mock import patch
 
 import pytest
@@ -230,9 +230,16 @@ async def test_led(dev):
     await dev.set_led(original)
 
 
-@plug
+@turn_on
-async def test_on_since(dev):
+async def test_on_since(dev, turn_on):
-    assert isinstance(dev.on_since, datetime.datetime)
+    await handle_turn_on(dev, turn_on)
+    orig_state = dev.is_on
+    if "on_time" not in dev.sys_info and not dev.is_strip:
+        assert dev.on_since is None
+    elif orig_state:
+        assert isinstance(dev.on_since, datetime)
+    else:
+        assert dev.on_since is None
 
 
 async def test_icon(dev):
@@ -240,7 +247,7 @@ async def test_icon(dev):
 
 
 async def test_time(dev):
-    assert isinstance(await dev.get_time(), datetime.datetime)
+    assert isinstance(await dev.get_time(), datetime)
     # TODO check setting?
 
 
@@ -421,8 +428,20 @@ async def test_children_alias(dev):
 
 @strip
 async def test_children_on_since(dev):
+    on_sinces = []
     for plug in dev.plugs:
-        assert plug.on_since
+        if plug.is_on:
+            on_sinces.append(plug.on_since)
+            assert isinstance(plug.on_since, datetime)
+        else:
+            assert plug.on_since is None
+
+    if dev.is_off:
+        assert dev.on_since is None
+    # TODO: testing this would require some mocking utcnow which is not
+    # very straightforward.
+    # else:
+    #    assert dev.on_since == max(on_sinces)
 
 
 @strip