Skip to content

Copilot/fix technitiumdns entity error #79

Open
tamaygz wants to merge 10 commits into
Atlas-Commons:mainfrom
tamaygz:copilot/fix-technitiumdns-entity-error-again
Open

Copilot/fix technitiumdns entity error #79
tamaygz wants to merge 10 commits into
Atlas-Commons:mainfrom
tamaygz:copilot/fix-technitiumdns-entity-error-again

Conversation

@tamaygz
Copy link
Copy Markdown
Contributor

@tamaygz tamaygz commented May 25, 2026

This pull request improves the robustness and clarity of device hostname handling and logging in both the device tracker and sensor components for TechnitiumDNS. The main changes ensure that hostnames are consistently treated as strings, improve model detection logic, and refine logging for better diagnostics and less noise.

Hostname handling and device model detection:

  • Ensured that hostnames are always treated as strings throughout device tracker and sensor code, preventing type errors and inconsistencies. (custom_components/technitiumdns/device_tracker.py, custom_components/technitiumdns/sensor.py) [1] [2] [3] [4]
  • Optimized device model detection by caching the lowercased hostname, reducing redundant calls to lower() and improving readability. (custom_components/technitiumdns/device_tracker.py, custom_components/technitiumdns/sensor.py) [1] [2]

Logging improvements:

  • Changed log messages regarding missing DHCP data to be less alarming and more informative, clarifying that device trackers and sensors will be created dynamically when data becomes available. (custom_components/technitiumdns/device_tracker.py, custom_components/technitiumdns/sensor.py) [1] [2]
  • Added debug logging and early return when no devices are provided for dynamic sensor creation, preventing unnecessary operations and clarifying behavior. (custom_components/technitiumdns/sensor.py)## Summary

Related issues

@tamaygz tamaygz requested a review from Amateur-God as a code owner May 25, 2026 02:51
@atlas-commons-bot
Copy link
Copy Markdown

Atlas Commons PR review

Requirement Status
DCO sign-off fail
Required checks fail
Summary filled fail
Checklist pass
Linked issue pass

Overall: fail

View PR #79 · Atlas-Commons/home-assistant-technitiumdns

@Atlas-Commons Atlas-Commons deleted a comment from atlas-commons-bot Bot May 27, 2026
@Atlas-Commons Atlas-Commons deleted a comment from atlas-commons-bot Bot May 27, 2026
@Amateur-God
Copy link
Copy Markdown
Member

/atlas llm

@atlas-commons-bot
Copy link
Copy Markdown

Running the LLM code review now.

@atlas-commons-bot
Copy link
Copy Markdown

Atlas Commons code review (LLM)

Summary

This PR hardens hostname handling (string coercion and cached lower() for model detection), reduces log noise when DHCP data is not ready yet, fixes a redundant empty call in the dynamic sensor manager’s else branch, and adds an early return when no devices are passed to sensor creation. The direction matches the reported entity/type errors, but hostname normalization is only applied at a few call sites, and one device_tracker.py hunk may not feed normalized values into lease processing.

Potential issues

  1. Possibly unused hostname normalization in device_tracker.py
    In _async_update_data, the PR normalizes lease.get("hostName", "") into a local hostname variable but the diff does not show that value written into processed_lease["hostname"] (on main, that field comes from lease.host_name). If nothing downstream uses the local variable, this block is dead code and non-string hostnames can still enter trackers until __init__ runs.
hostname = lease.get("hostName", "")
if not isinstance(hostname, str):
    hostname = "" if hostname is None else str(hostname)
# processed_lease still uses lease.host_name / raw value?
  1. __init__ remains the real crash site without ingress normalization
    TechnitiumDNSDeviceTracker.__init__ calls self._hostname.strip() and .replace() before device_info(). The new isinstance guard in __init__ fixes that path, but the same risk exists in sensor.py when building device names from lease.get("hostname", "") and in TechnitiumDHCPDeviceHostnameSensor.native_value (no coercion there). Coercing once when building processed_lease (or a small shared helper) would be more reliable than patching only device_info and tracker init.

  2. str(hostname) on unexpected API types
    The pattern "" if hostname is None else str(hostname) avoids AttributeError on .lower() / .strip(), but malformed API values (e.g. dict/list) become strings like "{'name': 'x'}", which can produce odd device names and misleading model heuristics. Logging at debug when coercion happens (or rejecting non-scalar types) would aid diagnosis.

if not isinstance(hostname, str):
    hostname = "" if hostname is None else str(hostname)
  1. Atlas Commons merge requirements not met
    Automated review reports missing Signed-off-by on all 10 commits, failed required checks, and an empty PR Summary (placeholder text). These are process blockers independent of the code quality.

Suggestions

  • Normalize hostname at data ingress in device_tracker.py when building processed_lease (e.g. coerce lease.host_name / API hostName once and assign to "hostname"), and drop or wire up the unused local variable if the branch still uses dict-style leases.
  • Extract a shared helper (e.g. _normalize_hostname(value) -> str) used from device_tracker.py, sensor.py _create_device_sensors, and TechnitiumDHCPDeviceHostnameSensor.native_value to avoid drift between three copies of the same isinstance logic.
  • Fill in the PR Summary, add Signed-off-by per commit (git rebase / amend), and get required CI green before merge.
  • Consider a small test or fixture with hostname as None, integer, and non-string JSON value to lock in the regression that motivated the PR.

Overall assessment

The changes address real failure modes (notably .lower() / .strip() on non-strings), improve operability by toning down premature warnings, and correctly remove the duplicate _create_sensors_for_devices(new_devices) call in the else branch of _handle_coordinator_update. I would treat this as a useful incremental fix once hostname normalization is applied consistently at lease ingestion (or confirmed the hostName block is wired through), DCO/CI/template requirements are satisfied, and the PR description is completed. The __init__ and device_info guards are worthwhile; they are not quite sufficient on their own for every code path that touches hostname data.


Advisory review only — not a merge approval.

View PR #79 · Atlas-Commons/home-assistant-technitiumdns

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants