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 Traffic Rule switches to UniFi Network #104671

Closed

Conversation

ViViDboarder
Copy link
Contributor

Proposed change

Add switch entities for UniFi Traffic Rules

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:

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:

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.
  • Untested files have been added to .coveragerc.

To help with the load of incoming pull requests:

@home-assistant
Copy link

Hey there @Kane610, mind taking a look at this pull request as it has been labeled with an integration (unifi) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of unifi can trigger bot actions by commenting:

  • @home-assistant close Closes the pull request.
  • @home-assistant rename Awesome new title Renames the pull request.
  • @home-assistant reopen Reopen the pull request.
  • @home-assistant unassign unifi Removes the current integration label and assignees on the pull request, add the integration domain after the command.
  • @home-assistant add-label needs-more-information Add a label (needs-more-information, problem in dependency, problem in custom component) to the pull request.
  • @home-assistant remove-label needs-more-information Remove a label (needs-more-information, problem in dependency, problem in custom component) on the pull request.

@ViViDboarder
Copy link
Contributor Author

@Kane610 this seems to work except that toggling the rule via the UniFi interface doesn't update the switch in Home Assistant. I suspect it's because the switches don't really poll. I'm not sure if everything else so far uses web sockets, but I think I gotta figure that out before marking this ready to review.

@ViViDboarder
Copy link
Contributor Author

It looks like pretty much everything else is able to subscribe to websocket events, but I'm not seeing any logged for Traffic Rules. Possibly for any V2 entities?

@Kane610
Copy link
Member

Kane610 commented Nov 30, 2023

It looks like pretty much everything else is able to subscribe to websocket events, but I'm not seeing any logged for Traffic Rules. Possibly for any V2 entities?

There are no web socket events for V2 APIs, at least not on the subscribed endpoint.

@ViViDboarder
Copy link
Contributor Author

Got it. So I'll need to implement some kind of polling mechanism for the V2 items.

@ViViDboarder
Copy link
Contributor Author

Where I’m at right now is finding a way to conditionally mix in the DataUpdateCoordinator for V2 polling entities. I have come up with a straightforward way to just subclass the UniFi Switch entity class, but I’m considering if there is a way to conditionally inherit via the meta class.

That’s probably more terrible than it’s worth though.

So my plan for now is to create the switch subclass, add a new field to the Description class that can be used as the class factory, then instruct the controller to prefer the subclass indicated by the Description.

Starting with this before meta classes or monkey patching seems wise.

@Kane610
Copy link
Member

Kane610 commented Dec 4, 2023

I think you can do it much easier; as all apihandlers signal updates to subscribers isn't it easier to set up some sort of timed poller that just calls the traffic_rules.update every two minutes?

@Kane610
Copy link
Member

Kane610 commented Jan 31, 2024

Any updates on this?

@ViViDboarder
Copy link
Contributor Author

Sorry. Just had a newborn and haven’t had much time to take a look at implementing the feedback. Got caught up in the weeks before with nursery automation and networking was bumped down on my priority queue. Once things stabilize a bit, I can try to take another look.

@Kane610
Copy link
Member

Kane610 commented Feb 1, 2024

Sorry. Just had a newborn and haven’t had much time to take a look at implementing the feedback. Got caught up in the weeks before with nursery automation and networking was bumped down on my priority queue. Once things stabilize a bit, I can try to take another look.

Congratulations! Take your time

@corvy
Copy link

corvy commented Mar 20, 2024

Hello @ViViDboarder! Thanks for starting on this. :) Just as a FYI I tried this patch, and get the following error when Unifi integration starts.

Logger: homeassistant.setup
Source: setup.py:466
First occurred: 18:10:02 (2 occurrences)
Last logged: 18:20:47

Unable to prepare setup for platform 'unifi.switch': Platform not found (No module named 'homeassistant.components.unifi.controller').

I just patched the switch.py in HA Core, deleted the integration, and added it back in.

My hardware is UDM Pro with Unifi Network 8.1.113.

I am also really looking forward to be able to enable / disable policy based routes from Home Assistant. That would solve a few problems for me. :)

@Kane610
Copy link
Member

Kane610 commented Mar 20, 2024

Hello @ViViDboarder! Thanks for starting on this. :) Just as a FYI I tried this patch, and get the following error when Unifi integration starts.

Logger: homeassistant.setup
Source: setup.py:466
First occurred: 18:10:02 (2 occurrences)
Last logged: 18:20:47
Unable to prepare setup for platform 'unifi.switch': Platform not found (No module named 'homeassistant.components.unifi.controller').

I just patched the switch.py in HA Core, deleted the integration, and added it back in.

My hardware is UDM Pro with Unifi Network 8.1.113.

I am also really looking forward to be able to enable / disable policy based routes from Home Assistant. That would solve a few problems for me. :)

I've been rewriting and renaming stuff in the integration so that's the reason it fails. I think you can get far by just replacing 'controller' with 'hub'

Copy link

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days.
If you are the author of this PR, please leave a comment if you want to keep it open. Also, please rebase your PR onto the latest dev branch to ensure that it's up to date with the latest changes.
Thank you for your contribution!

@github-actions github-actions bot added the stale label May 19, 2024
@Fetochini
Copy link

I would really like to see this addition come to fruition. I think many people could benefit from it. I am a novice, how can I help the project?

@github-actions github-actions bot removed the stale label May 22, 2024
@Kane610
Copy link
Member

Kane610 commented May 31, 2024

I would really like to see this addition come to fruition. I think many people could benefit from it. I am a novice, how can I help the project?

You're welcome to try! It is just to start looking at it and the comments on this issue if there is something you can resolve

@bdowden
Copy link
Contributor

bdowden commented Jun 4, 2024

If anyone else was following this waiting for it to be merged, I forked the code and put up my own PR after addressing some issues. Hopefully we'll get some progress on this!

I have 3 kids myself (and 1 on the way) so I know how time constraining they can be. ViViDboarder did a great job on getting this working.

@sonoseco
Copy link

sonoseco commented Jun 4, 2024

If anyone else was following this waiting for it to be merged, I forked the code and put up my own PR after addressing some issues. Hopefully we'll get some progress on this!

I have 3 kids myself (and 1 on the way) so I know how time constraining they can be. ViViDboarder did a great job on getting this working.

Thank you for making progress on this, I've been following for a while. Hopefully it gets some traction.
Totally understandable that @ViViDboarder got busy, children are literally another full time job!

@ViViDboarder
Copy link
Contributor Author

Closing because #118821 was merged

@home-assistant home-assistant locked as resolved and limited conversation to collaborators Aug 1, 2024
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

6 participants