Use aioharmony for remote.harmony platform #19595
Conversation
Use aioharmony to interact with Harmony hub. Due to this following improvements: -) Setting of available state for entity -) Automatic config update if configuration changes (including updating file containing config) -) Allow using of device name instead of number -) When sending command with repeat, nothing else will be able to put a IR command in between
ghost
added
the
| self.new_config() | ||
| self._available = True | ||
|
|
||
| async def got_disconnected(self, _ = None): |
There was a problem hiding this comment.
unexpected spaces around keyword / parameter equals
| self.schedule_update_ha_state() | ||
| self.write_config_file() | ||
|
|
||
| def got_connected(self, _ = None): |
There was a problem hiding this comment.
unexpected spaces around keyword / parameter equals
| _LOGGER.debug("%s activity reported as: %s", self._name, activity_name) | ||
| self._current_activity = activity_name | ||
| self._state = bool(self._current_activity != 'PowerOff') | ||
| self.schedule_update_ha_state() | ||
|
|
||
| def new_config(self, _ = None): |
There was a problem hiding this comment.
unexpected spaces around keyword / parameter equals
|
|
||
| _, self._current_activity = self._client.current_activity | ||
| self._state = bool(self._current_activity != 'PowerOff') | ||
| self.schedule_update_ha_state() |
There was a problem hiding this comment.
If this is called in async context we should use the async api, async_schedule_update_ha_state.
| """Call for updating the current activity.""" | ||
| activity_name = self._client.get_activity_name(activity_id) | ||
| activity_id, activity_name = activity_info | ||
| _LOGGER.debug("%s activity reported as: %s", self._name, activity_name) | ||
| self._current_activity = activity_name | ||
| self._state = bool(self._current_activity != 'PowerOff') | ||
| self.schedule_update_ha_state() |
There was a problem hiding this comment.
Is this called in async context?
|
|
||
| if not self._available: | ||
| # Still disconnected. Let the state engine know. | ||
| self.schedule_update_ha_state() |
| _, self._current_activity = self._client.current_activity | ||
| self._state = bool(self._current_activity != 'PowerOff') | ||
| self.schedule_update_ha_state() | ||
| self.write_config_file() |
There was a problem hiding this comment.
This should be run in the executor. It seems we're mixing things that possibly should be run in the event loop with things that should run in the thread pool.
|
|
||
| self.hass.bus.async_listen_once(EVENT_HOMEASSISTANT_STOP, shutdown) | ||
|
|
||
| _LOGGER.debug("Connecting.") | ||
| await self._client.connect() | ||
| await self._client.get_config() | ||
| if not Path(self._config_path).is_file(): |
There was a problem hiding this comment.
This does I/O, so we have to run this in the executor.
Updated requirements
ehendrix23
changed the title
Small version bump increase for aioharmony
| @@ -142,17 +137,17 @@ class HarmonyRemote(remote.RemoteDevice): | |||
| def __init__(self, name, host, port, activity, out_path, delay_secs): | |||
| """Initialize HarmonyRemote class.""" | |||
| from aioharmony.harmonyapi import ( | |||
| HarmonyAPI as harmony_client, ClientCallbackType) | |||
|
|
|||
| HarmonyAPI as Harmony_Client, ClientCallbackType | |||
There was a problem hiding this comment.
Class names should use CapWords convention.
| await self._client.sync() | ||
| except aioexc.TimeOut: | ||
| _LOGGER.error("%s: Syncing hub with Harmony cloud timed-out", | ||
| self.name) | ||
| await self.hass.async_add_executor_job(self.write_config_file) |
There was a problem hiding this comment.
Should we write the config even after error?
There was a problem hiding this comment.
Good point, fixed. :-)
|
That would be great, I have a few more PRs open for this platform. Each one dependent on the other one. Been busy over the holidays :-) Hoping to get all of them in for 0.85 release . :-) |
|
Good! Can be merged when build passes. |
|
@MartinHjelmare, build passed. :-) |
|
So I just updated last night, and I'm getting errors in the logs that reference pyharmony... Is this expected? Also, I still have issues where i get the above everytime I do anything with my remote... I'm using HA in Docker on my Ubuntu server, if that matters. |
|
@steve28, that does not look right. Like 205 in remote/harmony.py is: Assuming you are on 0.85 now? |
|
Please do not use merged PRs for bug reports. Please open a new issue and discuss it there. |
|
@ehendrix23 yes, I am on 0.85 - anything you want me to check? I simply pulled the latest docker image |
|
@steve28, as mentioned by @marchingphoenix, please open an issue first so we can discuss there. |
|
Opened #19947 |
* Use aioharmony for async Use aioharmony to interact with Harmony hub. Due to this following improvements: -) Setting of available state for entity -) Automatic config update if configuration changes (including updating file containing config) -) Allow using of device name instead of number -) When sending command with repeat, nothing else will be able to put a IR command in between * Requirements updated * Version update for fix * Mainly cleanup * Update requirements Updated requirements * Fixed lint issue * Small bump for aioharmony Small version bump increase for aioharmony * Updated based on review
Description:
Use new aioharmony instead of pyharmony. This provides the following benefits:
Related issue (if applicable):
fixes #19465, fixes #19466
Pull request in home-assistant.io with documentation (if applicable): home-assistant/home-assistant.io#7982
Checklist:
tox. Your PR cannot be merged unless tests passIf the code communicates with devices, web services, or third-party tools:
REQUIREMENTSvariable ([example][ex-requir]).requirements_all.txtby runningscript/gen_requirements_all.py.