Skip to content

Add valve platform support to google_assistant #106139

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

Merged
jbouwh merged 2 commits into from
Dec 23, 2023
Merged

Add valve platform support to google_assistant #106139

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
19d1d14
Add valve platform to google_assistant
jbouwh Dec 20, 2023
b84c5cf
Use constant for domains set
jbouwh Dec 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions homeassistant/components/google_assistant/const.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
sensor,
switch,
vacuum,
valve,
water_heater,
)

Expand Down Expand Up @@ -65,6 +66,7 @@
"sensor",
"switch",
"vacuum",
"valve",
"water_heater",
]

Expand Down Expand Up @@ -95,6 +97,7 @@
TYPE_TV = f"{PREFIX_TYPES}TV"
TYPE_WINDOW = f"{PREFIX_TYPES}WINDOW"
TYPE_VACUUM = f"{PREFIX_TYPES}VACUUM"
TYPE_VALVE = f"{PREFIX_TYPES}VALVE"
TYPE_WATERHEATER = f"{PREFIX_TYPES}WATERHEATER"

SERVICE_REQUEST_SYNC = "request_sync"
Expand Down Expand Up @@ -150,6 +153,7 @@
sensor.DOMAIN: TYPE_SENSOR,
switch.DOMAIN: TYPE_SWITCH,
vacuum.DOMAIN: TYPE_VACUUM,
valve.DOMAIN: TYPE_VALVE,
water_heater.DOMAIN: TYPE_WATERHEATER,
}

Expand Down
126 changes: 102 additions & 24 deletions homeassistant/components/google_assistant/trait.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
sensor,
switch,
vacuum,
valve,
water_heater,
)
from homeassistant.components.alarm_control_panel import AlarmControlPanelEntityFeature
Expand All @@ -41,6 +42,7 @@
from homeassistant.components.lock import STATE_JAMMED, STATE_UNLOCKING
from homeassistant.components.media_player import MediaPlayerEntityFeature, MediaType
from homeassistant.components.vacuum import VacuumEntityFeature
from homeassistant.components.valve import ValveEntityFeature
from homeassistant.components.water_heater import WaterHeaterEntityFeature
from homeassistant.const import (
ATTR_ASSUMED_STATE,
Expand Down Expand Up @@ -180,6 +182,57 @@

FAN_SPEED_MAX_SPEED_COUNT = 5

COVER_VALVE_STATES = {
cover.DOMAIN: {
"closed": cover.STATE_CLOSED,
"closing": cover.STATE_CLOSING,
"open": cover.STATE_OPEN,
"opening": cover.STATE_OPENING,
},
valve.DOMAIN: {
"closed": valve.STATE_CLOSED,
"closing": valve.STATE_CLOSING,
"open": valve.STATE_OPEN,
"opening": valve.STATE_OPENING,
},
}

SERVICE_STOP_COVER_VALVE = {
cover.DOMAIN: cover.SERVICE_STOP_COVER,
valve.DOMAIN: valve.SERVICE_STOP_VALVE,
}
SERVICE_OPEN_COVER_VALVE = {
cover.DOMAIN: cover.SERVICE_OPEN_COVER,
valve.DOMAIN: valve.SERVICE_OPEN_VALVE,
}
SERVICE_CLOSE_COVER_VALVE = {
cover.DOMAIN: cover.SERVICE_CLOSE_COVER,
valve.DOMAIN: valve.SERVICE_CLOSE_VALVE,
}
SERVICE_SET_POSITION_COVER_VALVE = {
cover.DOMAIN: cover.SERVICE_SET_COVER_POSITION,
valve.DOMAIN: valve.SERVICE_SET_VALVE_POSITION,
}

COVER_VALVE_CURRENT_POSITION = {
cover.DOMAIN: cover.ATTR_CURRENT_POSITION,
valve.DOMAIN: valve.ATTR_CURRENT_POSITION,
}

COVER_VALVE_POSITION = {
cover.DOMAIN: cover.ATTR_POSITION,
valve.DOMAIN: valve.ATTR_POSITION,
}

COVER_VALVE_SET_POSITION_FEATURE = {
cover.DOMAIN: CoverEntityFeature.SET_POSITION,
valve.DOMAIN: ValveEntityFeature.SET_POSITION,
}

COVER_VALVE_DOMAINS = {cover.DOMAIN, valve.DOMAIN}

FRIENDLY_DOMAIN = {cover.DOMAIN: "Cover", valve.DOMAIN: "Valve"}

_TraitT = TypeVar("_TraitT", bound="_Trait")


Expand Down Expand Up @@ -796,6 +849,9 @@ def supported(domain, features, device_class, _):
if domain == cover.DOMAIN and features & CoverEntityFeature.STOP:
return True

if domain == valve.DOMAIN and features & ValveEntityFeature.STOP:
return True

return False

def sync_attributes(self):
Expand All @@ -807,7 +863,7 @@ def sync_attributes(self):
& VacuumEntityFeature.PAUSE
!= 0
}
if domain == cover.DOMAIN:
if domain in COVER_VALVE_DOMAINS:
return {}

