Skip to content
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

Implement thermostat support for Alexa #13340

Merged
merged 5 commits into from
Mar 30, 2018

Conversation

trisk
Copy link
Contributor

@trisk trisk commented Mar 19, 2018

Description:

This provides full Alexa.ThermostatController support for thermostats with multiple setpoints. These entities also support the Alexa.TemperatureSensor interface for reporting the current temperature.

Example entry for configuration.yaml (if applicable):

alexa:
  smart_home:
    filter:
      include_domains:
        - climate

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass

If the code does not interact with devices:

  • Tests have been added to verify that the new code works.

'Alexa.TemperatureSensor',
)

properties = yield from reported_properties(hass, 'climate#test_thermostat')

Choose a reason for hiding this comment

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

line too long (80 > 79 characters)



def temperature_from_object(temperature, to_unit, absolute=True)
"""Get temperature from Temperature object in requested unit."""

Choose a reason for hiding this comment

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

unexpected indentation

temp = self.entity.state
if self.entity.domain == climate.DOMAIN:
temp =
self.entity.attributes.get(climate.ATTR_CURRENT_TEMPERATURE, 0)

Choose a reason for hiding this comment

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

unexpected indentation

@@ -383,12 +371,85 @@ def get_property(self, name):
raise _UnsupportedProperty(name)

unit = self.entity.attributes[CONF_UNIT_OF_MEASUREMENT]
temp = self.entity.state
if self.entity.domain == climate.DOMAIN:
temp =

Choose a reason for hiding this comment

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

SyntaxError: invalid syntax


API_TEMP_UNITS = {
TEMP_FAHRENHEIT: 'FAHRENHEIT',
TEMP_CELSIUS: 'CELSIUS',

Choose a reason for hiding this comment

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

undefined name 'TEMP_CELSIUS'

SMART_HOME_HTTP_ENDPOINT = '/api/alexa/smart_home'

API_TEMP_UNITS = {
TEMP_FAHRENHEIT: 'FAHRENHEIT',

Choose a reason for hiding this comment

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

undefined name 'TEMP_FAHRENHEIT'

'Alexa.TemperatureSensor',
)

properties = yield from reported_properties(hass, 'climate#test_thermostat')

Choose a reason for hiding this comment

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

line too long (80 > 79 characters)



def temperature_from_object(temperature, to_unit, absolute=True)
"""Get temperature from Temperature object in requested unit."""

Choose a reason for hiding this comment

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

unexpected indentation

temp = self.entity.state
if self.entity.domain == climate.DOMAIN:
temp =
self.entity.attributes.get(climate.ATTR_CURRENT_TEMPERATURE, 0)

Choose a reason for hiding this comment

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

unexpected indentation

@@ -383,12 +371,85 @@ def get_property(self, name):
raise _UnsupportedProperty(name)

unit = self.entity.attributes[CONF_UNIT_OF_MEASUREMENT]
temp = self.entity.state
if self.entity.domain == climate.DOMAIN:
temp =

Choose a reason for hiding this comment

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

SyntaxError: invalid syntax


API_TEMP_UNITS = {
TEMP_FAHRENHEIT: 'FAHRENHEIT',
TEMP_CELSIUS: 'CELSIUS',

Choose a reason for hiding this comment

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

undefined name 'TEMP_CELSIUS'

SMART_HOME_HTTP_ENDPOINT = '/api/alexa/smart_home'

API_TEMP_UNITS = {
TEMP_FAHRENHEIT: 'FAHRENHEIT',

Choose a reason for hiding this comment

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

undefined name 'TEMP_FAHRENHEIT'

@@ -383,12 +377,84 @@ def get_property(self, name):
raise _UnsupportedProperty(name)

unit = self.entity.attributes[CONF_UNIT_OF_MEASUREMENT]
temp = self.entity.state
if self.entity.domain == climate.DOMAIN:
temp =/ self.entity.attributes.get(climate.ATTR_CURRENT_TEMPERATURE, 0)

Choose a reason for hiding this comment

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

missing whitespace around operator
SyntaxError: invalid syntax
multiple spaces after operator
line too long (84 > 79 characters)


@HANDLERS.register(('Alexa.ThermostatController', 'SetTargetTemperature')
@asyncio.coroutine
def async_api_set_target_temperature(hass, config, request, entity):

Choose a reason for hiding this comment

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

SyntaxError: invalid syntax


data = {
ATTR_ENTITY_ID: entity.entity_id
ATTR_TEMPERATURE: entity.attributes[ATTR_TEMPERATURE] + delta

Choose a reason for hiding this comment

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

SyntaxError: invalid syntax

'target_temp_low': 60.0,
'current_temperature': 75.0,
'friendly_name': "Test Thermostat",
'supported_features': 1|2|4|128,

Choose a reason for hiding this comment

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

missing whitespace around bitwise or shift operator

request,
namespace='Alexa.ThermostatController',
error_type='UNSUPPORTED_THERMOSTAT_MODE',
error_message=msg

Choose a reason for hiding this comment

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

missing whitespace around operator

return api_error(
request,
namespace='Alexa.ThermostatController',
error_type='UNSUPPORTED_THERMOSTAT_MODE',

Choose a reason for hiding this comment

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

missing whitespace around operator

_LOGGER.info(msg)
return api_error(
request,
namespace='Alexa.ThermostatController',

Choose a reason for hiding this comment

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

missing whitespace around operator

msg = 'The requested thermostat mode {} is not supported'.format(mode)
_LOGGER.info(msg)
return api_error(
request,

Choose a reason for hiding this comment

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

unexpected indentation

None)
)
if ha_mode not in operation_list:
msg = 'The requested thermostat mode {} is not supported'.format(mode)

Choose a reason for hiding this comment

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

unexpected spaces around keyword / parameter equals

(k for k, v in API_THERMOSTAT_MODES.items() if v == mode),
None)
)
if ha_mode not in operation_list:

Choose a reason for hiding this comment

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

continuation line missing indentation or outdented

ha_mode = next(
(k for k, v in API_THERMOSTAT_MODES.items() if v == mode),
None)
)

Choose a reason for hiding this comment

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

SyntaxError: invalid syntax

@@ -693,6 +693,134 @@ def test_unknown_sensor(hass):
yield from discovery_test(device, hass, expected_endpoints=0)


@asyncio.coroutine
Copy link
Member

Choose a reason for hiding this comment

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

We've migrated to async/await syntax. Please drop this decorator and prefix async methods with async like async def test_thermostat. Replace any yield from with await.

mode = request[API_PAYLOAD]['thermostatMode']

operation_list = entity.attributes.get(climate.ATTR_OPERATION_LIST)
# pylint: disable=stop-iteration-return
Copy link
Member

Choose a reason for hiding this comment

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

What are you disabling here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The travis-ci build uses a version of pylint that incorrectly fails next(..., default) as throwing an exception. pylint-dev/pylint#1830

)
if ha_mode not in operation_list:
msg = 'The requested thermostat mode {} is not supported'.format(mode)
_LOGGER.info(msg)
Copy link
Member

Choose a reason for hiding this comment

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

This should be an error ? And should we have the alexa smart home handler just log an error when we are returning an error message?

Copy link
Contributor Author

@trisk trisk Mar 28, 2018

Choose a reason for hiding this comment

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

Totally valid requests from the user can specify modes or temperatures not supported by the device (since the API does not enumerate the accepted values) and I'm logging them at info level since they're not indicative of software bugs or configuration errors.

You're right, it might make sense to log all error responses we send though.


@HANDLERS.register(('Alexa.ThermostatController', 'AdjustTargetTemperature'))
@extract_entity
@asyncio.coroutine
Copy link
Member

Choose a reason for hiding this comment

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

We've migrated to async/await syntax. Please drop this decorator and prefix async methods with async like async def async_api_adjust_target_temp. Replace any yield from with await.

}

msg = 'The requested temperature {} is out of range'.format(temp)
_LOGGER.info(msg)
Copy link
Member

Choose a reason for hiding this comment

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

Stale debug

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm logging this intentionally, but it might make sense to do it from the api_error method.

_LOGGER.warning("Request %s/%s error %s: %s",
request[API_HEADER]['namespace'],
request[API_HEADER]['name'],
error_type, error_message)

Choose a reason for hiding this comment

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

continuation line under-indented for visual indent


_LOGGER.warning("Request %s/%s error %s: %s",
request[API_HEADER]['namespace'],
request[API_HEADER]['name'],

Choose a reason for hiding this comment

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

continuation line under-indented for visual indent

payload['message'] = error_message

_LOGGER.warning("Request %s/%s error %s: %s",
request[API_HEADER]['namespace'],

Choose a reason for hiding this comment

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

continuation line under-indented for visual indent

_LOGGER.info("Request %s/%s error %s: %s",
request[API_HEADER]['namespace'],
request[API_HEADER]['name'],
error_type, error_message)

Choose a reason for hiding this comment

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

continuation line over-indented for visual indent


_LOGGER.info("Request %s/%s error %s: %s",
request[API_HEADER]['namespace'],
request[API_HEADER]['name'],

Choose a reason for hiding this comment

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

continuation line over-indented for visual indent

payload['message'] = error_message

_LOGGER.info("Request %s/%s error %s: %s",
request[API_HEADER]['namespace'],

Choose a reason for hiding this comment

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

continuation line over-indented for visual indent

_LOGGER.info("Request %s/%s error %s: %s",
request[API_HEADER]['namespace'],
request[API_HEADER]['name'],
error_type, error_message)

Choose a reason for hiding this comment

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

continuation line over-indented for visual indent


_LOGGER.info("Request %s/%s error %s: %s",
request[API_HEADER]['namespace'],
request[API_HEADER]['name'],

Choose a reason for hiding this comment

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

continuation line over-indented for visual indent

payload['message'] = error_message

_LOGGER.info("Request %s/%s error %s: %s",
request[API_HEADER]['namespace'],

Choose a reason for hiding this comment

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

continuation line over-indented for visual indent

@balloob balloob merged commit 5801d78 into home-assistant:dev Mar 30, 2018
@balloob
Copy link
Member

balloob commented Mar 30, 2018

Nice! 🎉

@balloob balloob mentioned this pull request Apr 13, 2018
@home-assistant home-assistant locked and limited conversation to collaborators Jul 26, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants