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

Add Remootio integration #63155

Draft
wants to merge 70 commits into
base: dev
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
70 commits
Select commit Hold shift + click to select a range
72ed83f
Add Remootio integration.
ivgg-me Dec 31, 2021
0a63b42
Some of the changes requested by @elupus; see https://github.com/home…
ivgg-me Jan 1, 2022
c4f9583
Some of the changes requested by @elupus; see https://github.com/home…
ivgg-me Jan 1, 2022
ab7addc
Some of the changes requested by @elupus; see https://github.com/home…
ivgg-me Jan 6, 2022
d906872
Some of the changes requested by @elupus; see https://github.com/home…
ivgg-me Jan 8, 2022
bc483dc
Some changes requested by @elupus; see https://github.com/home-assist…
ivgg-me Mar 13, 2022
4c6cf9c
Fixing linting errors
ivgg-me Mar 13, 2022
1c35a75
Some changes requested by @elupus; see https://github.com/home-assist…
ivgg-me Mar 19, 2022
4d79011
Fixing merge conflicts
ivgg-me Apr 2, 2022
a392017
Changes rquested by @elupus; see https://github.com/home-assistant/co…
ivgg-me Apr 14, 2022
0bb357e
Requested changes; see https://github.com/home-assistant/core/pull/63…
ivgg-me Apr 17, 2022
af69d84
Solving merge conflicts
ivgg-me Apr 17, 2022
c098bfa
Log entries for debuging.
ivgg-me May 29, 2022
5077c98
Integration of new version of aioremootio
ivgg-me Jun 1, 2022
0f0312c
Integration of new version of aioremootio
ivgg-me Jun 1, 2022
ed5a2dc
Integration of new version of aioremootio
ivgg-me Jun 7, 2022
4864a95
bug fixes
zlodag Jun 16, 2022
70c4a70
Some refinements needed for the open MR
ivgg-me Oct 27, 2022
a0c3059
Fixing tests
ivgg-me Oct 27, 2022
5682939
Changes needed by the MR
ivgg-me Nov 1, 2022
541ee26
Changes needed by the MR
ivgg-me Nov 1, 2022
de882b3
Update homeassistant/components/remootio/cover.py
ivgg-me Dec 26, 2022
4b6212e
Update homeassistant/components/remootio/cover.py
ivgg-me Dec 26, 2022
4b5c272
Update homeassistant/components/remootio/cover.py
ivgg-me Dec 26, 2022
ad0bcda
Update homeassistant/components/remootio/cover.py
ivgg-me Dec 26, 2022
9412d8d
Update homeassistant/components/remootio/cover.py
ivgg-me Dec 26, 2022
964a871
Update homeassistant/components/remootio/cover.py
ivgg-me Dec 26, 2022
4788e55
Add Remootio integration.
ivgg-me May 31, 2023
e35d785
Add Remootio integration
ivgg-me Aug 26, 2023
205c3c6
Add Remootio integration.
ivgg-me Dec 31, 2021
07be815
Some of the changes requested by @elupus; see https://github.com/home…
ivgg-me Jan 1, 2022
c664fbd
Some of the changes requested by @elupus; see https://github.com/home…
ivgg-me Jan 1, 2022
d52ba0b
Some of the changes requested by @elupus; see https://github.com/home…
ivgg-me Jan 6, 2022
c295b76
Some of the changes requested by @elupus; see https://github.com/home…
ivgg-me Jan 8, 2022
c978d01
Some changes requested by @elupus; see https://github.com/home-assist…
ivgg-me Mar 13, 2022
d944651
Fixing linting errors
ivgg-me Mar 13, 2022
820fbd2
Some changes requested by @elupus; see https://github.com/home-assist…
ivgg-me Mar 19, 2022
9f07c8a
Fixing merge conflicts
ivgg-me Apr 2, 2022
b3cbe90
Changes rquested by @elupus; see https://github.com/home-assistant/co…
ivgg-me Apr 14, 2022
ca49fb9
Requested changes; see https://github.com/home-assistant/core/pull/63…
ivgg-me Apr 17, 2022
86d6e34
Solving merge conflicts
ivgg-me Apr 17, 2022
6935c68
Log entries for debuging.
ivgg-me May 29, 2022
1fe27b1
Integration of new version of aioremootio
ivgg-me Jun 1, 2022
44e1ab7
Integration of new version of aioremootio
ivgg-me Jun 1, 2022
66ea842
Integration of new version of aioremootio
ivgg-me Jun 7, 2022
550df9f
bug fixes
zlodag Jun 16, 2022
6baae92
Some refinements needed for the open MR
ivgg-me Oct 27, 2022
01fafc0
Fixing tests
ivgg-me Oct 27, 2022
8e0ef5c
Changes needed by the MR
ivgg-me Nov 1, 2022
9f73129
Changes needed by the MR
ivgg-me Nov 1, 2022
ffd7ba1
Update homeassistant/components/remootio/cover.py
ivgg-me Dec 26, 2022
37571b6
Update homeassistant/components/remootio/cover.py
ivgg-me Dec 26, 2022
fe32fd1
Update homeassistant/components/remootio/cover.py
ivgg-me Dec 26, 2022
1712872
Update homeassistant/components/remootio/cover.py
ivgg-me Dec 26, 2022
b43f7c0
Update homeassistant/components/remootio/cover.py
ivgg-me Dec 26, 2022
0fd038a
Update homeassistant/components/remootio/cover.py
ivgg-me Dec 26, 2022
bd64835
Add Remootio integration.
ivgg-me May 31, 2023
ced8e5b
Add Remootio integration
ivgg-me Aug 26, 2023
4c0932b
Remootio integration
ivgg-me Dec 24, 2023
ee279b1
Remootio integration
ivgg-me Dec 24, 2023
9ebc4c6
Remootio integration.
ivgg-me Dec 24, 2023
0cf86ba
Fix for ruff checks and support for device triggers
ivgg-me Dec 31, 2023
116d986
Support for device triggers
ivgg-me Dec 31, 2023
532d071
Update homeassistant/components/remootio/__init__.py
ivgg-me Feb 12, 2024
8481008
Update homeassistant/components/remootio/config_flow.py
ivgg-me Feb 12, 2024
d088fac
Review outcome.
ivgg-me Feb 12, 2024
662af0b
Review outcome
ivgg-me Feb 12, 2024
ab81bd7
Build checks outcome
ivgg-me Feb 13, 2024
b66d9cf
Review outcome
ivgg-me Feb 13, 2024
00bebf0
Update homeassistant/components/remootio/__init__.py
ivgg-me Feb 14, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions CODEOWNERS
Validating CODEOWNERS rules …
Expand Up @@ -1092,6 +1092,8 @@ build.json @home-assistant/supervisor
/homeassistant/components/refoss/ @ashionky
/tests/components/refoss/ @ashionky
/homeassistant/components/rejseplanen/ @DarkFox
/homeassistant/components/remootio/ @ivgg-me
/tests/components/remootio/ @ivgg-me
/homeassistant/components/remote/ @home-assistant/core
/tests/components/remote/ @home-assistant/core
/homeassistant/components/renault/ @epenet
Expand Down
105 changes: 105 additions & 0 deletions homeassistant/components/remootio/__init__.py
@@ -0,0 +1,105 @@
"""The Remootio integration."""
from __future__ import annotations

