Fix bug that changed brightness at each HSV update (#124)

* Fix bug that changed brightness at each hsv update The HSV setter should accept a percentage for the brightness value but actually assumed the brightness to be in absolute values between 1 and 255. This resulted in brightness reductions at each HSV update, in steps of 100% -> 100/255=39% -> 39/255=15% -> ... (see also home-assistant/core#15582, where I originally reported this bug). * Modify HSV property to return brightness in percent Switch from reported brightness values of 1..255 to percentage values, for consistency with the apidoc and 8761dd8. * Add checks and tests for the hsv setter - make sure that new (hue, saturation, brightness) values are within their valid ranges (0..255, 0..100, 0..100) and raise SmartDeviceException if they are not - add test function for the hsv setter
GadgetReactor · Sep 5, 2018 · df0a7f4 · df0a7f4
1 parent ce89c0d
commit df0a7f4
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 4 deletions.
diff --git a/pyHS100/smartbulb.py b/pyHS100/smartbulb.py
@@ -1,4 +1,4 @@
-from pyHS100 import SmartDevice
+from pyHS100 import SmartDevice, SmartDeviceException
 import re
 from typing import Any, Dict, Optional, Tuple
 
@@ -124,11 +124,11 @@ def hsv(self) -> Optional[Tuple[int, int, int]]:
         if not self.is_on:
             hue = light_state['dft_on_state']['hue']
             saturation = light_state['dft_on_state']['saturation']
-            value = int(light_state['dft_on_state']['brightness'] * 255 / 100)
+            value = light_state['dft_on_state']['brightness']
         else:
             hue = light_state['hue']
             saturation = light_state['saturation']
-            value = int(light_state['brightness'] * 255 / 100)
+            value = light_state['brightness']
 
         return hue, saturation, value
 
@@ -142,10 +142,25 @@ def hsv(self, state: Tuple[int, int, int]):
         if not self.is_color:
             return None
 
+        if not isinstance(state[0], int) or not (0 <= state[0] <= 255):
+            raise SmartDeviceException(
+                    'Invalid hue value: {} '
+                    '(valid range: 0-255)'.format(state[0]))
+
+        if not isinstance(state[1], int) or not (0 <= state[1] <= 100):
+            raise SmartDeviceException(
+                    'Invalid saturation value: {} '
+                    '(valid range: 0-100%)'.format(state[1]))
+
+        if not isinstance(state[2], int) or not (0 <= state[2] <= 100):
+            raise SmartDeviceException(
+                    'Invalid brightness value: {} '
+                    '(valid range: 0-100%)'.format(state[2]))
+
         light_state = {
             "hue": state[0],
             "saturation": state[1],
-            "brightness": int(state[2] * 100 / 255),
+            "brightness": state[2],
             "color_temp": 0
             }
         self.set_light_state(light_state)

diff --git a/pyHS100/tests/test_bulb.py b/pyHS100/tests/test_bulb.py
@@ -208,6 +208,23 @@ def test_temperature_range(self):
         with self.assertRaises(ValueError):
             self.bulb.color_temp = 10000
 
+    def test_hsv(self):
+        hue, saturation, brightness = self.bulb.hsv
+        self.assertTrue(0 <= hue <= 255)
+        self.assertTrue(0 <= saturation <= 100)
+        self.assertTrue(0 <= brightness <= 100)
+        for invalid_hue in [-1, 256, 0.5]:
+            with self.assertRaises(SmartDeviceException):
+                self.bulb.hsv = (invalid_hue, 0, 0)
+
+        for invalid_saturation in [-1, 101, 0.5]:
+            with self.assertRaises(SmartDeviceException):
+                self.bulb.hsv = (0, invalid_saturation, 0)
+
+        for invalid_brightness in [-1, 101, 0.5]:
+            with self.assertRaises(SmartDeviceException):
+                self.bulb.hsv = (0, 0, invalid_brightness)
+
 
 class TestSmartBulbLB100(TestSmartBulb):
     SYSINFO = sysinfo_lb100