New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update pyhomematic to 0.1.52 and add features for lights #18499

Merged
merged 5 commits into from Nov 19, 2018

Conversation

@danielperna84
Contributor

danielperna84 commented Nov 15, 2018

Description:

  • Fix for HM-CC-TC
  • Adds support for HomeMatic device: HmIP-PCBS, HmIP-MOD-TM, HmIP-PDT, HM-LC-RGBW-WM

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.

If the code communicates with devices, web services, or third-party tools:

  • New dependencies have been added to the REQUIREMENTS variable (example).
  • New or updated dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.

if self.supported_features & SUPPORT_COLOR:
self._data.update(
{"COLOR": STATE_UNKNOWN, "PROGRAM": STATE_UNKNOWN})

This comment has been minimized.

@pvizeli

pvizeli Nov 16, 2018

Member

STATE_UNKNOWN = None. We need clean up the others too, but later :)

@@ -53,15 +55,45 @@ def is_on(self):
@property
def supported_features(self):
"""Flag supported features."""
return SUPPORT_HOMEMATIC
if hasattr(self._hmdevice, "get_effect"):

This comment has been minimized.

@pvizeli

pvizeli Nov 16, 2018

Member

Can we look into hmdevice nodes if there exists the channel name? hasattr is very slow

@chr1st1ank

This comment has been minimized.

chr1st1ank commented Nov 16, 2018

Hi, actually I think it is not so simple. This workaround I wrote because the ColorLight and its parent Dimmer have no attribute in common we could use for this. You're saying hasattr() is to slow here?
Of course there are other ways, I am just not sure if they are better:

  • Use try/except
  • Cache the result of the hasattr() after the first call
  • ...
    Sadly the device is initialized late, so in the constructor the type is not yet known.
@danielperna84

This comment has been minimized.

Contributor

danielperna84 commented Nov 16, 2018

I've googled a bit. Seems this would be a possible solution:

if getattr(self._hmdevice, "get_effect", None) is not None:
    return SUPPORT_BRIGHTNESS | SUPPORT_COLOR | SUPPORT_EFFECT

What do you think?

@chr1st1ank

This comment has been minimized.

chr1st1ank commented Nov 16, 2018

Yes, that is another option. I now just tried everything out. Jupyter Notebook and %timeit is our friend.

There are differences but they not very impressive:

class A:
    def __init__(self):
        self.real_attr = "x"
    
    @property
    def attr_function(self):
        return "y"
    
class B:
    pass
    
class TestClass:
    def __init__(self, obj):
        self.obj = obj
        self.cache = None
        
    def test_hasattr(self):
        if hasattr(self.obj, "attr_function"):
            return True
        return False
        
    def test_getattr(self):
        if getattr(self.obj, "attr_function", None) is not None:
            return True
        return False
        
    def test_getattr_real(self):
        if hasattr(self.obj, "real_attr"):
            return True
        return False
        
    def test_try_except(self):
        try:
            x = self.obj.attr_function
            return True
        except:
            return False
        
    def test_hasattr_cashed(self):
        if self.cache == None:
            self.cache = hasattr(self.obj, "attr_function")
        return self.cache
t1 = TestClass(A())
t2 = TestClass(B())
%timeit (t1.test_hasattr() and t2.test_hasattr())
739 ns ± 12.5 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)
%timeit (t1.test_getattr() and t2.test_getattr())

717 ns ± 4.09 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)

%timeit (t1.test_getattr_real() and t2.test_getattr_real())

609 ns ± 1.66 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)

%timeit (t1.test_try_except() and t2.test_try_except())

1.08 µs ± 4.89 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)

%timeit (t1.test_hasattr_cashed() and t2.test_hasattr_cashed())

591 ns ± 1.76 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)

And finally, here comes the hypothetic part. If we had direct access to an attribute and could compare it, we get a slightly faster version than the version with the caching:

class C:
    x = 1
    
class D:
    x = 2
    
class TestClass:
    def __init__(self, obj):
        self.obj = obj
        
    def test_direct(self):
        if self.obj.x == 1:
            return True
        return False
    
t1 = TestClass(C())
t2 = TestClass(D())

%timeit (t1.test_direct() and t2.test_direct())

527 ns ± 11.4 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)

But actually to me it looks like all these differences are negligible. What do you think? Is this really a bottleneck?

@pvizeli

This comment has been minimized.

Member

pvizeli commented Nov 19, 2018

I think we lost the focus of my comment a bit :)

@pvizeli pvizeli merged commit fc4dd4e into home-assistant:dev Nov 19, 2018

5 checks passed

Hound No violations found. Woof!
WIP ready for review
Details
cla-bot Everyone involved has signed the CLA
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.02%) to 93.026%
Details

@wafflebot wafflebot bot removed the in progress label Nov 19, 2018

@danielperna84 danielperna84 deleted the danielperna84:homematic branch Nov 19, 2018

mxworm added a commit to mxworm/home-assistant that referenced this pull request Nov 19, 2018

Merge branch 'dev' of https://github.com/home-assistant/home-assistant
…into dev

* 'dev' of https://github.com/home-assistant/home-assistant:
  Bumped ghlocalapi to 0.1.0 (home-assistant#18584)
  Prefix all xiaomi_aqara events (home-assistant#17354)
  Fix MQTT async_added_to_hass (home-assistant#18575)
  Add mikrotik SSL support (home-assistant#17898)
  Reconfigure MQTT binary_sensor component if discovery info is changed (home-assistant#18169)
  Darksky: Expose missing conditions for day 0 forecast (home-assistant#18312)
  Update pyhomematic to 0.1.52 and add features for lights (home-assistant#18499)
  Fix for epson state not updating (home-assistant#18357)
  Support for Point component (home-assistant#17466)
  Template binary sensor to not track all state changes (home-assistant#18573)
  Correct cached stale device tracker handling (home-assistant#18572)
  Add support for sessions (home-assistant#18518)
  Remove turn_on and turn_off feature for clients (home-assistant#18234)
  Log delay and wait_template steps in scripts (home-assistant#18448)
  Logbook speedup (home-assistant#18376)
  Avoid race in entity_platform.async_add_entities() (home-assistant#18445)
  Fix small issue related to topic prefix (home-assistant#18512)
  Enable native support + ADB authentication for Fire TV (home-assistant#17767)
  Re-adding the season attribute (home-assistant#18523)

@balloob balloob referenced this pull request Nov 29, 2018

Merged

0.83 #18776

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment