Add minimal UniFi Access integration #135139

hagen93 · 2025-01-08T21:16:07Z

Proposed change

Adds very minimal initial support or UniFi Access (only lock entities).

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)
Deprecation (breaking change to happen in the future)
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: Add minimal UniFi Access integration home-assistant.io#36883

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
I have followed the perfect PR recommendations
The code has been formatted using Ruff (ruff format homeassistant tests)
Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

Documentation added/updated for www.home-assistant.io

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.

To help with the load of incoming pull requests:

I have reviewed two other open pull requests in this repository.

home-assistant

Hi @andreashagensjolvsagt

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

home-assistant · 2025-01-08T21:16:12Z

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

hagen93 · 2025-01-08T21:26:12Z

Hi! I've implemented a very bare bones UniFi Access lock integration based on the new(-ish) UniFi Access REST API. It can currently trigger an unlock and refreshes back to locked when notified by UniFi that the door has been reset back to locked. Locking is not implemented as unlocks in UniFi is mainly time based.

I've had a few stumbles with how the integration code is intended to be structured, but for now I've taken note of the following that might need changes before merge:

I've not found the indented way to start a background thread for a perpetual websocket, so my current implementation causes HA to never finish initialization (message on the bottom never disappears).
I've not been able to understand how tests are intended to be structured to ensure the integration plays nice with HA. I've gathered as much as one seems to be intended to initialize HA with the required integration components loaded and then orchestrate the test state, but I've not found reliable examples for how to do this for e.g. a coordinator or an entity class. I'm happy to improve this given some examples from other similar code, possibly with a quick line or two of explanation.

Cheers,
Andy

bdraco · 2025-01-09T00:24:54Z

Since this is new, please use unifi_access for the naming.

Thanks

andreashagensjolvsagt · 2025-01-09T09:58:14Z

Since this is new, please use unifi_access for the naming.

Thanks

How about now? 🙂

Stale

homeassistant/components/unifi_access/__init__.py

joostlek · 2025-01-13T12:24:23Z

homeassistant/components/unifi_access/__init__.py

+    host = f"https://{entry.data.get(CONF_HOST)}/api/v1/developer"
+    configuration = uiaccessclient.Configuration(
+        host, access_token=entry.data.get(CONF_API_TOKEN)
+    )


Why do we have the endpoint here? I find the /api/v1/developer url to be device/service specific and should be in the library IMO

Agreed. It is already in the library too, but was exposed awkwardly due to how I generated the client from OpenAPI. See the comment below about websocket. Will add a wrapper for configuration in the same way to fix this.

This is now moved to library.

joostlek · 2025-01-13T12:24:38Z

homeassistant/components/unifi_access/__init__.py

+    configuration = uiaccessclient.Configuration(
+        host, access_token=entry.data.get(CONF_API_TOKEN)
+    )
+    configuration.verify_ssl = entry.data.get(CONF_VERIFY_SSL)


why isnt this a parameter?

Will fix with wrapper. See comment above.

This is now moved to library.

joostlek · 2025-01-13T12:24:49Z

homeassistant/components/unifi_access/__init__.py

+    configuration.verify_ssl = entry.data.get(CONF_VERIFY_SSL)
+    api_client = uiaccessclient.ApiClient(configuration)
+
+    door_coordinator = UniFiAccessDoorCoordinator(hass, api_client)


why is it called door?

Door is terminology from the UniFi Access documentation. In reality this represents the accumulated state from a UniFi Access Hub, and any attached peripherals.

joostlek · 2025-01-13T12:28:38Z

homeassistant/components/unifi_access/config_flow.py

