-
-
Notifications
You must be signed in to change notification settings - Fork 33k
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
Move Z-Wave JS smoke, CO, CO2, Heat, Water problem entities to diagnostic #129922
base: dev
Are you sure you want to change the base?
Conversation
Hey there @home-assistant/z-wave, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. We should add or adjust existing tests to check the entity category of these entities from some example device.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The outdated notifications.json
URL content is now defined in https://github.com/zwave-js/node-zwave-js/blob/de4a7cecf2848f332f6646bbc24b51d469e5056a/packages/core/src/registries/Notifications.ts
You can also find the definitions at https://github.com/zwave-js/specs/blob/master/Registries/Notification%20Command%20Class%2C%20list%20of%20assigned%20Notifications.xlsx.
@MartinHjelmare there doesn't seem to be any tests for these. |
Thanks for the updated link @kpine, I've updated the values and the URL in the comment. With this PR, the following notifications will be moved to diagnostic: Smoke Alarm
CO Alarm
CO2 Alarm
Heat Alarm
Water Alarm
|
Then please add new tests. Also please add the summary above as a comment somewhere in the module. |
@balloob if you can attach the diagnostics dump of your smoke detector device to a comment in this PR someone else can write tests for that device. |
There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. |
This still needs to be finished. |
Breaking change
Proposed change
My Z-Wave smoke/CO alarm has a ton of entities as primary sensors.
I believe that things like "End of life" should be marked as diagnostic. This PR adjusts this.
Updated thee notifications.json link to point at the new location (an Excel spreadsheet).
I've moved the following entities to the diagnostic category:
Thanks for the updated link @kpine, I've updated the values and the URL in the comment.
With this PR, the following notifications will be moved to diagnostic:
Smoke Alarm
CO Alarm
CO2 Alarm
Heat Alarm
Water Alarm
I've also updated the specification to no longer have "catch-all" entity descriptions that mark all other entities as device class
PROBLEM
. Those are now specifically mentioned. That way, we don't mark things having a problem incorrectly (like Heat Alarm Test). (I followed the rule, ifdevice_class
is specified, thestates
key also needs to be specified)Type of change
Additional information
Checklist
ruff format homeassistant tests
)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest
.requirements_all.txt
.Updated by running
python3 -m script.gen_requirements_all
.To help with the load of incoming pull requests: