From df0a7f4c361d114cedf9fdc9ddf996d71e6039c2 Mon Sep 17 00:00:00 2001 From: Sebastian Templ Date: Wed, 5 Sep 2018 22:44:44 +0200 Subject: [PATCH] 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 https://github.com/home-assistant/home-assistant/issues/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 --- pyHS100/smartbulb.py | 23 +++++++++++++++++++---- pyHS100/tests/test_bulb.py | 17 +++++++++++++++++ 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/pyHS100/smartbulb.py b/pyHS100/smartbulb.py index b1638dbe..8f0bce6b 100644 --- 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 @@ class SmartBulb(SmartDevice): 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 @@ class SmartBulb(SmartDevice): 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 index 865f0b17..fce31032 100644 --- a/pyHS100/tests/test_bulb.py +++ b/pyHS100/tests/test_bulb.py @@ -208,6 +208,23 @@ class TestSmartBulb(TestCase): 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