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.

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

Pilight switch: restore last state after restart #8580

Merged
merged 6 commits into from Aug 22, 2017

Conversation

janLo
Copy link
Contributor

@janLo janLo commented Jul 21, 2017

Description:

This uses the restore_state helper to set the last known state to
pilight switches when the devices are initialized after a HA
restart.

Without this HA forget the state on every restart and needs to be told
the state by retoggling the switches. This can cause unwanted effects
as a switch toggling may emit an RF signal.

Checklist:

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

  • Local tests with tox run successfully. Your PR cannot be merged unless tests pass

@mention-bot
Copy link

@janLo, thanks for your PR! By analyzing the history of the files in this pull request, we identified @DavidLP and @fabaff to be potential reviewers.

Copy link
Member

@pvizeli pvizeli left a comment

Choose a reason for hiding this comment

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

That is not the correct flow for restore a flow. Look into input_boolean to see the correct workflow on entity. please add also:

   @property 
     def assumed_state(self) -> bool: 
         """Return True if unable to access real state of the entity.""" 
         return True 

@janLo
Copy link
Contributor Author

janLo commented Jul 21, 2017

I don't understand what you mean with a workflow. Also there is no async_added_to_hass in the Pilight switch. I don't understand what assumed_state should give, I only want to restore the previous state if there is one, not change any behavior.

@pvizeli
Copy link
Member

pvizeli commented Jul 23, 2017

for history, we explain it on discord

EDIT:
You need add async_added_to_hass on that object. Also assumed_state should be set while it is true that we have no access to right state of it.

@janLo
Copy link
Contributor Author

janLo commented Jul 25, 2017

I can do and test that after August the 14th. Please leave the PR open.

"""Call when entity about to be added to hass."""
if self.entity_id is None:
self.entity_id = generate_entity_id(
ENTITY_ID_FORMAT, name, hass=hass)

Choose a reason for hiding this comment

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

undefined name 'name'
undefined name 'hass'

CONF_PROTOCOL, STATE_ON)
from homeassistant.helpers.entity import generate_entity_id
from homeassistant.helpers.restore_state import async_get_last_state
from homeassistant.util.async import run_coroutine_threadsafe

Choose a reason for hiding this comment

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

'homeassistant.util.async.run_coroutine_threadsafe' imported but unused

@janLo
Copy link
Contributor Author

janLo commented Jul 26, 2017

Found a few minutes before my holiday. It seems to work like the solution before.

@janLo
Copy link
Contributor Author

janLo commented Jul 26, 2017

And fixed a copy/paste error

@janLo
Copy link
Contributor Author

janLo commented Jul 26, 2017

@pvizeli Should be alright now?

"""Call when entity about to be added to hass."""
if self.entity_id is None:
self.entity_id = generate_entity_id(
ENTITY_ID_FORMAT, self._name, hass=self._hass)
Copy link
Member

Choose a reason for hiding this comment

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

Remove this part

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pvizeli done.

This uses the restore_state helper to set the last known state to
pilight switches when the devices are initialized after a HA
restart.

Without this HA forget the state on every restart and needs to be told
the sttae by retoggling the switches. This can cause unwanted effects
as a switch toggling may emit an RF signal.
Signed-off-by: Jan Losinski <losinski@wh2.tu-dresden.de>
@@ -12,10 +13,15 @@
import homeassistant.components.pilight as pilight
from homeassistant.components.switch import (SwitchDevice, PLATFORM_SCHEMA)
from homeassistant.const import (CONF_NAME, CONF_ID, CONF_SWITCHES, CONF_STATE,
CONF_PROTOCOL)
CONF_PROTOCOL, STATE_ON)
from homeassistant.helpers.entity import generate_entity_id

Choose a reason for hiding this comment

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

'homeassistant.helpers.entity.generate_entity_id' imported but unused


_LOGGER = logging.getLogger(__name__)

DOMAIN = 'switch'
Copy link
Contributor

Choose a reason for hiding this comment

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

Import ENTITY_ID_FORMAT from component instead.

Why do you need it 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.

This is a leftover from the entity-id-generation that was there in an earlier version.

Is there any documentation anywhere what entity ids are generated, who is responsible and at which point in time I can be sure that it exist?

@property
def assumed_state(self):
"""Return True if unable to access real state of the entity."""
return True

Choose a reason for hiding this comment

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

trailing whitespace

@@ -130,6 +139,11 @@ def should_poll(self):
"""No polling needed, state set when correct code is received."""
return False

@property
def assumed_state(self):
"""Return True if unable to access real state of the entity."""

Choose a reason for hiding this comment

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

trailing whitespace

@@ -130,6 +139,11 @@ def should_poll(self):
"""No polling needed, state set when correct code is received."""
return False

@property
def assumed_state(self):

Choose a reason for hiding this comment

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

trailing whitespace

@@ -130,6 +139,11 @@ def should_poll(self):
"""No polling needed, state set when correct code is received."""
return False

@property

Choose a reason for hiding this comment

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

trailing whitespace

@pvizeli
Copy link
Member

pvizeli commented Aug 22, 2017

We can merg it after test have pass.

@janLo you can use customize for change the assume state flag. But the default should be correct.

@pvizeli pvizeli merged commit fd6fd76 into home-assistant:dev Aug 22, 2017
@balloob balloob mentioned this pull request Aug 25, 2017
@home-assistant home-assistant locked and limited conversation to collaborators Dec 11, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants