Skip to content

Commit 478dacb

Browse files
committed
Phase 4: entity translations, a useful diagnostic sensor, diagnostics
P2-1 — has_entity_name was set but each entity then hardcoded an English _attr_name, so a Home Assistant running in another language showed English names. The names move to strings.json under a translation key, which is what core requires. The config and options flows were already translated, so only the entity block was missing. P2-2 — the status sensor reported "Connected" or "Empty", and "Empty" was unreachable: after the first refresh coordinator.data is always a non-empty dict, and before it the entity is unavailable anyway, so "Connected" only restated the availability the entity already reported. It now reports when the cellar last synchronised, which is what tells a user that a six-hourly integration is still alive. It keeps its unique id, so the existing entity is repurposed rather than orphaned, and no second entity appears. Because the old value never changed once running, nothing could have been triggering on a transition — which is what makes the replacement safe rather than breaking. Existing installs keep the entity id sensor.<account>_status; fresh ones get _last_synchronised. P2-6 — its attributes were a fixed API path and a sentence of setup advice, persisted by the recorder on every state write. The README documents the endpoint; the state machine is not the place for docs. P2-3 — diagnostics.py, sequenced after P0-3 deliberately: a diagnostics download is routinely pasted into a public issue, so it is exactly the path that would have turned that latent credential exposure into a live one. Redacts the password and the username, and per bottle the Barcode, Location and Bin. Keeps the column list, which is the signal behind every "no 'iWine' column" report, plus one redacted sample row. P2-4 — the value sensor is MONETARY with state_class TOTAL, so it keeps a long-term statistic, and Home Assistant treats a unit change on an existing statistic as an error. The statistic is worth keeping: cellar value over time is the point of the sensor. So the sensor is unchanged and the *change* is made loud instead — the options form says what will happen beforehand, and a warning names both currencies at the moment the user makes the change and can still act on it. P3-4 — an unrecognised currency became USD silently, labelling a cellar in the wrong currency. Only reachable from legacy entry data, which is where it would least likely be noticed. It now warns and says how to fix it. Recognised codes and mapped legacy symbols stay silent. P3-3 — every sensor class now carries a docstring. The SensorEntity test double now resolves public properties from their _attr_ attributes the way Home Assistant's Entity base does, so these tests read the same surface a real instance would rather than reaching for private attributes. 40 tests added, 270 passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LLPEGFSy3fLEuXNUPAPWR4
1 parent cc5c0ca commit 478dacb

14 files changed

Lines changed: 733 additions & 41 deletions

README.md

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,11 @@ reach that data.
6969
|---|---|---|---|---|
7070
| Total bottles | `142` | `bottles` || `measurement` |
7171
| Total value | `9812.50` | your chosen currency | `monetary` | `total` |
72-
| Status | `Connected` ||| diagnostic |
72+
| Last synchronised | `2026-08-28 09:30:00` || `timestamp` | diagnostic |
73+
74+
Upgrading from 0.0.17 or earlier: the diagnostic entity that reported `Connected` now reports
75+
when the cellar last synchronised. It keeps its entity ID, so nothing has to be repointed. Its old
76+
value never changed once the integration was running, so nothing could have been triggering on it.
7377

7478
### Entity IDs
7579

@@ -78,9 +82,13 @@ The device is named after the account, so entity IDs follow the account name:
7882
```
7983
sensor.<account>_total_bottles
8084
sensor.<account>_total_value
81-
sensor.<account>_status
85+
sensor.<account>_last_synchronised
8286
```
8387

88+
That third ID is what a **fresh install** gets. An install that predates 0.0.18 keeps
89+
`sensor.<account>_status`, because Home Assistant assigns an entity ID once, at first
90+
registration, and never rewrites it. Both point at the same entity; only the name differs.
91+
8492
**If you installed before v0.0.15**, your entity IDs were generated when the entities were first
8593
registered and Home Assistant keeps them — they will still be `sensor.cellartracker_total_bottles`
8694
and friends. Existing dashboards and automations keep working; only the display names change.

custom_components/cellar_tracker/cellar_data.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
from homeassistant.helpers.aiohttp_client import async_get_clientsession
2828
from homeassistant.helpers.json import json_bytes
2929
from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed
30+
from homeassistant.util import dt as dt_util
3031

