Skip to content

Change Plugwise integration to plugwise module#43036

Merged
ctalkington merged 8 commits intohome-assistant:devfrom
plugwise:plugwise-module
Nov 21, 2020
Merged

Change Plugwise integration to plugwise module#43036
ctalkington merged 8 commits intohome-assistant:devfrom
plugwise:plugwise-module

Conversation

@CoMPaTech
Copy link
Copy Markdown
Member

@CoMPaTech CoMPaTech commented Nov 10, 2020

Proposed change

Final changes before we can draft the successor of #35713 - Some background to this PR. As requested in #35713 we joined forces but as a team decided to not only merge the HA-code but also our module! As such this PR swaps Plugwise_Smile for plugwise. To keep changes to a minimum this PR only adjusts the current integration for usage of the new module.

  • Changelog of the new module (including predecessors)
  • Dependency (as per above) switched from lxml to defusedxml (improving security)
  • Final plumbing/placeholders for USB integration
  • Adding new tests (exceptions changed due to the merged new module, upsetting codecov - so tests specifically for the renamed exceptions were added).
  • Adding @brefra to CODEOWNERS

For reviewers, to clarify why we use async_setup_entry as a wrapper in switch.py will default to the current workings unless PW_TYPE is set to USB. (Hence the placeholder stub).

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

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][dev-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:

  • Documentation added/updated for [www.home-assistant.io][docs-repository]

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

  • The [manifest file][manifest-docs] 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.

To help with the load of incoming pull requests:

  • I have reviewed two other [open pull requests][prs] in this repository.

@probot-home-assistant
Copy link
Copy Markdown

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

@CoMPaTech CoMPaTech changed the title Switch to plugwise module and forthcoming changes Change Plugwise integration module Nov 10, 2020
Copy link
Copy Markdown
Contributor

@bouwew bouwew left a comment

Choose a reason for hiding this comment

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

One small improvement possible.

Copy link
Copy Markdown
Contributor

@bouwew bouwew left a comment

Choose a reason for hiding this comment

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

Looking good now after the changes.

@CoMPaTech
Copy link
Copy Markdown
Member Author

Looking good now after the changes.

Only added tests to pass patch coverage - waiting for open PR to 'ready for review'

@CoMPaTech CoMPaTech marked this pull request as ready for review November 17, 2020 08:12
@ctalkington ctalkington self-requested a review November 17, 2020 16:19
@deltachonglee81
Copy link
Copy Markdown

deltachonglee81 commented Nov 18, 2020 via email

blocking=True,
)
state = hass.states.get("climate.zone_thermostat_jessie")
attrs = state.attributes
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

was it intended to assert something here?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ill go ahead and approve this, just double check this test in your usb branch or next PR.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@ctalkington will do in the usb-branch coming up

@ctalkington ctalkington merged commit db60a71 into home-assistant:dev Nov 21, 2020
@ctalkington ctalkington changed the title Change Plugwise integration module Change Plugwise integration to plugwise module Nov 21, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Nov 22, 2020
@CoMPaTech CoMPaTech deleted the plugwise-module branch March 13, 2022 18:59
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.

5 participants