def query_attributes(self):
Expand All @@ -823,14 +879,16 @@ def query_attributes(self):

if domain == cover.DOMAIN:
return {"isRunning": state in (cover.STATE_CLOSING, cover.STATE_OPENING)}
if domain == valve.DOMAIN:
return {"isRunning": True}
Comment on lines +882 to +883
Copy link
Copy Markdown
Contributor Author

@jbouwh jbouwh Dec 20, 2023

Choose a reason for hiding this comment

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

We return True, so the state is always running and the Stop function will be always available (if supported)

Copy link
Copy Markdown
Contributor

@emontnemery emontnemery Dec 22, 2023

Choose a reason for hiding this comment

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

Hm, why don't we rely on CLOSING + OPENING states like we do for cover?

Copy link
Copy Markdown
Contributor Author

@jbouwh jbouwh Dec 22, 2023

Choose a reason for hiding this comment

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

The reason is that not all valves support that, but they can support the stop function. Which should be always available and not depend on the reported state.
Futher the Start button, that will pop up instead will not make any sense, hence I made this button to always show Stop.

To avoid this:

Copy link
Copy Markdown
Contributor

@rokam rokam Dec 22, 2023

Choose a reason for hiding this comment

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

I think that should be a UX fix. Replace Start button for open/close depending on the state. And isRunning uses the same logic as the cover.

Copy link
Copy Markdown
Contributor Author

@jbouwh jbouwh Dec 22, 2023

Choose a reason for hiding this comment

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

This is not something we can easily customize. It is how the start stop treat works. The integration can decide if a stop command should be processed.

Copy link
Copy Markdown
Contributor Author

@jbouwh jbouwh Dec 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

BTW. the cover implentation ideed has the same issue:

Copy link
Copy Markdown
Contributor Author

@jbouwh jbouwh Dec 23, 2023

Choose a reason for hiding this comment

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

I suggest I open a separate PR to fix this for cover as well.

Copy link
Copy Markdown
Contributor Author

@jbouwh jbouwh Dec 25, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Another suggestion is to use Start to toggle the valve (or cover) #106378

Comment thread
jbouwh marked this conversation as resolved.

async def execute(self, command, data, params, challenge):
"""Execute a StartStop command."""
domain = self.state.domain
if domain == vacuum.DOMAIN:
return await self._execute_vacuum(command, data, params, challenge)
if domain == cover.DOMAIN:
return await self._execute_cover(command, data, params, challenge)
if domain in COVER_VALVE_DOMAINS:
return await self._execute_cover_or_valve(command, data, params, challenge)

async def _execute_vacuum(self, command, data, params, challenge):
"""Execute a StartStop command."""
Expand Down Expand Up @@ -869,28 +927,35 @@ async def _execute_vacuum(self, command, data, params, challenge):
context=data.context,
)

