8000 Use DeviceClass Enum in alarmdecoder schema by epenet · Pull Request #61967 · home-assistant/core · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Use DeviceClass Enum in alarmdecoder schema #61967

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jan 2, 2022
Merged

Use DeviceClass Enum in alarmdecoder schema #61967

merged 2 commits into from
Jan 2, 2022

Conversation

epenet
Copy link
Contributor
@epenet epenet commented Dec 15, 2021

Proposed change

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
  • 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.

The integration reached or maintains the following Integration Quality Scale:

  • No score or internal
  • 🥈 Silver
  • 🥇 Gold
  • 🏆 Platinum

To help with the load of incoming pull requests:

@probot-home-assistant
Copy link

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

@epenet epenet requested a review from farmio December 15, 2021 22:25
@ajschmidt8
Copy link
Contributor

@epenet, can you provide a description of these changes? i.e. What's the impact? Why were they made?

@epenet
Copy link
Contributor Author
epenet commented Dec 16, 2021

DEVICE_CLASSES is deprecated:

core/homeassistant/components/binary_sensor/__init__.py

Lines 118 to 122 in 574f0fc

# DEVICE_CLASS* below are deprecated as of 2021.12
# use the BinarySensorDeviceClass enum instead.
DEVICE_CLASSES = [cls.value for cls in BinarySensorDeviceClass]
DEVICE_CLASS_BATTERY = BinarySensorDeviceClass.BATTERY.value
DEVICE_CLASS_BATTERY_CHARGING = BinarySensorDeviceClass.BATTERY_CHARGING.value

@farmio
Copy link
Contributor
farmio commented Dec 16, 2021

@epenet I'm not sure how the flow form renders a StrEnum. The others were yaml so this didn't apply.

@epenet epenet marked this pull request as draft December 16, 2021 08:06
@epenet
Copy link
Contributor Author
epenet commented Dec 16, 2021

DEVICE_CLASSES is deprecated, so I think we need to find a solution to remove it's use.
@ajschmidt8 can you please confirm if the config flow renders the enums correctly?

@epenet epenet mentioned this pull request Dec 16, 2021
Merged
22 tasks
@ajschmidt8
Copy link
Contributor

@ajschmidt8 can you please confirm if the config flow renders the enums correctly?

@epenet, I just tested this locally and I can confirm that config flow does not render the enums correctly. Let me see if I can figure out a quick fix.

@ajschmidt8
Copy link
Contributor

The following changes seem to fix it locally. It essentially just inlines the previous DEVICE_CLASSES value which is apparently now deprecated.

diff --git a/homeassistant/components/alarmdecoder/config_flow.py b/homeassistant/components/alarmdecoder/config_flow.py
index 37ff5b9799..eab2850d07 100644
--- a/homeassistant/components/alarmdecoder/config_flow.py
+++ b/homeassistant/components/alarmdecoder/config_flow.py
@@ -7,9 +7,7 @@ from alarmdecoder.util import NoDeviceError
 import voluptuous as vol
 
 from homeassistant import config_entries
-from homeassistant.components.binary_sensor import (
-    DEVICE_CLASSES_SCHEMA as BINARY_SENSOR_DEVICE_CLASSES_SCHEMA,
-)
+from homeassistant.components.binary_sensor import BinarySensorDeviceClass
 from homeassistant.const import CONF_HOST, CONF_PORT, CONF_PROTOCOL
 from homeassistant.core import callback
 
@@ -250,7 +248,7 @@ class AlarmDecoderOptionsFlowHandler(config_entries.OptionsFlow):
                         default=existing_zone_settings.get(
                             CONF_ZONE_TYPE, DEFAULT_ZONE_TYPE
                         ),
-                    ): BINARY_SENSOR_DEVICE_CLASSES_SCHEMA,
+                    ): vol.In([cls.value for cls in BinarySensorDeviceClass]),
                     vol.Optional(
                         CONF_ZONE_RFID,
                         description={

@epenet
Copy link
Contributor Author
epenet commented Dec 20, 2021

Hi

I have just rebased my branch on latest dev.
I think balloob fixed the original issue via #62363 so my original change should work now.

Could you please try again?

@ajschmidt8
Copy link
Contributor

@epenet, yup. that fixed it. confirmed working now.

ajschmidt8
ajschmidt8 approved these changes Dec 20, 2021
View reviewed changes
Copy link
Contributor
@ajschmidt8 ajschmidt8 left a comment

Choose a reason for hiding this comment

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

Confirmed working locally.

@epenet epenet marked this pull request as ready for review December 20, 2021 18:23
davet2001
davet2001 approved these changes Jan 2, 2022
View reviewed changes
Copy link
Contributor
@davet2001 davet2001 left a comment

Choose a reason for hiding this comment

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

Looks good to me

@frenck frenck merged commit 631e4c6 into home-assistant:dev Jan 2, 2022
@epenet epenet deleted the enum-alarmdecoder branch January 2, 2022 15:18
@github-actions github-actions bot locked and limited conversation to collaborators Jan 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@frenck frenck frenck approved these changes

@ajschmidt8 ajschmidt8 ajschmidt8 approved these changes

@davet2001 davet2001 davet2001 approved these changes

@farmio farmio Awaiting requested review from farmio

Assignees
No one assigned
Labels
cla-signed code-quality integration: alarmdecoder small-pr PRs with less than 30 lines.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@epenet @ajschmidt8 @farmio @frenck @davet2001 @homeassistant
0