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

Migrate ISY994 to PyISY v2 #35338

Merged
merged 6 commits into from
May 8, 2020
Merged

Migrate ISY994 to PyISY v2 #35338

merged 6 commits into from
May 8, 2020

Conversation

shbatm
Copy link
Contributor

@shbatm shbatm commented May 7, 2020

Breaking change

PyISY Version 2 is a significant update to the original module used to communicate with the ISY. As part of the update and many bug fixes, some breaking changes were introduced:

  • Remove ISY Climate Module support: the ISY Climate Module was retired by UDI on 3/30/2020: UDI Announcement, support has been removed from the module, so any entities based on Climate module nodes will no longer import into Home Assistant. The enable_climate configuration option will need be removed from your configuration.yaml file.
  • Device State Attributes have changed: some attributes' names and types will have changed as part of the changes to PyISY. If a user relied on a device state attribute for a given entity, they should check that it is still there and formatted the same. In general, more state attributes that were previously unavailable, should appear.
  • isy994_control events now return with additional information about the event. If a user relies on the control event property in Automations, these will need to be updated since the format has changed to include the additional detail.
  • Nodes that are "grouped" together in the ISY Admin Console will now be correctly identified and sorted, this will cause additional entities to be added to Home Assistant. If you were using this "group" feature to ignore some sub-devices in Home Assistant, you will now need to use the "ignore_string" in the name instead.
  • Turning on a light without providing a brightness value will use the ISY Device's On Level property instead of turning on to full brightness (if Home Assistant doesn't have a stored value for the last brightness).

Proposed change

PyISY version 2 has been released which supports significant back-end updates but is not backwards-compatible with the current v1.1.2.

This is the third PR in a series (9 total) to include migration to PyISYv2 and "modernization" of the isy994 integration based on testing done over the past year in the HACS custom component.

Full migration plan is captured in PR #35212

This specific PR includes the necessary changed to add basic support and compatibility for the update module:

  • Bare minimum changes to support PyISYv2.
  • Renaming imports and functions to new names.
  • Use available constants from the module.
  • Remove ISY Climate Module support.
  • Device State Attribute fixes (using NodeProperty)
  • isy994_control changes (using NodeProperty)
  • Turn On command uses device on-level instead of full brightness (part of PyISYv2).

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Example entry for configuration.yaml:

# Example configuration.yaml

# The following line will need to be removed:
enable_climate: true  

Additional information

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • The code has been formatted using Black (black --fast homeassistant tests)
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

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

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

  • No score or internal
  • 🥈 Silver
  • 🥇 Gold
  • 🏆 Platinum

Tag: @bdraco @OverloadUT

shbatm and others added 4 commits May 7, 2020 15:21
- Bare minimum changes to be able to support PyISYv2.
- Renaming imports and functions to new names.
- Use necessary constants from module.
- **BREAKING CHANGE** Remove ISY Climate Module support.
    - Climate module was retired on 3/30/2020: [UDI Annoucement](https://www.universal-devices.com/byebyeclimatemodule/)
- **BREAKING CHANGE** Device State Attributes use NodeProperty
    - Some attributes names and types will have changed as part of the changes to PyISY. If a user relied on a device state attribute for a given entity, they should check that it is still there and formatted the same. In general, *more* state attributes should be getting picked up now that the underlying changes have been made.
- **BREAKING CHANGE** `isy994_control` event changes (using NodeProperty)
    - Control events now return an object with additional information. Control events are now parsed to the friendly names and will need to be updated in automations.
Remove cast
@probot-home-assistant
Copy link

Hey there @bdraco, mind taking a look at this pull request as its been labeled with a integration (isy994) you are listed as a codeowner for? Thanks!
(message by CodeOwnersMention)

@MartinHjelmare MartinHjelmare changed the title ISY994 migration to PyISY v2 (Update PyISY to 2.0.2 and add basic compatibility) Migrate ISY994 to PyISY v2 May 7, 2020
Co-authored-by: J. Nick Koston <nick@koston.org>
@shbatm
Copy link
Contributor Author

shbatm commented May 7, 2020

Documentation link added in original description.

@bdraco
Copy link
Member

bdraco commented May 7, 2020

Doing testing now

@bdraco
Copy link
Member

bdraco commented May 7, 2020

2020-05-07 21:07:46 ERROR (MainThread) [homeassistant.components.binary_sensor] Error while setting up isy994 platform for binary_sensor
Traceback (most recent call last):
  File "/usr/src/homeassistant/homeassistant/helpers/entity_platform.py", line 178, in _async_setup_platform
    await asyncio.wait_for(asyncio.shield(task), SLOW_SETUP_MAX_WAIT)
  File "/usr/local/lib/python3.7/asyncio/tasks.py", line 442, in wait_for
    return fut.result()
  File "/usr/local/lib/python3.7/concurrent/futures/thread.py", line 57, in run
    result = self.fn(*self.args, **self.kwargs)
  File "/config/custom_components/isy994/binary_sensor.py", line 34, in setup_platform
    devices_by_nid[node.nid] = device
AttributeError: 'Node' object has no attribute 'nid'
2020-05-07 21:07:46 ERROR (MainThread) [homeassistant.components.switch] Error while setting up isy994 platform for switch
Traceback (most recent call last):
  File "/usr/src/homeassistant/homeassistant/helpers/entity_platform.py", line 178, in _async_setup_platform
    await asyncio.wait_for(asyncio.shield(task), SLOW_SETUP_MAX_WAIT)
  File "/usr/local/lib/python3.7/asyncio/tasks.py", line 442, in wait_for
    return fut.result()
  File "/usr/local/lib/python3.7/concurrent/futures/thread.py", line 57, in run
    result = self.fn(*self.args, **self.kwargs)
  File "/config/custom_components/isy994/switch.py", line 21, in setup_platform
    if not node.dimmable:
AttributeError: 'Group' object has no attribute 'dimmable'

@shbatm Looks like a few more the v2 conversion items need to make it into this PR

@shbatm
Copy link
Contributor Author

shbatm commented May 7, 2020

@bdraco Missed a test between 3 & 4.

Updated and confirmed tested on my end:

  • Loads without errors on my ISY.
  • Safely fails to load expected "test" entities with just warning to log.
  • EventStream connects and responds to local controls
  • Device state attributes work for programs and nodes.
  • Nodes, Groups, and Program based entities all controllable.
  • Motion sensor node loads.

@bdraco
Copy link
Member

bdraco commented May 8, 2020

Test failure on py37 isn't related. Ok to ignore as py38 passed

FAILED tests/components/automatic/test_device_tracker.py::test_valid_credentials[pyloop]

@bdraco
Copy link
Member

bdraco commented May 8, 2020

Everything appears to be working as expected

@bdraco
Copy link
Member

bdraco commented May 8, 2020

Ran though another test cycle. Everything still good to go.

@bdraco bdraco merged commit 4ec88b4 into home-assistant:dev May 8, 2020
@lock lock bot locked and limited conversation to collaborators May 21, 2020
@shbatm shbatm deleted the isy994_v2_pr3 branch December 21, 2022 00:38
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.

3 participants