Add support for Plugwise USB stick#44186
Conversation
probot-home-assistant
bot
added
has-tests
integration: plugwise
new-feature
by-code-owner
labels
|
Hey there @CoMPaTech, @bouwew, mind taking a look at this pull request as its been labeled with an integration ( |
CoMPaTech
left a comment
CoMPaTech
left a comment
There was a problem hiding this comment.
Looks good now - and most of this was in the previous PR as well as tested through our now also integrated custom_component by the beta-testers.
bouwew
left a comment
bouwew
left a comment
There was a problem hiding this comment.
I agree, looking good. And tested by me as well.
|
There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. |
| stick = await self.async_add_executor_job(plugwise.stick, device_path) | ||
| try: | ||
| await self.async_add_executor_job(stick.connect) | ||
| await self.async_add_executor_job(stick.initialize_stick) | ||
| await self.async_add_executor_job(stick.disconnect) | ||
| except PortError: | ||
| errors[CONF_BASE] = "cannot_connect" | ||
| except StickInitError: | ||
| errors[CONF_BASE] = "stick_init" | ||
| except NetworkDown: | ||
| errors[CONF_BASE] = "network_down" | ||
| except TimeoutException: | ||
| errors[CONF_BASE] = "network_timeout" | ||
| return errors, stick |
There was a problem hiding this comment.
Please avoid all the jumps into the executor by wrapping them up in one function.
There was a problem hiding this comment.
@bdraco, first a question: why should this be avoided?
And, combining them in one function is not practical: there are 4 functions, we call use 3 of them here, and in usb.py we use all four of them but in a different combination.
Also, this happens in config_flow.py and usb.py, the functions in these 2 files are only called during init and when removing the integration, how much harm can that cause?
There was a problem hiding this comment.
Switching from async to sync context is expensive. We should try to avoid doing it multiple times when we can do one call to the executor.
| _LOGGER.debug("Connect to USB-Stick") | ||
| await hass.async_add_executor_job(stick.connect) | ||
| _LOGGER.debug("Initialize USB-stick") | ||
| await hass.async_add_executor_job(stick.initialize_stick) | ||
| _LOGGER.debug("Discover Circle+ node") | ||
| await hass.async_add_executor_job(stick.initialize_circle_plus) |
There was a problem hiding this comment.
Please avoid multiple jumps into the executor. Wrap them into a single function.
| errors[CONF_BASE] = "already_configured" | ||
| return errors, None | ||
|
|
||
| stick = await self.async_add_executor_job(plugwise.stick, device_path) |
There was a problem hiding this comment.
Is it really necessary to call a class using async_add_executor_job()?
Maybe better to use from plugwise import stick
and then use something like api_stick = stick(device_path)?
5 participants
Proposed change
This PR is the successor of #35713 - adds support for legacy Plugwise Circle(+), and Stealth(+) devices.
These devices are controlled by using an USB-stick which communicates directly to the devices.
More details is written down in the related documentation PR.
We'll adhere to trying it small and per platform, so this PR only contains the
switchplatform to control the power relays.In future PR's the other platforms will be added to support multiple sensors (mainly) related to the power consumption and also the legacy Scan device (motion detection sensor)
Type of change
Example entry for
configuration.yaml:# Example configuration.yamlAdditional information
Checklist
black --fast homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all..coveragerc.The integration reached or maintains the following Integration Quality Scale:
To help with the load of incoming pull requests: