feat: add update_statistics_date service to unstick partial hour statistics - #369
Open
GuyKh wants to merge 1 commit into
Open
feat: add update_statistics_date service to unstick partial hour statistics#369GuyKh wants to merge 1 commit into
GuyKh wants to merge 1 commit into
Conversation
Review Summary by QodoAdd update_statistics_date service to unstick partial hour statistics
WalkthroughsDescription• Add update_statistics_date service to manually advance statistics fetch point • Implement thread-safe statistics operations with asyncio.Lock • Normalize device numbers by stripping leading zeros in statistics processing • Comprehensive validation for target datetime and device existence • Register service with English and Hebrew translations Diagramflowchart LR
A["Service Call<br/>update_statistics_date"] --> B["Validate Input<br/>datetime, device_number"]
B --> C["Check Device<br/>Exists in Contract"]
C --> D["Acquire Lock<br/>_statistics_lock"]
D --> E["Verify Target Date<br/>After Last Stats"]
E --> F["Insert Zero-Value<br/>Statistics Record"]
F --> G["Return Success<br/>with Details"]
File Changes1. custom_components/iec/__init__.py
|
Code Review by Qodo
1. Untyped call in service
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+43
to
+46
| async def handle_update_statistics_date(call) -> None: # noqa: ANN001 | ||
| datetime_str = call.data.get("datetime") | ||
| device_number = call.data.get("device_number") | ||
| result = await iec_coordinator.set_statistics_from_date(datetime_str, device_number) |
There was a problem hiding this comment.
1. Untyped call in service 📘 Rule violation ✓ Correctness
The new handle_update_statistics_date service handler leaves the call parameter untyped (suppressed via # noqa: ANN001), which breaks the requirement for full type hints under strict mypy. This can hide real typing issues for service data access (call.data.get(...)).
Agent Prompt
## Issue description
`handle_update_statistics_date` defines an untyped `call` parameter and suppresses the missing annotation with `# noqa: ANN001`, which violates strict typing requirements.
## Issue Context
This is a Home Assistant service handler; `call` should be typed (typically `homeassistant.core.ServiceCall`) so mypy can type-check service payload access.
## Fix Focus Areas
- custom_components/iec/__init__.py[43-46]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+1259
to
+1265
| new_readings: list[PeriodConsumption] = filter( | ||
| lambda reading: ( | ||
| reading.interval | ||
| >= localize_datetime(datetime.fromtimestamp(last_stat_time)) | ||
| ), | ||
| readings.meter_list[0].period_consumptions, | ||
| ) |
There was a problem hiding this comment.
2. filter() typed as list 📘 Rule violation ✓ Correctness
new_readings is annotated as list[PeriodConsumption] but is assigned the result of filter(...), which is an iterator, not a list. This is a strict mypy incompatibility and can lead to incorrect assumptions about the variable's behavior.
Agent Prompt
## Issue description
`new_readings` is annotated as `list[PeriodConsumption]` but `filter(...)` returns an iterator, causing a mypy strict type error.
## Issue Context
`itertools.groupby(...)` works with any iterable, so `new_readings` does not need to be a list unless later logic depends on list operations.
## Fix Focus Areas
- custom_components/iec/coordinator.py[1259-1269]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+1252
to
1257
| _LOGGER.debug( | ||
| f"[IEC Statistics] Last Consumption Sum for C[{contract_id}] D[{device.device_number}]: {consumption_sum}" | ||
| ) | ||
| _LOGGER.debug( | ||
| f"[IEC Statistics] Last statistics are from {from_date.strftime('%Y-%m-%d %H:%M:%S')}" | ||
| f"[IEC Statistics] Last Estimated Cost Sum for C[{contract_id}] D[{device.device_number}]: {cost_sum}" | ||
| ) |
There was a problem hiding this comment.
3. F-strings used in logging 📘 Rule violation ⛯ Reliability
New _LOGGER.debug(...) calls use f-strings, which commonly violates Home Assistant’s Ruff logging-format rules (and will be flagged by ruff check under HA defaults). This can block CI and formatting checks.
Agent Prompt
## Issue description
Several new `_LOGGER.debug(...)` calls use f-strings, which can violate Ruff logging rules used by Home Assistant.
## Issue Context
Use lazy formatting: `_LOGGER.debug("... %s", value)` to avoid eager string interpolation and satisfy lint rules.
## Fix Focus Areas
- custom_components/iec/coordinator.py[1252-1257]
- custom_components/iec/coordinator.py[1282-1290]
- custom_components/iec/coordinator.py[1331-1340]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+43
to
+47
| async def handle_update_statistics_date(call) -> None: # noqa: ANN001 | ||
| datetime_str = call.data.get("datetime") | ||
| device_number = call.data.get("device_number") | ||
| result = await iec_coordinator.set_statistics_from_date(datetime_str, device_number) | ||
| _LOGGER.info("update_statistics_date result: %s", result) |
There was a problem hiding this comment.
4. Service call can crash 🐞 Bug ⛯ Reliability
update_statistics_date passes call.data.get(...) values directly into set_statistics_from_date; missing or non-string fields cause int(None) / datetime.fromisoformat(None) to raise TypeError, which is not caught and will fail the service call.
Agent Prompt
### Issue description
`update_statistics_date` can crash with an uncaught `TypeError` when `datetime` or `device_number` are missing or not strings, because the handler passes `None` through and `set_statistics_from_date` only catches `ValueError`.
### Issue Context
The service is registered without a validation schema, and the handler uses `dict.get`, which returns `None` for missing keys.
### Fix Focus Areas
- custom_components/iec/__init__.py[43-52]
- custom_components/iec/coordinator.py[1348-1362]
### Implementation notes
- Add explicit `if not datetime_str or not device_number: ...` checks (and ensure they are strings).
- Update exception handling to `except (TypeError, ValueError):` for both the `int(...)` and `fromisoformat(...)` conversions.
- Consider registering the service with a schema (e.g., `vol.Schema`) to enforce required fields and types.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+1384
to
+1387
| for contract_id in self._contract_ids: | ||
| devices = await self._get_devices_by_contract_id(contract_id) | ||
| for device in devices: | ||
| if str(int(device.device_number)) == device_number: |
There was a problem hiding this comment.
5. Device list none crash 🐞 Bug ⛯ Reliability
set_statistics_from_date iterates for device in devices without checking devices is iterable; _get_devices_by_contract_id can return None on IEC API errors, causing a TypeError and aborting the service.
Agent Prompt
### Issue description
`set_statistics_from_date` assumes `_get_devices_by_contract_id` always returns an iterable list, but it can return `None` when the IEC API call fails, causing `TypeError: 'NoneType' object is not iterable`.
### Issue Context
`_get_devices_by_contract_id` currently logs on `IECError` but does not set a fallback value, so it may return `None`.
### Fix Focus Areas
- custom_components/iec/coordinator.py[312-323]
- custom_components/iec/coordinator.py[1384-1392]
### Implementation notes
- Option A (preferred): In `_get_devices_by_contract_id`, return `[]` on exception / when no cached devices exist.
- Option B: In `set_statistics_from_date`, add `if not devices: continue` or return a structured error indicating device list could not be fetched.
- Ensure type annotations match behavior (`list[Device]` should not return `None`).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
GuyKh
force-pushed
the
feature/update-statistics-date
branch
2 times, most recently
from
March 15, 2026 13:58
04b7e27 to
90ae86d
Compare
…istics When the IEC API returns partial data for an hour (e.g., 23:00 exists but 23:15/23:30/23:45 are missing), the statistics fetch gets stuck because _insert_statistics skips hours with fewer than 4 readings. This commit adds an `update_statistics_date` service that inserts a zero-value statistic record at a target hour, naturally advancing the DB's "last statistics" point so the next refresh continues from there. - Add asyncio.Lock to protect concurrent statistics operations - Wrap _insert_statistics with the lock - Add set_statistics_from_date method with full validation - Normalize device_number by stripping leading zeros - Register service, services.yaml, and translations (en/he) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
GuyKh
force-pushed
the
feature/update-statistics-date
branch
from
April 6, 2026 08:40
90ae86d to
f212945
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When the IEC API returns partial data for an hour (e.g., 23:00 exists but 23:15/23:30/23:45 are missing), the statistics fetch gets stuck because
_insert_statisticsskips hours with fewer than 4 readings. The next refresh then re-attempts the same hour indefinitely.This PR adds an
update_statistics_dateservice that inserts a zero-value statistic record at a target hour, naturally advancing the DB's "last statistics" point so the next refresh continues from there.Changes
asyncio.Lock(_statistics_lock) to protect concurrent statistics operations, wrapped_insert_statisticswith the lock, addedset_statistics_from_datemethod with full validation, normalizeddevice_numberby stripping leading zerosupdate_statistics_dateserviceValidation
The service validates:
Test Plan
hass.services.async_call("iec", "update_statistics_date", {"datetime": "2024-01-16T01:00:00", "device_number": "12345"})_insert_statisticscycle picks up from the new hour🤖 Generated with Claude Code