Add page count sensors to IPP integration - #165570
Conversation
|
Hey there @ctalkington, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
There was a problem hiding this comment.
Pull request overview
Adds page-count diagnostic sensors to the IPP integration by issuing an additional Get-Printer-Attributes request and exposing relevant counters as Home Assistant sensor entities.
Changes:
- Fetch page count attributes via a separate IPP
GET_PRINTER_ATTRIBUTESrequest and parse both integer and collection-based counter attributes. - Add new diagnostic sensors for pages/impressions/media sheets completed (including monochrome/full-color impressions where available).
- Update fixtures and snapshots to include the new entities, plus add translation strings and icons.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| homeassistant/components/ipp/coordinator.py | Adds page count attribute fetch + parsing and stores results on the coordinator. |
| homeassistant/components/ipp/sensor.py | Defines page count sensor descriptions and entities; conditionally adds them based on fetched attributes. |
| homeassistant/components/ipp/strings.json | Adds names for new sensor translation keys. |
| homeassistant/components/ipp/icons.json | Adds default icons for the new sensors. |
| tests/components/ipp/conftest.py | Extends mocked IPP client to return page-count attributes for tests. |
| tests/components/ipp/snapshots/test_sensor.ambr | Updates sensor snapshots to include the newly created page-count entities. |
You can also share your feedback on Copilot code review. Take the survey.
95dac1d to
49f923b
Compare
49f923b to
70fb173
Compare
There was a problem hiding this comment.
Pull request overview
Adds optional page count sensors to the IPP integration by issuing an additional Get-Printer-Attributes request and exposing any supported counter attributes as diagnostic sensors.
Changes:
- Extend the IPP coordinator to fetch and parse page count attributes (including collection-based HP counters) into
coordinator.page_counts. - Add new sensor entities that are only created when the corresponding IPP attributes are present.
- Update/extend test fixtures, snapshots, and add a regression test ensuring no new sensors are created when unsupported.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| homeassistant/components/ipp/coordinator.py | Adds a second IPP request to fetch page-count-related attributes and parses them into page_counts. |
| homeassistant/components/ipp/sensor.py | Introduces page count sensor descriptions/entities and conditionally sets them up based on available attributes. |
| homeassistant/components/ipp/strings.json | Adds translation strings for new page count sensors. |
| homeassistant/components/ipp/icons.json | Adds default icons for the new page count sensors. |
| tests/components/ipp/conftest.py | Extends the mocked IPP client to return page count attributes for snapshot tests. |
| tests/components/ipp/test_sensor.py | Adds a test ensuring no page count sensors/entities are created when attributes are absent. |
| tests/components/ipp/snapshots/test_sensor.ambr | Updates snapshots to include the newly added page count sensors. |
You can also share your feedback on Copilot code review. Take the survey.
ce5b074 to
3070239
Compare
There was a problem hiding this comment.
Pull request overview
Adds page count sensors to the IPP integration by issuing an additional Get-Printer-Attributes request to retrieve page/impression counters (including HP-style collection attributes) and exposing them as diagnostic sensors.
Changes:
- Extend the IPP coordinator to fetch and parse page count attributes via
ipp.execute(...). - Add new diagnostic sensor entities for page/impression/media-sheet counters (including monochrome/full-color breakdown).
- Update translations/icons and test snapshots; add a test ensuring no sensors are created when counters aren’t exposed.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
homeassistant/components/ipp/coordinator.py |
Adds a second IPP request to fetch page-count attributes and stores parsed results on the coordinator. |
homeassistant/components/ipp/sensor.py |
Defines page-count sensor descriptions/entities and conditionally adds them based on fetched attributes. |
homeassistant/components/ipp/strings.json |
Adds translation keys/names for the new sensors. |
homeassistant/components/ipp/icons.json |
Adds default icons for the new sensors. |
tests/components/ipp/conftest.py |
Extends the IPP mock to return page-count attributes for tests. |
tests/components/ipp/test_sensor.py |
Adds coverage for the “unsupported page counts” case. |
tests/components/ipp/snapshots/test_sensor.ambr |
Updates snapshots to include new entities and states. |
You can also share your feedback on Copilot code review. Take the survey.
There was a problem hiding this comment.
Pull request overview
Adds page count (lifetime counter) sensors to the IPP integration by performing an additional IPP Get-Printer-Attributes request and exposing supported attributes as diagnostic sensors.
Changes:
- Fetch and cache page-count-related IPP attributes via a separate
GET_PRINTER_ATTRIBUTEScall in the coordinator. - Add new diagnostic sensors for total page/impression/media-sheet counts, including HP “collection” counters (mono/color).
- Extend tests/fixtures and snapshots, plus add a negative test to ensure no entities are created when counters are unsupported.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| homeassistant/components/ipp/coordinator.py | Adds periodic page-count attribute fetch via ipp.execute(...) and caches parsed counters. |
| homeassistant/components/ipp/sensor.py | Introduces page-count sensor descriptions and conditionally creates entities based on discovered counters. |
| homeassistant/components/ipp/strings.json | Adds translation names for the new page-count sensors. |
| homeassistant/components/ipp/icons.json | Adds default icons for the new page-count sensors. |
| tests/components/ipp/conftest.py | Extends the IPP client mock to return page-count attributes from execute(). |
| tests/components/ipp/test_sensor.py | Makes disabled-by-default test more robust and adds coverage for “unsupported counters” case. |
| tests/components/ipp/snapshots/test_sensor.ambr | Updates snapshots to include the new page-count sensors. |
|
Triage request: this PR is blocked by the |
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
f6252a0 to
63d7cad
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (3)
homeassistant/components/ipp/coordinator.py:84
- Move the page-counter request and parsing into
pyippbefore adding the Home Assistant entities. Home Assistant integrations are expected to remain thin wrappers and not carry workarounds for client-library limitations; this comment explicitly identifies the raw request as such a workaround, while the referenced upstream PR is still open and the manifest remains pinned topyipp==0.17.0. Merge/release the upstream support, bump the dependency, and consume its typed counter model here instead.
# Page counts are fetched via a separate request for now. Once pyipp PR #715
# (https://github.com/ctalkington/python-ipp/pull/715) is merged, page
# counters will be included in printer.counters by default and this extra
# request can be removed.
tests/components/ipp/test_sensor.py:110
- Cover successful empty or partial responses in the retention test. This test only exercises the exception path, so it does not detect the separate successful-response path that currently rebuilds the counter dictionary and drops previously known values; adding a case such as
{"printers": []}would have caught that regression.
mock_ipp.execute.side_effect = IPPError("boom")
await init_integration.runtime_data.async_refresh()
homeassistant/components/ipp/coordinator.py:110
- Preserve previously fetched counters when a successful response is partial.
page_countsis rebuilt from{}, so an empty or partialprinterspayload drops existing keys and makes already-created sensors becomeunknown; this also contradicts the stated fix in the prior review reply. Start from a copy ofprevious_page_countsand overwrite only values present in this response.
parsed: dict[str, Any] = next(iter(response.get("printers") or []), {})
page_counts: dict[str, int] = {}
|
@copilot Fix the code for all comments in this review thread. When a review comment includes a suggested change, apply the suggestion exactly. Do not make changes beyond what is described in the linked review thread. |
Fetch page count attributes from IPP printers via a separate Get-Printer-Attributes request. Supports both simple integer attributes (printer-impressions-completed, printer-pages-completed, printer-media-sheets-completed) and collection-based attributes (printer-impressions-completed-col with monochrome/full-color sub-values). Sensors are only created when the printer exposes the corresponding attributes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Sort strings.json keys alphabetically to pass prek checks - Preserve previous page_counts on transient fetch failure - Add test for printers that don't support page count attributes Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Merge upstream dev to pick up entity registry changes, regenerate test snapshots, and update tests to look up entities by unique_id instead of hardcoded entity_ids that depend on translation resolution. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Previous snapshot generation was missing translation files, causing entity IDs to be generic numbered suffixes instead of proper names. Generated translations first, then regenerated snapshots. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Refactor _async_update_data to return an IPPData dataclass containing the printer and page_counts, instead of storing page_counts as a separate coordinator attribute. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Use pages/impressions/sheets as translatable units in strings.json so the sensor unit is displayed and can be localized. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Tuples better convey constant intent and prevent accidental mutation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Start page count fetch from previous values so a partial response (printer returning a subset of attributes) doesn't drop known keys - Rename test variable entry -> entity_id for clarity Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Avoids mistyping init_integration as IPPConfigEntry when it is actually a MockConfigEntry. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Separate int-typed attributes from the collection attribute (printer-impressions-completed-col) so each is parsed explicitly rather than using a generic isinstance dispatch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…solidated when pyipp home-assistant#715 is available
- Restore conditional entity creation based on page_counts (fixes broken test) - Broaden exception handling to catch all errors from execute, not just IPPError, to prevent coordinator crashes
- Don't retain previous page counts on successful responses (fixes stale value persistence) - Remove unnecessary __init__ that just forwards to super - Parametrize unsupported printer test to cover empty list and missing printers key cases
…her IPP exceptions
pyipp's parser guarantees collection attributes parse to dicts, so the isinstance check is unnecessary. IPPConnectionError and IPPResponseError are subclasses of IPPError, so catching them separately is redundant. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…page count response in tests Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
44d5efa to
de099b7
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Suppressed comments (2)
homeassistant/components/ipp/coordinator.py:85
- Move the page-counter request and parsing into
pyipp, then consume a released library API here. Home Assistant integrations must remain thin wrappers and must not implement protocol parsing or workarounds for library limitations; upstream PR #715 is still open/blocked and the latest release, 0.17.2, does not provide this API, so this change should wait for the upstream release and update the dependency instead of callingexecute()directly.
# Page counts are fetched via a separate request for now. Once pyipp PR #715
# (https://github.com/ctalkington/python-ipp/pull/715) is merged, page
# counters will be included in printer.counters by default and this extra
# request can be removed.
homeassistant/components/ipp/coordinator.py:82
- Restore the complete Home Assistant pull request template in the PR description. The current Summary/Details/Test plan text omits the required Proposed change, Type of change, Additional information, full Checklist, documentation/dependency checklists, and review-two-PRs checklist; required sections and unchecked boxes must remain present.
# Page counts are fetched via a separate request for now. Once pyipp PR #715
Summary
Get-Printer-AttributesIPP requestprinter-impressions-completed,printer-pages-completed,printer-media-sheets-completed) and collection-based attributes (printer-impressions-completed-colwithmonochrome/full-colorsub-values used by HP printers)Details
The pyipp library doesn't currently request or parse page count attributes. This PR works around that by using
ipp.execute()directly to request these attributes in a separate call. An upstream PR to add native support in pyipp has been opened: ctalkington/python-ipp#715Tested on an HP Color LaserJet Pro MFP 3301 which exposes
printer-impressions-completed-colwith monochrome and full-color sub-values.Test plan
🤖 Generated with Claude Code