Skip to content
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.

Sign up for GitHub

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

Expose internal states and fixed on/off state of Dyson Fans #15716

Merged
merged 6 commits into from
Aug 1, 2018
Merged

Expose internal states and fixed on/off state of Dyson Fans #15716

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e0bb523
exposing internal state and fixed onoff state
soraxas Jul 28, 2018
629f609
fixed styling
soraxas Jul 28, 2018
2007cfd
revert file mode changes
soraxas Jul 28, 2018
9710cfe
removed self type hints
soraxas Jul 31, 2018
80d3180
better unit test and changed the way to return attributes
soraxas Aug 1, 2018
535bcf8
made wolfie happy
soraxas Aug 1, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion homeassistant/components/fan/dyson.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@

CONF_NIGHT_MODE = 'night_mode'

ATTR_IS_NIGHT_MODE = 'is_night_mode'
ATTR_IS_AUTO_MODE = 'is_auto_mode'

DEPENDENCIES = ['dyson']
DYSON_FAN_DEVICES = 'dyson_fan_devices'

Expand Down Expand Up @@ -159,7 +162,7 @@ def oscillating(self):
def is_on(self):
"""Return true if the entity is on."""
if self._device.state:
return self._device.state.fan_state == "FAN"
return self._device.state.fan_mode == "FAN"
return False

@property
Expand Down Expand Up @@ -233,3 +236,11 @@ def speed_list(self: ToggleEntity) -> list:
def supported_features(self: ToggleEntity) -> int:
"""Flag supported features."""
return SUPPORT_OSCILLATE | SUPPORT_SET_SPEED

@property
def state_attributes(self: ToggleEntity) -> dict:
"""Return optional state attributes."""
attributes = super().state_attributes
Copy link
Contributor

@awarecan awarecan Jul 28, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need call super,

attributes = {}

or, you use just use one statement

return {
    ATTR_IS_NIGHT_MODE: self.is_night_mode,
    ATTR_IS_AUTO_MODE: self.is_auto_mode,
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes originally I thought so too! ...But appreantly doing so will lose its predefined attributes, such as oscillating, speed_list, which are defined in its def supported_features and exposed by the super().state_attributes in __init__.py

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, I was confused by the typing hints, state_attributes(self: ToggleEntity. so, self is FanEntity, not ToggleEntity, this typing was wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean I should change it to def state_attributes(self: FanEntity) -> dict:? I was just following the type hint of self from same class and the state_attributes's super class method signature

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is so wrong, self should not have type hint at all.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does that mean the type hints in __ init __.py are wrong? I myself am not sure because these are new to me. Let me know and if so I can simply delete the type hint no problem! Thanks

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see #15732

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't needed. device_state_attributes will be merged with state_attributes in the entity base class.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh okay I see!

attributes[ATTR_IS_NIGHT_MODE] = self.is_night_mode
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this code right? self._device.is_night_mode?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes the code is correct. The property is defined in line 185. Similar for self.is_auto_mode.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, i see.

attributes[ATTR_IS_AUTO_MODE] = self.is_auto_mode
return attributes
27 changes: 26 additions & 1 deletion tests/components/fan/test_dyson.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
"""Test the Dyson fan component."""
import asyncio
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'asyncio' imported but unused

import unittest
from unittest import mock

from homeassistant.components.dyson import DYSON_DEVICES
from homeassistant.components.fan import dyson
from homeassistant.components.fan import (dyson, ATTR_SPEED, ATTR_SPEED_LIST,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'homeassistant.components.fan.ATTR_DIRECTION' imported but unused

ATTR_OSCILLATING)
from tests.common import get_test_home_assistant
from libpurecoollink.const import FanSpeed, FanMode, NightMode, Oscillation
from libpurecoollink.dyson_pure_state import DysonPureCoolState
Expand Down Expand Up @@ -91,6 +93,29 @@ def _add_device(devices):
self.hass.data[dyson.DYSON_DEVICES] = [device_fan, device_non_fan]
dyson.setup_platform(self.hass, None, _add_device)

def test_get_state_attributes(self):
"""Test async added to hass."""
device = _get_device_on()
component = dyson.DysonPureCoolLinkDevice(self.hass, device)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not a component, but an entity.

attributes = component.state_attributes
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be better to set up the platform, add an entity and get the state attributes for the entity from the core state machine.

assert dyson.ATTR_IS_NIGHT_MODE in attributes
assert dyson.ATTR_IS_AUTO_MODE in attributes
assert ATTR_SPEED in attributes
assert ATTR_SPEED_LIST in attributes
assert ATTR_OSCILLATING in attributes

def test_async_added_to_hass(self):
"""Test async added to hass."""
loop = asyncio.new_event_loop()

async def run():
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

expected 1 blank line before a nested definition, found 0

"""Test async function."""
device = _get_device_on()
component = dyson.DysonPureCoolLinkDevice(self.hass, device)
await component.async_added_to_hass()
assert device.add_message_listener.called
loop.run_until_complete(run())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's better to let the home assistant core set up the platform and add an entity. The core will then call async_added_to_hass on the entity, and we can assert after that.

Use setup_component to set up the component/platform.


def test_dyson_set_speed(self):
"""Test set fan speed."""
device = _get_device_on()
Expand Down