async def _execute_cover(self, command, data, params, challenge):
async def _execute_cover_or_valve(self, command, data, params, challenge):
"""Execute a StartStop command."""
domain = self.state.domain
if command == COMMAND_STARTSTOP:
if params["start"] is False:
if self.state.state in (
cover.STATE_CLOSING,
cover.STATE_OPENING,
) or self.state.attributes.get(ATTR_ASSUMED_STATE):
if (
self.state.state
in (
COVER_VALVE_STATES[domain]["closing"],
COVER_VALVE_STATES[domain]["opening"],
)
or domain == valve.DOMAIN
Copy link
Copy Markdown
Contributor Author

@jbouwh jbouwh Dec 20, 2023

Choose a reason for hiding this comment

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

We want the stop function to be always available since we can not read determine from, the state if stop should be available,

Copy link
Copy Markdown
Contributor

@emontnemery emontnemery Dec 22, 2023

Choose a reason for hiding this comment

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

Why is that?

Copy link
Copy Markdown
Contributor Author

@jbouwh jbouwh Dec 22, 2023

Choose a reason for hiding this comment

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

It will ensure that the Stop button is shown, not the Start button. See also the comment below.

or self.state.attributes.get(ATTR_ASSUMED_STATE)
):
await self.hass.services.async_call(
self.state.domain,
cover.SERVICE_STOP_COVER,
domain,
SERVICE_STOP_COVER_VALVE[domain],
{ATTR_ENTITY_ID: self.state.entity_id},
blocking=not self.config.should_report_state,
context=data.context,
)
else:
raise SmartHomeError(
ERR_ALREADY_STOPPED, "Cover is already stopped"
ERR_ALREADY_STOPPED,
f"{FRIENDLY_DOMAIN[domain]} is already stopped",
)
else:
raise SmartHomeError(
ERR_NOT_SUPPORTED, "Starting a cover is not supported"
ERR_NOT_SUPPORTED, f"Starting a {domain} is not supported"
)
else:
raise SmartHomeError(
Expand Down Expand Up @@ -2081,7 +2146,7 @@ class OpenCloseTrait(_Trait):
@staticmethod
def supported(domain, features, device_class, _):
"""Test if state is supported."""
if domain == cover.DOMAIN:
if domain in COVER_VALVE_DOMAINS:
return True

return domain == binary_sensor.DOMAIN and device_class in (
Expand Down Expand Up @@ -2116,6 +2181,17 @@ def sync_attributes(self):
and features & CoverEntityFeature.CLOSE == 0
):
response["queryOnlyOpenClose"] = True
elif (
self.state.domain == valve.DOMAIN
and features & ValveEntityFeature.SET_POSITION == 0
):
response["discreteOnlyOpenClose"] = True

if (
features & ValveEntityFeature.OPEN == 0
and features & ValveEntityFeature.CLOSE == 0
):
response["queryOnlyOpenClose"] = True

if self.state.attributes.get(ATTR_ASSUMED_STATE):
response["commandOnlyOpenClose"] = True
Expand All @@ -2134,17 +2210,17 @@ def query_attributes(self):
if self.state.attributes.get(ATTR_ASSUMED_STATE):
return response

if domain == cover.DOMAIN:
if domain in COVER_VALVE_DOMAINS:
if self.state.state == STATE_UNKNOWN:
raise SmartHomeError(
ERR_NOT_SUPPORTED, "Querying state is not supported"
)

position = self.state.attributes.get(cover.ATTR_CURRENT_POSITION)
position = self.state.attributes.get(COVER_VALVE_CURRENT_POSITION[domain])

if position is not None:
response["openPercent"] = position
elif self.state.state != cover.STATE_CLOSED:
elif self.state.state != COVER_VALVE_STATES[domain]["closed"]:
response["openPercent"] = 100
else:
response["openPercent"] = 0
Expand All @@ -2162,11 +2238,13 @@ async def execute(self, command, data, params, challenge):
domain = self.state.domain
features = self.state.attributes.get(ATTR_SUPPORTED_FEATURES, 0)

if domain == cover.DOMAIN:
if domain in COVER_VALVE_DOMAINS:
svc_params = {ATTR_ENTITY_ID: self.state.entity_id}
should_verify = False
if command == COMMAND_OPENCLOSE_RELATIVE:
position = self.state.attributes.get(cover.ATTR_CURRENT_POSITION)
position = self.state.attributes.get(
COVER_VALVE_CURRENT_POSITION[domain]
)
if position is None:
raise SmartHomeError(
ERR_NOT_SUPPORTED,
Expand All @@ -2177,16 +2255,16 @@ async def execute(self, command, data, params, challenge):
position = params["openPercent"]

if position == 0:
service = cover.SERVICE_CLOSE_COVER
service = SERVICE_CLOSE_COVER_VALVE[domain]
should_verify = False
elif position == 100:
service = cover.SERVICE_OPEN_COVER
service = SERVICE_OPEN_COVER_VALVE[domain]
should_verify = True
elif features & CoverEntityFeature.SET_POSITION:
service = cover.SERVICE_SET_COVER_POSITION
elif features & COVER_VALVE_SET_POSITION_FEATURE[domain]:
service = SERVICE_SET_POSITION_COVER_VALVE[domain]
if position > 0:
should_verify = True
svc_params[cover.ATTR_POSITION] = position
svc_params[COVER_VALVE_POSITION[domain]] = position
else:
raise SmartHomeError(
ERR_NOT_SUPPORTED, "No support for partial open close"
Expand All @@ -2200,7 +2278,7 @@ async def execute(self, command, data, params, challenge):
_verify_pin_challenge(data, self.state, challenge)

await self.hass.services.async_call(
cover.DOMAIN,
domain,
service,
svc_params,
blocking=not self.config.should_report_state,
Expand Down
1 change: 1 addition & 0 deletions tests/components/google_assistant/snapshots/test_diagnostics.ambr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@
'sensor',
'switch',
'vacuum',
'valve',
'water_heater',
]),
'project_id': '1234',
Expand Down
Loading