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
Check supported features in calls to vacuum services #95833
Conversation
Hey there @home-assistant/core, 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.
LGTM,
Thnx @emontnemery 👍
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.
Thanks!
We should do a developer blog too for custom integrations. Go ahead and merge when all affected core integrations are handled. |
I have reviewed and double reviewed the core integrations, and I believe they all set the feature flags correctly 👍 |
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.
try to clear up confusion deprecated VacuumEntity and StateVacuumEntity
specially for defines what are the vacuums supporting
@@ -77,19 +77,19 @@ | |||
class VacuumEntityFeature(IntFlag): | |||
"""Supported features of the vacuum entity.""" | |||
|
|||
TURN_ON = 1 | |||
TURN_OFF = 2 | |||
TURN_ON = 1 # Deprecated, not supported by StateVacuumEntity |
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.
why not remove it to avoid confusion?
there are only 3 lefts which are not StateVacuumEntity) if i count right:
StateVacuumEntity
demo
litterrobot
mqtt
neato
roborock
romy
roomba
sharkiq
template
tuya
xiaommmmi_miio
VacuumEntity (wich is deprecated)
demo
mqtt
ecovacs
cause demo does not count and mqtt is generic, only ecovacs will be hard to test after porting?
at least i would mark the demo and the class VacuumEntity
personal i would prefer removing VacuumEntity completly don't know side effects/effort here but it would help a lot for new vacuum implementation that they don't get confused
SEND_COMMAND = 256 | ||
LOCATE = 512 | ||
CLEAN_SPOT = 1024 | ||
MAP = 2048 | ||
STATE = 4096 | ||
STATE = 4096 # Must be set by vacuum platforms derived from StateVacuumEntity | ||
START = 8192 | ||
|
||
|
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.
can we remove this? cause the deprecated uses of VacuumEntity (demo, mqtt, ecovacs) uses the wrong define -> VacuumEntityFeature.TURN_ON anyhow
SUPPORT_TURN_ON:
is not used at all
SUPPORT_TURN_OFF:
is used only in test/components/alexa we should remove it since its for new StateVacuumEntity deprecated anyhow
SUPPORT_PAUSE:
used in test/components/alexa/test_smart_home.py -> we should change it to enum VacuumEntityFeature.PAUSE
used in test/components/google/test_trait.py -> we should change it to enum VacuumEntityFeature.PAUSE
SUPPORT_STOP:
used in test/components/alexa/test_smart_home.py -> we should change it to enum VacuumEntityFeature.SUPPORT_STOP
used in test/components/google/test_trait.py -> we should change it to enum VacuumEntityFeature.SUPPORT_STOP
SUPPORT_RETURN_HOME
used in test/components/alexa/test_smart_home.py -> we should change it to enum VacuumEntityFeature.RETURN_HOME
used in test/components/homekit/test_get_accessories.py -> we should change it to enum VacuumEntityFeature.RETURN_HOME
SUPPORT_FAN_SPEED
used in test/components/alexa/test_smart_home.py -> we should change it to enum VacuumEntityFeature.FAN_SPEED
SUPPORT_BATTERY
used in components/google_assistant/traits.py -> we should change it to enum VacuumEntityFeature.SUPPORT_BATTERY
used in test/components/google/test_trait.py -> we should change it to enum VacuumEntityFeature.SUPPORT_BATTERY
SUPPORT_STATUS -> not used at all
SUPPORT_SEND_COMMAND -> not used at all
SUPPORT_LOCATE
used in components/google_assistant/traits.py -> we should change it to enum VacuumEntityFeature.SUPPORT_LOCATE
used in test/components/google/test_trait.py -> we should change it to enum VacuumEntityFeature.SUPPORT_LOCATE
SUPPORT_CLEAN_SPOT -> not used at all
SUPPORT_MAP -> not used at all
SUPPORT_STATE -> not used at all
SUPPORT_START
used in test/components/alexa/test_smart_home.py -> we should change it to enum VacuumEntityFeature.START
used in test/components/homekit/test_get_accessories.py -> we should change it to enum VacuumEntityFeature.SUPPORT_START
can prepare a merge if you like?
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.
Sure, please prepare a PR cleaning up the use of the deprecated constants
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.
created: #96202
Proposed change
Check supported features in calls to vacuum services
Type of change
Additional information
Checklist
black --fast 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
..coveragerc
.To help with the load of incoming pull requests: