Skip to content

Consolidate device info and clean-up ISY994 code base#85657

Merged
bdraco merged 13 commits into
home-assistant:devfrom
shbatm:isy994_code_cleanup2
Jan 12, 2023
Merged

Consolidate device info and clean-up ISY994 code base#85657
bdraco merged 13 commits into
home-assistant:devfrom
shbatm:isy994_code_cleanup2

Conversation

@shbatm
Copy link
Copy Markdown
Contributor

@shbatm shbatm commented Jan 10, 2023

Proposed change

PR accomplishes several items centered around cleanup of the ISY994 codebase--there should be no change in end-user functionality for this change, only backend cleanup and efficiencies.

  1. Bump PyISY to 3.1.3 and code updates to use new helpers ... automicus/PyISY@v3.0.12...v3.1.3
  2. Generate DeviceInfo on config entry loading and store in a dict.
  3. Additional generalization to change ISY994_ string literals to ISY_ (follow-up to Update ISY994 integration to be model agnostic #85017).
  4. Remove import DOMAIN as ISY994_DOMAIN in favor of import DOMAIN (follow-up Use Platform enum and remove DOMAIN as X imports in ISY994 #85341)
  5. Cleanup of hass.data["isy994"] dict organization to be more consistent, store root/parent nodes, and prep for adding enable swtiches.
  6. Fix function to generate expected unique_ids and fix broken cleanup_entities service function.
  7. Use correct parent device unique_ids for async_remove_config_entry_device
  8. Remove unnecessary overrides on entity properties and use base _attr during init.
  9. Consolidate button entity classes.

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)
  • Deprecation (breaking change to happen in the future)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue:
  • Link to documentation pull request:

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.
  • For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
  • Untested files have been added to .coveragerc.

To help with the load of incoming pull requests:

@home-assistant
Copy link
Copy Markdown
Contributor

Hey there @bieniu, mind taking a look at this pull request as it has been labeled with an integration (gios) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of gios can trigger bot actions by commenting:

  • @home-assistant close Closes the issue.
  • @home-assistant rename Awesome new title Change the title of the issue.
  • @home-assistant reopen Reopen the issue.
  • @home-assistant unassign gios Removes the current integration label and assignees on the issue, add the integration domain after the command.

@home-assistant
Copy link
Copy Markdown
Contributor

Hey there @bdraco, mind taking a look at this pull request as it has been labeled with an integration (isy994) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of isy994 can trigger bot actions by commenting:

  • @home-assistant close Closes the issue.
  • @home-assistant rename Awesome new title Change the title of the issue.
  • @home-assistant reopen Reopen the issue.
  • @home-assistant unassign isy994 Removes the current integration label and assignees on the issue, add the integration domain after the command.

Comment thread homeassistant/components/gios/sensor.py
@shbatm shbatm force-pushed the isy994_code_cleanup2 branch from d9fb756 to a2f29ce Compare January 10, 2023 22:59
@shbatm shbatm marked this pull request as ready for review January 11, 2023 00:44
@shbatm
Copy link
Copy Markdown
Contributor Author

shbatm commented Jan 11, 2023

@home-assistant unassign gios

@shbatm
Copy link
Copy Markdown
Contributor Author

shbatm commented Jan 11, 2023

@bdraco Ready for you.
@bieniu, sorry, not sure why you got dragged into this--you can unassign.

Comment thread homeassistant/components/isy994/__init__.py
Comment thread homeassistant/components/isy994/button.py
Comment thread homeassistant/components/isy994/const.py
Comment thread homeassistant/components/isy994/const.py
Comment thread homeassistant/components/isy994/entity.py Outdated
Comment thread homeassistant/components/isy994/util.py
@shbatm
Copy link
Copy Markdown
Contributor Author

shbatm commented Jan 11, 2023

@bdraco you can pull the new-platform label too--there shouldn't be any end-user changes with this PR.

Comment thread homeassistant/components/isy994/services.py
Comment thread homeassistant/components/isy994/entity.py Outdated
@shbatm
Copy link
Copy Markdown
Contributor Author

shbatm commented Jan 12, 2023

Tested to make sure no devices left behind:

This branch: 03920c2 (#85657)

image

Dev @ 3ee73f0:

image

Prod @ 2023.1.1 (no variable or network entities):

image

Comment on lines +141 to +146
hass_isy_data[ISY_NODES] = {p: [] for p in (NODE_PLATFORMS + [SENSOR_AUX])}
hass_isy_data[ISY_ROOT_NODES] = {p: [] for p in ROOT_NODE_PLATFORMS}
hass_isy_data[ISY_PROGRAMS] = {p: [] for p in PROGRAM_PLATFORMS}
hass_isy_data[ISY_VARIABLES] = {p: [] for p in VARIABLE_PLATFORMS}
hass_isy_data[ISY_NET_RES] = []
hass_isy_data[ISY_DEVICES] = {}
Copy link
Copy Markdown
Member

@bdraco bdraco Jan 12, 2023

Choose a reason for hiding this comment

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

For a future PR: This might be a bit easier to manage as a top level dataclass instead of a dict so you can use named attributes instead. lookin has an example of this in models

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@bdraco Good call. I may go ahead and do that before I push the next PR for on level sensors.

Thanks for reviewing all these. I have a few more on deck coming your way while I have time.

@bdraco
Copy link
Copy Markdown
Member

bdraco commented Jan 12, 2023

codecov/config flow is unrelated to this PR (there are no config flow changes here)

@bdraco bdraco merged commit 255a836 into home-assistant:dev Jan 12, 2023
Comment thread homeassistant/components/isy994/__init__.py
@shbatm shbatm deleted the isy994_code_cleanup2 branch January 12, 2023 16:14
@github-actions github-actions Bot locked and limited conversation to collaborators Jan 13, 2023
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.

4 participants