+
+STEP_USER_DATA_SCHEMA = vol.Schema(
+    {
+        vol.Required(CONF_HOST, default=DEFAULT_HOST): str,


So users host this themselves? What should the user enter? An url with port?

Because

In that URL in __init__.py we assume that they have this hosting on https, is that required for this kind of software?

Can they host it on different ports?

This is a service exposed by UniFi controllers. I might be wrong, but I don't think Ubiquity have made this available on any non-HTTPS endpoints. As far as I know you would need to have either a compatible UniFi controller on official Ubiquity hardware on premise where this is hosted, or connect to a cloud hosted UniFi controller. Either would be over HTTPS as far as I know.

Little quick on the last one and forgot to answer as to the port, but as far as I can see that port is also fixed. I don't see any exposed configuration on my UniFi console to change it, and the documentation for the API seems to indicate that both HTTPS and port is fixed as is.

joostlek · 2025-01-13T12:32:40Z

homeassistant/components/unifi_access/coordinator.py

+            hass,
+            _LOGGER,
+            name="UniFi Access door",
+            always_update=False,


why the always_update = False?

The models returned by the client are comparable and as such as far as I can tell "always_update" is not needed, as we can limit updates to whenever the state actually changes. Is this not the correct way of doing that?

joostlek · 2025-01-13T12:33:08Z

homeassistant/components/unifi_access/coordinator.py

+    async def receive_updated_data(self) -> None:
+        """Start websocket receiver for updated data from UniFi Access."""
+        _LOGGER.debug(
+            "Starting UniFi Access websocket with %s", self.configuration.host
+        )
+        try:
+            async with (
+                aiohttp.ClientSession(
+                    base_url=self.configuration.host.rstrip("/") + "/",
+                    headers={
+                        "Authorization": f"Bearer {self.configuration.access_token}"
+                    },
+                ) as session,
+                session.ws_connect(
+                    "devices/notifications", verify_ssl=self.configuration.verify_ssl
+                ) as socket,
+            ):
+                # WebSocket API is poorly documented so we will just use the REST API whenever we get
+                # an update to fetch all the relevant data.
+                async for message in socket:
+                    json = message.json()
+                    if (
+                        type(json) is dict
+                        and json.get("event") == "access.data.v2.device.update"
+                    ):
+                        _LOGGER.debug(
+                            "Received update from UniFi Access: %s", json.get("event")
+                        )
+                        self.async_set_updated_data(
+                            await self.hass.async_add_executor_job(self._update_data)
+                        )
+        except Exception as exc:
+            _LOGGER.error("Error in UniFi Access websocket receiver: %s", exc)
+            raise


This should be abstracted away in the library

Agreed, will add it to library. This is here as library as of now is just a generated OpenAPI client; and anything outside the scope of OpenAPI like websocket I've not yet implemented in the library. I'll add some extra custom endpoints for websocket to the library on top of the OpenAPI generated code to resolve this.

My 2 cents, don't use an OpenAPI generated library. Yes it works quick and easy, but it will end you up with an integration that has to make up for the fact that the library is generated.

This is now moved to library.

homeassistant/components/unifi_access/lock.py

joostlek · 2025-01-13T12:34:08Z

homeassistant/components/unifi_access/data.py

+@dataclass
+class UniFiAccessData:
+    """Data structure for UniFi Access integration."""
+
+    api_client: uiaccessclient.ApiClient
+    door_coordinator: UniFiAccessDoorCoordinator


Why do we store the api client here as we can just access it via the coordinator?

The coordinator carries the specific sub-client for the "space" sub-API, and not the general API client. I was thinking I might need to call multiple sub-APIs in some contexts, for example the the "space" and "device" sub-APIs to build support for devices in the entities. But maybe you guys have a structure where this should always come from the coordinators; and thus coordinated entries should never use clients not also used by their coordinator?

joostlek · 2025-01-13T12:34:48Z

homeassistant/components/unifi_access/manifest.json

+{
+  "domain": "unifi_access",
+  "name": "UniFi Access",
+  "codeowners": ["@hagen93"],
+  "config_flow": true,
+  "dependencies": [],
+  "documentation": "https://www.home-assistant.io/integrations/unifi_access",
+  "homekit": {},
+  "iot_class": "local_push",
+  "requirements": ["uiaccessclient==0.9.1"],
+  "ssdp": [],
+  "zeroconf": []
+}


Quality scale should be set to bronze and the quality_scale.yaml should be evaluated

Absolutely. About evaluating the quality scale; do I manually evaluate each criteria, or has this been automated and will be generated by tooling?

A part is automated, but I strongly recommend reading the documentation to see what we do and why we do what we do

hagen93 · 2025-01-13T16:34:48Z

I'm unsure about the preferred procedure for reviews. Should I mark resolved threads myself, or do you prefer reviewers to confirm the resolution? 🙂

joostlek · 2025-01-13T16:50:05Z

I usually have the following:

If you think you solved it correctly, resolve it
If you want to double check that you solved it correctly, as a question and don't
If the comment was a question, don't resolve it
If there is an ongoing discussion without consencus, don't resolve it

hagen93 · 2025-01-13T16:56:13Z

I usually have the following:

* If you think you solved it correctly, resolve it

* If you want to double check that you solved it correctly, as a question and don't

* If the comment was a question, don't resolve it

* If there is an ongoing discussion without consencus, don't resolve it

Great thanks. The ones I've fixed for now is simple typing issues and such, so I'll just resolve them then. 🙂

joostlek · 2025-01-14T13:12:35Z

Please use the Ready for review button instead of requesting reviews directly

hagen93 · 2025-01-14T13:16:41Z

Please use the Ready for review button instead of requesting reviews directly

Oh, so sorry. Not used to your conventions. 🙂

home-assistant bot added cla-needed core has-tests new-integration integration: unifiaccess labels Jan 8, 2025

home-assistant bot previously requested changes Jan 8, 2025

View reviewed changes

home-assistant bot marked this pull request as draft January 8, 2025 21:16

home-assistant bot added quality-scale labels Jan 8, 2025

home-assistant bot added cla-recheck cla-signed and removed cla-recheck cla-needed labels Jan 8, 2025

hagen93 force-pushed the unifiaccess branch from 61236a1 to f3f0497 Compare January 9, 2025 09:56

hagen93 force-pushed the unifiaccess branch 2 times, most recently from bb0284a to 49433c5 Compare January 10, 2025 07:16

hagen93 mentioned this pull request Jan 11, 2025

Add minimal UniFi Access integration home-assistant/home-assistant.io#36883

Open

8 tasks

hagen93 force-pushed the unifiaccess branch from 49433c5 to fa832d0 Compare January 11, 2025 22:55

hagen93 marked this pull request as ready for review January 11, 2025 22:56

joostlek requested changes Jan 13, 2025

View reviewed changes

home-assistant bot marked this pull request as draft January 13, 2025 12:34

hagen93 force-pushed the unifiaccess branch 3 times, most recently from 39881bd to 1ff59d8 Compare January 13, 2025 16:30

hagen93 force-pushed the unifiaccess branch from 1ff59d8 to d3b8379 Compare January 14, 2025 13:05

hagen93 requested a review from joostlek January 14, 2025 13:10

hagen93 force-pushed the unifiaccess branch from d3b8379 to e7e6a11 Compare January 14, 2025 13:17

hagen93 marked this pull request as ready for review January 14, 2025 13:17

Add minimal UniFi Access integration

f6af2ce

hagen93 force-pushed the unifiaccess branch from e7e6a11 to f6af2ce Compare January 24, 2025 21:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add minimal UniFi Access integration #135139

Add minimal UniFi Access integration #135139

hagen93 commented Jan 8, 2025 •

edited

Loading

home-assistant bot left a comment

home-assistant bot commented Jan 8, 2025

hagen93 commented Jan 8, 2025

bdraco commented Jan 9, 2025

andreashagensjolvsagt commented Jan 9, 2025

joostlek Jan 13, 2025

hagen93 Jan 13, 2025

hagen93 Jan 14, 2025

joostlek Jan 13, 2025

hagen93 Jan 13, 2025

hagen93 Jan 14, 2025

joostlek Jan 13, 2025

hagen93 Jan 13, 2025

joostlek Jan 13, 2025

hagen93 Jan 13, 2025

hagen93 Jan 13, 2025

joostlek Jan 13, 2025

hagen93 Jan 13, 2025

joostlek Jan 13, 2025

hagen93 Jan 13, 2025

joostlek Jan 13, 2025

hagen93 Jan 14, 2025

joostlek Jan 13, 2025

hagen93 Jan 14, 2025

joostlek Jan 13, 2025

hagen93 Jan 13, 2025

joostlek Jan 13, 2025

hagen93 commented Jan 13, 2025

joostlek commented Jan 13, 2025

hagen93 commented Jan 13, 2025

joostlek commented Jan 14, 2025

hagen93 commented Jan 14, 2025

Add minimal UniFi Access integration #135139

Are you sure you want to change the base?

Add minimal UniFi Access integration #135139

Conversation

hagen93 commented Jan 8, 2025 • edited Loading

Proposed change

Type of change

Additional information

Checklist

home-assistant bot left a comment

Choose a reason for hiding this comment

home-assistant bot commented Jan 8, 2025

hagen93 commented Jan 8, 2025

bdraco commented Jan 9, 2025

andreashagensjolvsagt commented Jan 9, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hagen93 commented Jan 13, 2025

joostlek commented Jan 13, 2025

hagen93 commented Jan 13, 2025

joostlek commented Jan 14, 2025

hagen93 commented Jan 14, 2025

hagen93 commented Jan 8, 2025 •

edited

Loading