Add notification binary_sensor to Plugwise integration by CoMPaTech · Pull Request #41473 · home-assistant/core

CoMPaTech · 2020-10-08T08:02:06Z

Proposed change

Adding notifications (warnings/errors) from the Plugwise Smile to Home Assistant.
Both as notification for the persistent notifications handling and as binary_sensor to indicate notification present.

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

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
[ x 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 · 2020-10-08T08:02:16Z

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)}}

bouwew

Looking good!

CoMPaTech · 2020-10-15T21:42:01Z

Catching up with dev and as per failing 48f6987 and 00ecb01 (codecov-patch) asked for advise (tnx @derekatkins and @bdraco) and found out we did bump our module version but not the forthcoming tests in #41550. These were needed for the notifications to be eligible (it obviously turned out while actually testing the patch).

Code should now appropriately run tests on the patch (i.e. delete a notification and find one present from fixtures).

homeassistant/components/plugwise/binary_sensor.py

CoMPaTech · 2020-10-16T18:12:06Z

Should be fine for final review and eventual merge

CoMPaTech · 2020-11-01T14:48:29Z

Catchup with dev - still waiting eagerly for review. (Our backlog of changes is building waiting for these PRs to be merged, among them the requested integration closing #35713).

…luding notifications

tests/components/plugwise/test_init.py

CoMPaTech · 2020-11-10T19:33:28Z

@ctalkington anything else we should do to it or just waiting for after beta-cutoff? (no rush, just checking, so we can shift our focus to further drafting #43036 and working on the USB-one in advance)

ctalkington · 2020-11-13T02:22:27Z

@CoMPaTech just havent had as much time lately, ill try to wrap up review tonight or this weekend at latest

homeassistant/components/plugwise/binary_sensor.py

tests/components/plugwise/test_init.py

Co-authored-by: Martin Hjelmare <marhje52@gmail.com>

homeassistant/components/plugwise/bla

ctalkington · 2020-11-15T20:32:09Z

homeassistant/components/plugwise/binary_sensor.py

+                    if msg_type not in SEVERITIES:
+                        msg_type = "other"
+
+                    self._attributes[f"{msg_type.lower()}_msg"] = msg


is it possible for there to be more than one of a type at a time? this logic seems like it would overwrite and just show last msg of each type.

I don't think it gives more than one (and/or of each) - @bouwew can you check?

@ctalkington discussed offline and we've decided to ensure it wouldn't override, fb8a18f incorporates this

homeassistant/components/plugwise/binary_sensor.py

Co-authored-by: Martin Hjelmare <marhje52@gmail.com>

ctalkington

thanks for the followup during review!

* 'dev' of https://github.com/home-assistant/core: (77 commits) Fix kodi media_player unavailable at start (home-assistant#41714) Add an option to template delay_on/off in template binary sensor (home-assistant#43259) Bump hatasmota to 0.0.31 (home-assistant#43319) Update cloud integration to 0.38.0 (home-assistant#43314) Add progress translation key to hassfest (home-assistant#43311) Bump codecov/codecov-action from v1.0.14 to v1.0.15 (home-assistant#43304) Improvement to allow parsing of station ID in vasttrafik integration. Addresses home-assistant#34851 (home-assistant#43136) Abort vizio discovery flow without unique ID (home-assistant#43303) Update directv to 0.4.0 (home-assistant#43302) Add notification binary_sensor to Plugwise integration (home-assistant#41473) [ci skip] Translation update Bump bimmer_connected to 0.7.13 (home-assistant#43294) Bump aioguardian to 1.0.4 (home-assistant#43299) Refactor how entities are created for homekit_controller services (home-assistant#43242) Updated frontend to 20201111.1 (home-assistant#43298) Update pytradfri to 7.0.4 (home-assistant#43297) Remove pts adjustments in stream (home-assistant#42399) Fix Enigma2 available entity property (home-assistant#43292) Make MQTT climate return PRESET_NONE when no preset is set (home-assistant#43257) Bump env_canada to 0.2.4, fix config validation (home-assistant#43251) ...

deltachonglee81

@on

CoMPaTech · 2020-11-18T05:57:23Z

@on

?

probot-home-assistant bot added the Hacktoberfest label Oct 8, 2020

homeassistant added the cla-signed label Oct 8, 2020

probot-home-assistant bot added integration: plugwise new-feature by-code-owner labels Oct 8, 2020

CoMPaTech marked this pull request as ready for review October 8, 2020 08:20

bouwew approved these changes Oct 8, 2020

View reviewed changes

CoMPaTech force-pushed the plugwise-notifications branch 2 times, most recently from 9c03e92 to 1681255 Compare October 15, 2020 21:04

bouwew reviewed Oct 16, 2020

View reviewed changes

homeassistant/components/plugwise/binary_sensor.py Outdated Show resolved Hide resolved

MartinHjelmare added the hacktoberfest-accepted label Oct 31, 2020

CoMPaTech force-pushed the plugwise-notifications branch from 7a962f5 to a0cc9d1 Compare November 1, 2020 14:26

CoMPaTech added 12 commits November 8, 2020 19:12

Notifications extract from beta

10a31a9

Remove info loggings

676c0b2

Delete notification service

bc55916

Only notifications for right smiles

a30ec02

Revert to correct logic

ea9a0be

Catchup with dev (mostly with ourselves from home-assistant#41201)

5f41283

Remove debug logging

79f5ad9

Naming improvement

d7ac320

Improve test quality as per codecov patch requirement

d2ee748

Revert to original condition (and appropriately test)

d2dd7e0

Fix delete_notification_service, bring tests fixtures up to 1.6.0 inc…

5d7eac4

…luding notifications

Review comment (@bouwew)

dc3ca29

CoMPaTech force-pushed the plugwise-notifications branch from a0cc9d1 to dc3ca29 Compare November 8, 2020 18:18

Correct test value

ab4d307

ctalkington reviewed Nov 8, 2020

View reviewed changes

tests/components/plugwise/test_init.py Outdated Show resolved Hide resolved

ctalkington reviewed Nov 8, 2020

View reviewed changes

tests/components/plugwise/test_init.py Outdated Show resolved Hide resolved

CoMPaTech added 2 commits November 8, 2020 20:28

Remove separate gw unload code

a8c8e8a

Change entry_fail approach

b79dc91

CoMPaTech requested a review from ctalkington November 12, 2020 15:10

ctalkington reviewed Nov 13, 2020

View reviewed changes

homeassistant/components/plugwise/binary_sensor.py Outdated Show resolved Hide resolved

CoMPaTech added 2 commits November 13, 2020 19:52

Revert persistent notification part

6fbd885

Revert persistent notification part - lint

6d41832

CoMPaTech changed the title ~~Add notifications to Plugwise integration~~ Add notification binary_sensor to Plugwise integration Nov 13, 2020

MartinHjelmare requested changes Nov 15, 2020

View reviewed changes

CoMPaTech and others added 4 commits November 15, 2020 13:32

Update homeassistant/components/plugwise/binary_sensor.py

fb90ad6

Co-authored-by: Martin Hjelmare <marhje52@gmail.com>

Update homeassistant/components/plugwise/binary_sensor.py

d773f53

Co-authored-by: Martin Hjelmare <marhje52@gmail.com>

Rework reuse of sensor in binary_sensor

5a5d32f

Explicit state attribute keys

e31f106

ctalkington reviewed Nov 15, 2020

View reviewed changes

homeassistant/components/plugwise/bla Outdated Show resolved Hide resolved

Remove tempfile

642320f

ctalkington reviewed Nov 15, 2020

View reviewed changes

List of notifications per severity

fb8a18f

MartinHjelmare reviewed Nov 16, 2020

View reviewed changes

homeassistant/components/plugwise/binary_sensor.py Outdated Show resolved Hide resolved

homeassistant/components/plugwise/binary_sensor.py Show resolved Hide resolved

Update homeassistant/components/plugwise/binary_sensor.py

5be8a99

Co-authored-by: Martin Hjelmare <marhje52@gmail.com>

MartinHjelmare approved these changes Nov 16, 2020

View reviewed changes

ctalkington approved these changes Nov 17, 2020

View reviewed changes

ctalkington merged commit ed36cb7 into home-assistant:dev Nov 17, 2020

deltachonglee81 reviewed Nov 18, 2020

View reviewed changes

home-assistant locked as resolved and limited conversation to collaborators Nov 18, 2020

CoMPaTech deleted the plugwise-notifications branch March 13, 2022 18:59

Uh oh!

Conversation

CoMPaTech commented Oct 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed change

Type of change

Additional information

Checklist

Uh oh!

probot-home-assistant bot commented Oct 8, 2020

Uh oh!

bouwew left a comment

Choose a reason for hiding this comment

Uh oh!

CoMPaTech commented Oct 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

CoMPaTech commented Oct 16, 2020

Uh oh!

CoMPaTech commented Nov 1, 2020

Uh oh!

Uh oh!

Uh oh!

CoMPaTech commented Nov 10, 2020

Uh oh!

ctalkington commented Nov 13, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ctalkington Nov 15, 2020

Choose a reason for hiding this comment

Uh oh!

CoMPaTech Nov 15, 2020

Choose a reason for hiding this comment

Uh oh!

CoMPaTech Nov 16, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ctalkington left a comment

Choose a reason for hiding this comment

Uh oh!

deltachonglee81 left a comment

Choose a reason for hiding this comment

Uh oh!

CoMPaTech commented Nov 18, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

CoMPaTech commented Oct 8, 2020 •

edited

Loading

CoMPaTech commented Oct 15, 2020 •

edited

Loading