import logging

from aioremootio import ConnectionOptions, RemootioClient

from homeassistant.config_entries import ConfigEntry
from homeassistant.const import ATTR_ENTITY_ID, ATTR_NAME, CONF_HOST, Platform
from homeassistant.core import HomeAssistant, callback

from .const import (
ATTR_SERIAL_NUMBER,
ATTR_TYPE,
CONF_API_AUTH_KEY,
CONF_API_SECRET_KEY,
CONF_SERIAL_NUMBER,
DOMAIN,
EVENT_HANDLER_CALLBACK,
EVENT_TYPE,
REMOOTIO_CLIENT,
)
from .cover import RemootioCoverEvent
from .utils import create_client

_LOGGER = logging.getLogger(__name__)

PLATFORMS = [Platform.COVER]


async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
"""Set up Remootio from a config entry."""

_LOGGER.debug("Doing async_setup_entry. entry [%s]", entry.as_dict())

@callback
def handle_event(event: RemootioCoverEvent) -> None:
_LOGGER.debug(
"Firing event. EvenType [%s] RemootioCoverEntityId [%s] RemootioDeviceSerialNumber [%s]",
event.type,
event.entity_id,
event.device_serial_number,
)

hass.bus.async_fire(
EVENT_TYPE,
{
ATTR_ENTITY_ID: event.entity_id,
ATTR_SERIAL_NUMBER: event.device_serial_number,
ATTR_NAME: event.entity_name,
ATTR_TYPE: event.type,
},
)
Comment on lines +36 to +53
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove the custom event for now, this functionality should be modeled as an event entity instead, in a follow-up PR: https://developers.home-assistant.io/docs/core/entity/event

Copy link
Author

Choose a reason for hiding this comment

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

How should I then fire events based on the events that the Remootio fires and on which the user may want to set up an automation, e.g. if the controlled garden gate or garage door remains open for a time that can be configured on the Remootio?

Copy link
Contributor

Choose a reason for hiding this comment

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

Please look into event entities which I linked to: https://developers.home-assistant.io/docs/core/entity/event/, I think it will fit this use case very well.


connection_options: ConnectionOptions = ConnectionOptions(
entry.data[CONF_HOST],
entry.data[CONF_API_SECRET_KEY],
entry.data[CONF_API_AUTH_KEY],
False,
)
serial_number: str = entry.data[CONF_SERIAL_NUMBER]

remootio_client: RemootioClient = await create_client(
hass, connection_options, _LOGGER, serial_number
)

async def terminate_client() -> None:
_LOGGER.debug(
"Remootio client will now be terminated. entry [%s]", entry.as_dict()
)

terminated: bool = await remootio_client.terminate()
if terminated:
_LOGGER.debug(
"Remootio client successfully terminated. entry [%s]", entry.as_dict()
)

entry.async_on_unload(terminate_client)

hass.data.setdefault(DOMAIN, {})[entry.entry_id] = {
REMOOTIO_CLIENT: remootio_client,
EVENT_HANDLER_CALLBACK: handle_event,
}

await hass.config_entries.async_forward_entry_setups(entry, PLATFORMS)

return True


async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
"""Unload a config entry."""

_LOGGER.debug(
"Doing async_unload_entry. entry [%s] hass.data[%s][%s] [%s]",
entry.as_dict(),
DOMAIN,
entry.entry_id,
hass.data.get(DOMAIN, {}).get(entry.entry_id, {}),
)

platforms_unloaded = await hass.config_entries.async_unload_platforms(
entry, PLATFORMS
)

return platforms_unloaded
179 changes: 179 additions & 0 deletions homeassistant/components/remootio/config_flow.py
@@ -0,0 +1,179 @@
"""Config flow for Remootio integration."""
from __future__ import annotations

import logging
from typing import Any

from aioremootio import (
ConnectionOptions,
RemootioClientAuthenticationError,
RemootioClientConnectionEstablishmentError,
)
from aioremootio.constants import (
CONNECTION_OPTION_REGEX_API_AUTH_KEY,
CONNECTION_OPTION_REGEX_API_SECRET_KEY,
CONNECTION_OPTION_REGEX_HOST,
)
import voluptuous as vol
from voluptuous.error import RequiredFieldInvalid
from voluptuous.schema_builder import REMOVE_EXTRA

from homeassistant import config_entries
from homeassistant.components.cover import CoverDeviceClass
from homeassistant.const import CONF_DEVICE_CLASS, CONF_HOST
from homeassistant.core import HomeAssistant
from homeassistant.data_entry_flow import FlowResult

from .const import (
CONF_API_AUTH_KEY,
CONF_API_SECRET_KEY,
CONF_DATA,
CONF_SERIAL_NUMBER,
CONF_TITLE,
DOMAIN,
)
from .exceptions import UnsupportedRemootioDeviceError
from .utils import get_serial_number

_LOGGER = logging.getLogger(__name__)

INPUT_VALIDATION_SCHEMA = vol.Schema(
{
vol.Required(CONF_HOST, msg="Host is required"): vol.All(
vol.Coerce(str),
vol.Match(CONNECTION_OPTION_REGEX_HOST),
msg="Host appears to be invalid; it can be an IP address or a host name that complies with RFC-1123",
),
vol.Required(CONF_API_SECRET_KEY, msg="API Secret Key is required"): vol.All(
vol.Coerce(str),
vol.Upper,
vol.Match(CONNECTION_OPTION_REGEX_API_SECRET_KEY),
msg="API Secret Key appears to be invalid; it must be a sequence of 64 characters and can contain only numbers and english letters",
),
vol.Required(CONF_API_AUTH_KEY, msg="API Auth Key is required"): vol.All(
vol.Coerce(str),
vol.Upper,
vol.Match(CONNECTION_OPTION_REGEX_API_AUTH_KEY),
msg="API Auth Key appears to be invalid; it must be a sequence of 64 characters and can contain only numbers and english letters",
),
vol.Required(
CONF_DEVICE_CLASS,
default=CoverDeviceClass.GARAGE,
msg="Controlled device's class is required",
): vol.All(
vol.Coerce(str),
vol.In([CoverDeviceClass.GARAGE, CoverDeviceClass.GATE]),
msg="Controlled device's class appears to be invalid",
Comment on lines +45 to +66
Copy link
Member

Choose a reason for hiding this comment

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

I don't think you should need all these messages. You can use the data_description field in the strings.json to give the user some more info.

Downside to these messages is that they cant be translated

Copy link
Author

Choose a reason for hiding this comment

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

This messages are only for logging purposes, the messages for the user comes from strings.json. - See handling of validation errors from line 115.

Copy link
Contributor

Choose a reason for hiding this comment

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

If it's for logging, there's no need to have such detailed error messages though.

Copy link
Author

Choose a reason for hiding this comment

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

Sorry @emontnemery, but I can't share this opinion. Why are log messages like the following ones not needed?

2024-02-13 14:41:41.980 ERROR (MainThread) [homeassistant.components.remootio.config_flow] Invalid user input. MultipleInvalid.Errors [[RequiredFieldInvalid('API Auth Key is required')]]

2024-02-13 14:42:12.787 ERROR (MainThread) [homeassistant.components.remootio.config_flow] Invalid user input. MultipleInvalid.Errors [[RequiredFieldInvalid('API Secret Key is required'), RequiredFieldInvalid('API Auth Key is required'), RequiredFieldInvalid('Host is required')]]

Copy link
Contributor

Choose a reason for hiding this comment

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

Feedback to the user about how to fill in the form should happen via the UI not via error messages in the log.

Your examples, where keys are is missing, are unrealistic because the UI will not allow the user to submit the form if not filled in.
Hence, for the missing keys please remove the messages:

  • There's no need to customize the error messages for something which can't happen, because the error messages will never be printed
  • It's confusing for others looking at your code, because the customized error messages suggests there's something special happening and we can't trust frontend.

For the values, there's again no need to customize the messages because user feedback happens via the UI. I guess it's OK to keep the custom messages if you really want them, but it's an unnecessary burden to maintain because the natural language error messages now need to be maintained both in strings.json and in the Python code.

),
},
extra=REMOVE_EXTRA,
)

DEVICE_NAME = "Remootio Device"


async def validate_input(hass: HomeAssistant, data: dict[str, Any]) -> dict[str, Any]:
"""Validate the user input."""

_LOGGER.debug("Validating input... Input [%s]", data)

data = INPUT_VALIDATION_SCHEMA(data)

connection_options: ConnectionOptions = ConnectionOptions(
data[CONF_HOST], data[CONF_API_SECRET_KEY], data[CONF_API_AUTH_KEY]
)

device_serial_number: str = await get_serial_number(
hass, connection_options, _LOGGER
)

data[CONF_SERIAL_NUMBER] = device_serial_number

return {
CONF_TITLE: f"{DEVICE_NAME} (Host: {data[CONF_HOST]}, S/N: {device_serial_number})",
CONF_DATA: data,
}


class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
"""Handle a config flow for Remootio."""

VERSION = 1

async def async_step_user(
self, user_input: dict[str, Any] | None = None
) -> FlowResult:
"""Handle the user step."""
user_input = user_input or {}
Copy link
Member

Choose a reason for hiding this comment

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

No need to do this?

Copy link
Author

Choose a reason for hiding this comment

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

The parameter user_input defaults to None in this case it will be set to an empty array and I can check the length of it in line 113.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree, this is not needed if combined with the corresponding change below

Suggested change
user_input = user_input or {}

errors = {}

if len(user_input) != 0:
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't check len, just check if the user input is truthy, if it is, it's dict with all required keys

Suggested change
if len(user_input) != 0:
if user_input:

validation_result = {}

try:
validation_result = await validate_input(self.hass, user_input)
except UnsupportedRemootioDeviceError:
_LOGGER.debug("Remootio device isn't supported", exc_info=True)
return self.async_abort(reason="unsupported_device")
except vol.MultipleInvalid as ex:
_LOGGER.error(
Copy link
Contributor

Choose a reason for hiding this comment

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

There's no reason why the user should see this in their log, please lower severity to debug

Suggested change
_LOGGER.error(
_LOGGER.debug(

"Invalid user input. MultipleInvalid.Errors [%s]", ex.errors
)
for error in ex.errors:
_LOGGER.debug(
"Error [%s] Path [%s]", error.__class__.__name__, error.path[0]
)
if isinstance(error, RequiredFieldInvalid):
errors[str(error.path[0])] = f"{error.path[0]}_required"
else:
errors[str(error.path[0])] = f"{error.path[0]}_invalid"
Comment on lines +122 to +129
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think you need to do this yourself anymore, please take a look at #108075

except RemootioClientConnectionEstablishmentError:
_LOGGER.error("Can't connect to Remootio device")
errors["base"] = "cannot_connect"
except RemootioClientAuthenticationError:
_LOGGER.error("Can't authenticate by the Remootio device")
errors["base"] = "invalid_auth"
except Exception: # pylint: disable=broad-except
_LOGGER.exception("Unexpected exception/error")
errors["base"] = "unknown"
else:
await self.async_set_unique_id(
validation_result[CONF_DATA][CONF_SERIAL_NUMBER]
)
self._abort_if_unique_id_configured(validation_result[CONF_DATA])

return self.async_create_entry(
title=validation_result[CONF_TITLE],
data=validation_result[CONF_DATA],
)

return self.async_show_form(
step_id="user",
data_schema=vol.Schema(
{
vol.Optional(
CONF_HOST,
default=user_input.get(CONF_HOST, vol.UNDEFINED),
): vol.Coerce(str),
vol.Optional(
CONF_API_SECRET_KEY,
default=user_input.get(CONF_API_SECRET_KEY, vol.UNDEFINED),
): vol.Coerce(str),
vol.Optional(
CONF_API_AUTH_KEY,
default=user_input.get(CONF_API_AUTH_KEY, vol.UNDEFINED),
): vol.Coerce(str),
Comment on lines +154 to +165
Copy link
Member

Choose a reason for hiding this comment

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

Why are they all optional?

Copy link
Author

Choose a reason for hiding this comment

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

Because the user_input will be validated based on the INPUT_VALIDATION_SCHEMA by invoking the method validate_input in line 114 and if a validation error occurs, an meaningful message will be shown to the user.

Copy link
Contributor

Choose a reason for hiding this comment

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

The user should not have to look at error messages in the log to understand how to fill in the schema, please make the fields required here as requested by @joostlek

vol.Optional(
CONF_DEVICE_CLASS,
default=user_input.get(
CONF_DEVICE_CLASS, CoverDeviceClass.GARAGE
),
): vol.All(
vol.Coerce(str),
vol.In([CoverDeviceClass.GARAGE, CoverDeviceClass.GATE]),
),
ivgg-me marked this conversation as resolved.
Show resolved Hide resolved
Comment on lines +166 to +174
Copy link
Contributor

Choose a reason for hiding this comment

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

As mentioned in another comment, please remove the device class setting from here

},
extra=REMOVE_EXTRA,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need this? Where do the extra keys come from?

),
errors=errors,
)
34 changes: 34 additions & 0 deletions homeassistant/components/remootio/const.py
@@ -0,0 +1,34 @@
"""Constants for the Remootio integration."""
from homeassistant.const import CONF_TYPE

# Domain of the integration
DOMAIN = "remootio"

# Event type of the integration
EVENT_TYPE = f"{DOMAIN.lower()}_event"

# Timeout used in methods of remootio.utils
REMOOTIO_TIMEOUT = 60

# Deleay used in methods of remootio.utils
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# Deleay used in methods of remootio.utils
# Delay used in methods of remootio.utils

REMOOTIO_DELAY = 0.5

# Expected minimum Remootio Websocket API version supported by this integration
EXPECTED_MINIMUM_API_VERSION = 2

# Keys used by the Config flow
CONF_API_SECRET_KEY = "secret__api_secret_key"
CONF_API_AUTH_KEY = "secret__api_auth_key"
CONF_TITLE = "title"
CONF_SERIAL_NUMBER = "serial_number"
CONF_DATA = "data"

# Keys for event data fired by remootio.cover.RemootioCoverEventListener
ATTR_SERIAL_NUMBER = CONF_SERIAL_NUMBER
ATTR_TYPE = CONF_TYPE

# Key for the dictionary entry which holds the instance of aioremootio.RemootioClient to connect to the Remootio device using the Remootio Websocket API
REMOOTIO_CLIENT = "remootio_client"

# Key for the dictionary entry which holds reference to the callback to be invoked to handle events triggered by the Remootio device
EVENT_HANDLER_CALLBACK = "event_handler_callback"