3132
from .const import (
3233
CONF_CURRENCY,
@@ -228,6 +229,10 @@ def __init__(self, hass: HomeAssistant, entry: ConfigEntry):
228229
# one refresh runs at a time - so the loop can read it without a lock.
229230
self._inventory_body: bytes = b"[]"
230231

232+
# When the cellar last synchronised. None until the first success, so
233+
# the sensor can report "unknown" rather than invent a time.
234+
self._last_success = None
235+
231236
@property
232237
def currency(self) -> str:
233238
"""Return the configured currency symbol."""
@@ -245,6 +250,16 @@ def inventory_body(self) -> bytes:
245250
"""
246251
return self._inventory_body
247252

253+
@property
254+
def last_success(self):
255+
"""When the last poll succeeded, or None if none has yet.
256+
257+
``last_update_success`` says whether the most recent attempt worked;
258+
this says when the data was last actually refreshed, which is what
259+
tells a user that a six-hourly integration is still alive.
260+
"""
261+
return self._last_success
262+
248263
def _backoff_for(self, retry_after: int | None) -> timedelta:
249264
"""How long to wait after being throttled.
250265
@@ -420,6 +435,7 @@ async def _async_update_data(self) -> dict:
420435
# than leaving the coordinator to log a traceback.
421436
raise UpdateFailed(f"Malformed CellarTracker export: {err}") from err
422437

438+
self._last_success = dt_util.utcnow()
423439
self._restore_interval()
424440
return data
425441

custom_components/cellar_tracker/config_flow.py

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -130,23 +130,42 @@ class CellarTrackerOptionsFlowHandler(config_entries.OptionsFlow):
130130

131131
# `self.config_entry` is provided automatically by the base class.
132132

133+
def _current_currency(self) -> str:
134+
return normalize_currency(
135+
self.config_entry.options.get(
136+
CONF_CURRENCY, self.config_entry.data.get(CONF_CURRENCY, DEFAULT_CURRENCY)
137+
)
138+
)
139+
133140
async def async_step_init(self, user_input=None):
134141
"""Manage the options."""
135142
if user_input is not None:
143+
previous = self._current_currency()
136144
user_input[CONF_CURRENCY] = normalize_currency(
137145
user_input.get(CONF_CURRENCY, DEFAULT_CURRENCY)
138146
)
147+
if user_input[CONF_CURRENCY] != previous:
148+
# The cellar value is a long-term statistic, and Home Assistant
149+
# treats a unit change on an existing statistic as an error: it
150+
# logs a mismatch and stops recording until the statistic is
151+
# cleared. Say so here, where the user has just done it and can
152+
# still act, rather than leaving them to find it in the log.
153+
_LOGGER.warning(
154+
"CellarTracker currency changed from %s to %s. This relabels "
155+
"the cellar value rather than converting it, and Home "
156+
"Assistant will refuse to record the value sensor's "
157+
"long-term statistic until its existing statistic is "
158+
"cleared in Developer tools > Statistics.",
159+
previous,
160+
user_input[CONF_CURRENCY],
161+
)
139162
return self.async_create_entry(title="", data=user_input)
140163

141164
current_scan_interval = self.config_entry.options.get(
142165
CONF_SCAN_INTERVAL,
143166
self.config_entry.data.get(CONF_SCAN_INTERVAL, DEFAULT_SCAN_INTERVAL),
144167
)
145-
current_currency = normalize_currency(
146-
self.config_entry.options.get(
147-
CONF_CURRENCY, self.config_entry.data.get(CONF_CURRENCY, DEFAULT_CURRENCY)
148-
)
149-
)
168+
current_currency = self._current_currency()
150169

151170
options_schema = vol.Schema(
152171
{

custom_components/cellar_tracker/const.py

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
"""Constants for the CellarTracker integration."""
22

3+
import logging
4+
5+
_LOGGER = logging.getLogger(__name__)
6+
37
DOMAIN = "cellar_tracker"
48
PLATFORMS = ["sensor"]
59

@@ -73,4 +77,16 @@ def normalize_currency(value: str | None) -> str:
7377
if value_upper in CURRENCY_OPTIONS:
7478
return value_upper
7579

76-
return LEGACY_CURRENCY_MAP.get(value, DEFAULT_CURRENCY)
80+
if value in LEGACY_CURRENCY_MAP:
81+
return LEGACY_CURRENCY_MAP[value]
82+
83+
# Only reachable from entry data written before the flows constrained the
84+
# choice - which is exactly where labelling someone's cellar in the wrong
85+
# currency would go unnoticed.
86+
_LOGGER.warning(
87+
"Unrecognised CellarTracker currency %r; falling back to %s. Set the "
88+
"currency again in the integration options to correct it.",
89+
value,
90+
DEFAULT_CURRENCY,
91+
)
92+
return DEFAULT_CURRENCY
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
"""Diagnostics for the CellarTracker integration.
2+
3+
A diagnostics download is routinely pasted into a public issue, so the question
4+
is not only what would help us but what the user would regret publishing.
5+
6+
Redacted: the password, and the username too - it is half of a credential pair
7+
and names a real CellarTracker account. Per bottle, the Barcode, Location and
8+
Bin, which describe someone's home and are no use in diagnosing a parsing bug.
9+
10+
Kept: the column list, which is the signal that matters. Every "no 'iWine'
11+
column" report comes down to CellarTracker having changed its export, and the
12+
column names are how we see that without asking for a copy of the cellar.
13+
"""
14+
15+
from __future__ import annotations
16+
17+
from typing import Any
18+
19+
from homeassistant.components.diagnostics import async_redact_data
20+
from homeassistant.config_entries import ConfigEntry
21+
from homeassistant.const import CONF_PASSWORD, CONF_USERNAME
22+
from homeassistant.core import HomeAssistant
23+
24+
TO_REDACT = {CONF_PASSWORD, CONF_USERNAME}
25+
26+
BOTTLE_REDACT = {"Barcode", "Location", "Bin"}
27+
28+
29+
async def async_get_config_entry_diagnostics(
30+
hass: HomeAssistant, entry: ConfigEntry
31+
) -> dict[str, Any]:
32+
"""Return redacted diagnostics for a config entry."""
33+
coordinator = entry.runtime_data
34+
data = coordinator.data or {}
35+
bottles = data.get("bottles") or []
36+
37+
return {
38+
"entry": async_redact_data(entry.as_dict(), TO_REDACT),
39+
"coordinator": {
40+
"last_update_success": coordinator.last_update_success,
41+
"last_success": coordinator.last_success,
42+
"update_interval": str(coordinator.update_interval),
43+
"currency": coordinator.currency,
44+
},
45+
"totals": {
46+
"total_bottles": data.get("total_bottles"),
47+
"total_value": data.get("total_value"),
48+
},
49+
# Sorted so two reports can be diffed when a schema change is suspected.
50+
"columns": sorted(bottles[0]) if bottles else [],
51+
# One row is enough to show how the export is shaped. Shipping the whole
52+
# cellar would be both useless and a privacy problem.
53+
"sample_bottle": (
54+
async_redact_data(bottles[0], BOTTLE_REDACT) if bottles else None
55+
),
56+
}

custom_components/cellar_tracker/sensor.py

Lines changed: 36 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -37,20 +37,23 @@ async def async_setup_entry(
3737
sensors = [
3838
TotalBottlesSensor(coordinator, device_info, entry.entry_id),
3939
TotalValueSensor(coordinator, device_info, entry.entry_id, currency),
40-
CellarInventorySensor(coordinator, device_info, entry.entry_id),
40+
CellarLastSyncSensor(coordinator, device_info, entry.entry_id),
4141
]
4242

4343
async_add_entities(sensors)
4444

4545

4646
class TotalBottlesSensor(CoordinatorEntity, SensorEntity):
47+
"""How many bottles the cellar currently holds."""
48+
4749
# Home Assistant composes the friendly name as "<device> <entity>", so the
48-
# entity name must not repeat the integration's own name.
50+
# entity name must not repeat the integration's own name. The name itself
51+
# comes from strings.json via the translation key, not from a literal here.
4952
_attr_has_entity_name = True
53+
_attr_translation_key = "total_bottles"
5054

5155
def __init__(self, coordinator, device_info, entry_id):
5256
super().__init__(coordinator)
53-
self._attr_name = "Total bottles"
5457
self._attr_unique_id = f"{entry_id}_total_bottles"
5558
self._attr_icon = "mdi:bottle-wine"
5659
self._attr_device_info = device_info
@@ -64,11 +67,19 @@ def native_value(self):
6467

6568

6669
class TotalValueSensor(CoordinatorEntity, SensorEntity):
70+
"""What the cellar is worth, in the configured currency.
71+
72+
MONETARY with state_class TOTAL, so Home Assistant keeps a long-term
73+
statistic - cellar value over time being the reason to have the sensor.
74+
That also means the unit cannot change without invalidating the existing
75+
statistic, which is why the options flow warns before letting it happen.
76+
"""
77+
6778
_attr_has_entity_name = True
79+
_attr_translation_key = "total_value"
6880

6981
def __init__(self, coordinator, device_info, entry_id, currency=DEFAULT_CURRENCY):
7082
super().__init__(coordinator)
71-
self._attr_name = "Total value"
7283
self._attr_unique_id = f"{entry_id}_total_value"
7384
self._attr_device_info = device_info
7485
self._attr_device_class = SensorDeviceClass.MONETARY
@@ -81,29 +92,35 @@ def native_value(self):
8192
return (self.coordinator.data or {}).get("total_value", 0.0)
8293

8394

84-
class CellarInventorySensor(CoordinatorEntity, SensorEntity):
85-
"""
86-
Master sensor indicating status.
87-
NOTE: Detailed bottle list is exposed via API, not attributes, to avoid DB crash.
95+
class CellarLastSyncSensor(CoordinatorEntity, SensorEntity):
96+
"""When the cellar last synchronised with CellarTracker.
97+
98+
This replaces a status sensor that reported "Connected" or "Empty". After
99+
the first refresh ``coordinator.data`` is always a non-empty dict - an
100+
empty cellar still yields ``{"total_bottles": 0, ...}`` - and before it the
101+
entity is unavailable anyway, so "Empty" was unreachable and "Connected"
102+
only restated the availability the entity already reports.
103+
104+
It keeps the old unique id, so the existing entity is repurposed rather
105+
than orphaned and a second one is not created. Nothing could have been
106+
triggering on the old value, which never changed.
107+
108+
A timestamp is what a user actually needs from a diagnostic entity here: it
109+
is how you tell that an integration polling every six hours is still alive.
88110
"""
111+
89112
_attr_has_entity_name = True
113+
_attr_translation_key = "last_synchronised"
90114

91115
def __init__(self, coordinator, device_info, entry_id):
92116
super().__init__(coordinator)
93-
self._attr_name = "Status"
94117
self._attr_unique_id = f"{entry_id}_inventory_status"
95-
self._attr_icon = "mdi:api"
118+
self._attr_icon = "mdi:cloud-check-outline"
96119
self._attr_device_info = device_info
120+
self._attr_device_class = SensorDeviceClass.TIMESTAMP
97121
self._attr_entity_category = EntityCategory.DIAGNOSTIC
98122

99123
@property
100124
def native_value(self):
101-
return "Connected" if self.coordinator.data else "Empty"
102-
103-
@property
104-
def extra_state_attributes(self):
105-
# We purposely do NOT include 'bottles' here.
106-
return {
107-
"api_endpoint": "/api/cellartracker/inventory",
108-
"info": "Configure Flex Table Card with 'url: /api/cellartracker/inventory'"
109-
}
125+
"""The last successful refresh, or None if none has happened yet."""
126+
return self.coordinator.last_success

custom_components/cellar_tracker/strings.json

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,21 @@
3737
"data": {
3838
"scan_interval": "Seconds between refreshes",
3939
"currency": "Currency"
40-
}
40+
},
41+
"description": "Changing the currency relabels the cellar value; it does not convert it. Home Assistant records the value as a long-term statistic in its current unit, so after a change you may need to clear that sensor's statistics before it records again."
42+
}
43+
}
44+
},
45+
"entity": {
46+
"sensor": {
47+
"total_bottles": {
48+
"name": "Total bottles"
49+
},
50+
"total_value": {
51+
"name": "Total value"
52+
},
53+
"last_synchronised": {
54+
"name": "Last synchronised"
4155
}
4256
}
4357
}

custom_components/cellar_tracker/translations/en.json

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,21 @@
3737
"data": {
3838
"scan_interval": "Seconds between refreshes",
3939
"currency": "Currency"
40-
}
40+
},
41+
"description": "Changing the currency relabels the cellar value; it does not convert it. Home Assistant records the value as a long-term statistic in its current unit, so after a change you may need to clear that sensor's statistics before it records again."
42+
}
43+
}
44+
},
45+
"entity": {
46+
"sensor": {
47+
"total_bottles": {
48+
"name": "Total bottles"
49+
},
50+
"total_value": {
51+
"name": "Total value"
52+
},
53+
"last_synchronised": {
54+
"name": "Last synchronised"
4155
}
4256
}
4357
}

0 commit comments

Comments
 (0)