Skip to content

ISY994 migration to PyISY v2 (Structure Changes to enable upgrade, Part 1)#35212

Merged
bdraco merged 3 commits intohome-assistant:devfrom
shbatm:isy994_v2_pr1
May 5, 2020
Merged

ISY994 migration to PyISY v2 (Structure Changes to enable upgrade, Part 1)#35212
bdraco merged 3 commits intohome-assistant:devfrom
shbatm:isy994_v2_pr1

Conversation

@shbatm
Copy link
Copy Markdown
Contributor

@shbatm shbatm commented May 4, 2020

Breaking change

Sorting of certain devices based on the ISY's Node Def ID and Insteon Type properties have been corrected to match the ISY's provided device categories, as well as user feedback of incorrect sorting for specific devices. As a result, some entities that were incorrectly categorized will now appear under a different platform (e.g. switch to binary_sensor, light to switch, etc.).

The following device node types have changed platforms to correct their categorization.

  • "BinaryControl" (SWITCH->BINARY_SENSOR)
  • "BinaryControl_ADV" (SWITCH->BINARY_SENSOR; IOLinc Sensor)
  • "EZIO2x4_Input" (SWITCH->BINARY_SENSOR)
  • "EZRAIN_Input" (SWITCH->BINARY_SENSOR)
  • "OnOffControl" (SWITCH->BINARY_SENSOR)
  • "OnOffControl_ADV" (New; Thermostat Control/Running Sensors)
  • "EZIO2x4_Input_ADV" (SWITCH->SENSOR, Analog input on EZIO).
  • "RemoteLinc2" (LIGHT->SWITCH),
  • "RemoteLinc2_ADV" (LIGHT->SWITCH),
    • RemoteLincs only report button presses as events, are not controllable and do not accurately report dimmable states.
  • New Insteon Types for BINARY_SENSORS: "7.0.", "7.13." (IOLinc/EZIO Sensors)
    • IOLinc sensor/control logic will be updated in PR-4 to sort them correctly into two different platforms.
  • New Insteon Type for LOCKS: "4.64." added.
  • New Insteon Types for SWITCHES: "0.16.", "7.3.255.", "9.10."

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 first 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. A preliminary version of the migration plan has been discussed with codeowner @bdraco and PyISY maintainer @OverloadUT on the PyISY repo here and is included at the bottom of this description and is open for any comments that you'd like to see changed.

This specific PR includes the first of two sets of integration structure changes needed to prep for the new PyISY version and additional forthcoming features:

  • Update device platforms to be correct.
  • Move constants to separate file.
  • Consolidate Logging to Constants file.
  • Use Home Assistant Constants where possible.
  • Use string literals where possible.
  • Rename "domain" to "platform" in most places.
  • Add additional device support (NODE_FILTER updates).

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

Additional information

Current Migration and Upgrade Plan

Here's my plan for merging hacs-isy994 & PyISYv2 into Home Assistant Core. I have it broken up into 9 PRs to help with reviews.
The links are to the staged commits, and will be updated with PR numbers as progressed.

  1. Structure updates in prep for PyISYv2 Part 1 (this PR)
  2. Structure updates in prep for PyISYv2 Part 2 (https://github.com/shbatm/home-assistant/commit/f85c857f2aa60dd63dcdb1018fe4fdb9975f1de2).
    • Target no breaking changes in this PR.
    • Move core Entity Class definitions to separate file.
      • New ISYNodeEntity and ISYProgramEntity base classes.
      • Update inheriances on other classes.
    • Move helper functions to separate file.
  3. Basic support for PyISYv2 (https://github.com/shbatm/home-assistant/commit/6890de807f9d61c7672a6be40be904de694a7518).
    • Bare minimum changes to support PyISYv2.
    • Renaming imports and functions to new names.
    • Use necessary constants from module.
    • BC Remove ISY Climate Module support.
    • BC Device State Attribute fixes (using NodeProperty)
    • BC isy994_control changes (using NodeProperty)
    • BC Turn On command uses device on-level instead of full brightness (part of PyISYv2).
  4. Improved device sorting (incl Z-Wave Cats) post-PyISYv2 (https://github.com/shbatm/home-assistant/commit/d70dbb53099ec5015e323de391b2bcf88ff9847d).
    • State Unknown updates for binary_sensors.
    • Proper fix for heartbeat devices.
    • Better classification of binary_sensors.
    • Z-Wave device classification using new properties.
    • BC Some devices may change platforms based on new sorting methods.
  5. Add Config and Options Flow (https://github.com/shbatm/home-assistant/commit/a9c8ec641a7b7667d4d9fd94600e8a95d1b79b0c).
    • Add device registry support.
    • Add support for multiple ISYs.
    • Add option to use device on_level or last_brightness state when ON is called.
    • Add tests for config flow.
  6. Add Device Info (https://github.com/shbatm/home-assistant/commit/1345e01293c342070c53ad6fea696ad922c911c6).
  7. Add Climate Plaform (Thermostat) Support (https://github.com/shbatm/home-assistant/commit/d3e42b59f4f9992040cb43b432ebe2d9270605b5)
  8. Add support for Variables as Sensors (https://github.com/shbatm/home-assistant/commit/4fb4e7d7b96a358dfaa25f8a168dd7af8dbd8644)
  9. Add Services (https://github.com/shbatm/home-assistant/commit/fa8fc3f957238cc4ab4177d6bd2ef77fbcf205fd).

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

shbatm and others added 3 commits May 3, 2020 12:56
- **BREAKING CHANGE**: The following device node type have changed platforms to correct their categorization. Some entities will now show up under a different (correctted) platform (e.g. switch to binary_sensor, etc.).
    - New Binary Sensor Types:
        - "BinaryControl (moved from SWITCH)" ,
        - "BinaryControl_ADV" (moved from SWITCH; IOLinc Sensor),
        - "EZIO2x4_Input" (moved from SWITCH),
        - "EZRAIN_Input" (moved from SWITCH),
        - "OnOffControl" (moved from SWITCH),
        - "OnOffControl_ADV" (Added; Thermostat Running Sensors),
        - Insteon Types: "7.0.", "7.13." (IOLinc/EZIO Sensors)
        - IOLinc sensor/control logic will be updated in PR#4 to sort them correctly into two different platforms.
    - New Sensor Types:
        - "EZIO2x4_Input_ADV" (moved from SWITCH, ADC input on EZIO).
    - New Lock Types:
        - Insteon Type "4.64." added.
    - New Switch Types:
        - "DimmerSwitchOnly" (moved from LIGHT - on/off switches, non-dimmable),
        - "DimmerSwitchOnly_ADV" (moved from LIGHT - on/off switches, non-dimmable),
        - "RemoteLinc2" (moved from LIGHT - only report button presses as events, otherwise uncontrollable devices),
        - "RemoteLinc2_ADV" (moved from LIGHT - only report button presses as events, otherwise uncontrollable devices),
        - Insteon Types: "0.16.", "7.3.255.", "9.10." added based on user feedback.

- Move constants to separate file.
- Consolidate Logging to Constants file.
- Use Home Assistant Constants where possible.
- Use string literals where possible.
- Rename "domain" to "platform" in most places.
- Add additional device support (NODE_FILTER updates).
@probot-home-assistant
Copy link
Copy Markdown

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)

@bdraco
Copy link
Copy Markdown
Member

bdraco commented May 4, 2020

The breaking changes section is what we publish in the release notes so everything the user needs to know about should be in there.

Comment thread homeassistant/components/isy994/helpers.py Outdated
Comment thread homeassistant/components/isy994/helpers.py Outdated
@bdraco
Copy link
Copy Markdown
Member

bdraco commented May 4, 2020

Tested and verified all of my devices are still working. Nice cleanup!

@bdraco
Copy link
Copy Markdown
Member

bdraco commented May 4, 2020

The CI is likely going to find a few pyupgrade items (maybe not in this PR though)

Copy link
Copy Markdown
Member

@bdraco bdraco left a comment

Choose a reason for hiding this comment

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

Please rework the breaking changes section to be more user consumable, and small items with the programs code.

@shbatm
Copy link
Copy Markdown
Contributor Author

shbatm commented May 4, 2020

Please rework the breaking changes section to be more user consumable, and small items with the programs code.

Updated breaking changes section. Please take a look if that's satisfactory.

The CI is likely going to find a few pyupgrade items (maybe not in this PR though)

I think they show up in the later ones with more "new" code. I know it fixed a few things on pre-commit runs.

@bdraco
Copy link
Copy Markdown
Member

bdraco commented May 4, 2020

Please rework the breaking changes section to be more user consumable, and small items with the programs code.

Updated breaking changes section. Please take a look if that's satisfactory.

LGTM. Thanks for adjusting.

The CI is likely going to find a few pyupgrade items (maybe not in this PR though)

I think they show up in the later ones with more "new" code. I know it fixed a few things on pre-commit runs.

Excellent. I think I'm too used to doing reviews where the submitter does not have all the pre-commit hooks working. 👍

@bdraco
Copy link
Copy Markdown
Member

bdraco commented May 4, 2020

@shbatm
Thank you for your contribution thus far! 🎖 Since this is a significant contribution, we would appreciate you'd added yourself to the list of code owners for this integration. ❤️

Please, add your GitHub username to the manifest.json of this integration.

For more information about "code owners", see: Architecture Decision Record 0008: Code owners.

@shbatm
Copy link
Copy Markdown
Contributor Author

shbatm commented May 4, 2020

Updated code owners.

@bdraco
Copy link
Copy Markdown
Member

bdraco commented May 5, 2020

Doing some more manual testing and will merge if everything is ok

@bdraco
Copy link
Copy Markdown
Member

bdraco commented May 5, 2020

Motions sensors still working as expected (these are always the most problematic on isy)

@bdraco
Copy link
Copy Markdown
Member

bdraco commented May 5, 2020

Everything else looks good as well.

@bdraco bdraco merged commit 4be1006 into home-assistant:dev May 5, 2020
@shbatm shbatm deleted the isy994_v2_pr1 branch May 5, 2020 02:53
@lock lock Bot locked and limited conversation to collaborators May 6, 2020
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