Skip to content

Conversation

@offbyone
Copy link

  • added getters for the list of client / autoclient / supported tags resources

Proposed Changes

Adding support for the Client / AutoClient resources in AdGuard Home

The first commit here is to open discussion on structure and naming; if this is desirable in this state, I will add the update operations to it as well.

I don't intend to get super clever with remote ORM stuff here; I only want to have simple "add", "update", "delete" methods for them that refer to the APIs directly.

Related Issues

(Github link to related issues or pull requests)

- added getters for the list of client / autoclient / supported tags resources
- docstrings everywhere
- removed some imports I'd missed
Fixed by using RFC 5737 compliant test range IPs

(as an aside, how dumb is it that the scanner couldn't tell this was
test code?)
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 4 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@github-actions

This comment was marked as resolved.

@github-actions github-actions bot added the stale There has not been activity on this issue or PR for quite some time. label Feb 15, 2023
@offbyone

This comment was marked as resolved.

@github-actions github-actions bot removed the stale There has not been activity on this issue or PR for quite some time. label Feb 16, 2023
@bendalton
Copy link

I'll ping too. Please test/merge this. I'd like to use it upstream with HomeAssistant

@sandervankasteel

This comment was marked as off-topic.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 4 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@frenck
Copy link
Owner

frenck commented Mar 15, 2023

I'll ping too. Please test/merge this. I'd like to use it upstream with HomeAssistant

I hate to bring it to you, but if you goal is to use this in Home Assistant: You can't.

To make this possible, we need an identifier. A request for this is open @ the AdGuard Home team.

So, if that is your only goal, I suggest closing this PR as it has no foreseeing future.

@offbyone
Copy link
Author

Nope, I'm using this for scripting independent of HA.

@frenck
Copy link
Owner

frenck commented Mar 15, 2023

Sorry I now realize I responded to a non-author, sorry about that 👍

../Frenck

@offbyone
Copy link
Author

Okay; I'll add update methods here as well and then update the PR. Thanks.

@github-actions

This comment was marked as resolved.

@github-actions github-actions bot added the stale There has not been activity on this issue or PR for quite some time. label Apr 15, 2023
@offbyone
Copy link
Author

I'm still interested in merging this, just haven't had the time to finish it up.

@github-actions github-actions bot removed the stale There has not been activity on this issue or PR for quite some time. label Apr 17, 2023
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 4 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@offbyone

This comment was marked as off-topic.

@github-actions

This comment was marked as resolved.

@github-actions github-actions bot added the stale There has not been activity on this issue or PR for quite some time. label May 26, 2023
@offbyone
Copy link
Author

I am still interested in merging this, I just need the maintainer to okay it.

@github-actions github-actions bot removed the stale There has not been activity on this issue or PR for quite some time. label May 27, 2023
@github-actions

This comment was marked as resolved.

@github-actions github-actions bot removed the stale There has not been activity on this issue or PR for quite some time. label Oct 31, 2023
@frenck frenck added new-feature New features or options. no-stale This issue or PR is exempted from the stable bot. labels Nov 7, 2023
@offbyone
Copy link
Author

@frenck if you are not interested in maintaining this -- and that's entirely fair! -- how about discussing handing it off? I'm interested and willing.

@frenck
Copy link
Owner

frenck commented Nov 15, 2024

@frenck if you are not interested in maintaining this

I am, sorry. I don't see the use case for Home Assistant in this regard (which is the primary use of this library).

how about discussing handing it off?

Because I haven't merged your itch? 🤷 Sorry, nope.

@offbyone
Copy link
Author

So, for clarity, this isn't a "Python" adguard client, it is a home assistant adguard client library that happens to be written in Python?

@frenck
Copy link
Owner

frenck commented Nov 15, 2024

From the readme:

CleanShot 2024-11-15 at 22 17 22@2x

There is my main use case. Right now, I'm drinking a few beers, so not going to dive in. I can try to make some time tomorrow morning.

../Frenck

@offbyone
Copy link
Author

Mostly I'd like to know if this is considered a general purpose client library for the Adguard API, with features that are usable outside of HA, or if additions will purely be considered in the context of whether they are usable for HA. I've got some decisions to make about how I use it depending on that. The README indicates that home assistant is an example of an integration, not the only one that matters. Can you clarify please?

@frenck frenck changed the title Feature: Client Resources Add client resources Nov 15, 2025
@frenck frenck requested a review from Copilot November 15, 2025 09:05
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for AdGuard Home Client resources, enabling retrieval of automatically discovered clients (AutoClients), manually configured clients (Clients), and supported tags through a new Clients resource facade.

  • Introduces three dataclasses (WhoisInfo, AutoClient, Client) to represent client data structures
  • Adds a Clients resource facade with methods to fetch auto clients, configured clients, and supported tags
  • Integrates the Clients resource into the main AdGuardHome class

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 17 comments.

File Description
src/adguardhome/client.py Implements the new Clients resource facade with dataclasses and methods for retrieving client information from the AdGuard Home API
src/adguardhome/adguardhome.py Adds a clients property to the AdGuardHome class for accessing the Clients resource
tests/test_client.py Provides comprehensive test coverage for the new client resource methods including empty lists, auto clients, and configured clients

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Owner

@frenck frenck left a comment

Choose a reason for hiding this comment

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

Alright, let's move this one forward.

However, the are some issues raised by the Copilot review and by the CI (which is failing).

Could you take a look?

../Frenck

                       

Blogging my personal ramblings at frenck.dev

@frenck frenck marked this pull request as draft November 15, 2025 12:03
Copilot AI review requested due to automatic review settings January 1, 2026 01:09
@offbyone offbyone force-pushed the feat-client-resources branch from 1735801 to cf158ef Compare January 1, 2026 01:13
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 16 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@offbyone offbyone force-pushed the feat-client-resources branch from cf158ef to 49fad81 Compare January 1, 2026 01:19
Copilot AI review requested due to automatic review settings January 1, 2026 01:27
@offbyone offbyone force-pushed the feat-client-resources branch from 49fad81 to a54f118 Compare January 1, 2026 01:27
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

from __future__ import annotations

from dataclasses import dataclass
from typing import TYPE_CHECKING, Any, Mapping
Copy link

Copilot AI Jan 1, 2026

Choose a reason for hiding this comment

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

Import of 'Mapping' is not used.

Suggested change
from typing import TYPE_CHECKING, Any, Mapping
from typing import TYPE_CHECKING, Any

Copilot uses AI. Check for mistakes.
@offbyone offbyone marked this pull request as ready for review January 1, 2026 01:30
- numpy-style comments
- removed ic()
- clearer typing
@offbyone offbyone force-pushed the feat-client-resources branch from a54f118 to 8992269 Compare January 1, 2026 01:30
@offbyone
Copy link
Author

offbyone commented Jan 1, 2026

Just saw this in the morass of notifications; I've addressed the changes. I elected not to rebase on top of main in order to avoid patch churn. Hopefully this works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

new-feature New features or options. no-stale This issue or PR is exempted from the stable bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants