From ac100216eec0c7986e0fa96d214ba671b52be859 Mon Sep 17 00:00:00 2001 From: Julian Pawlowski Date: Sat, 8 Nov 2025 16:50:55 +0000 Subject: [PATCH] refactor: Update attribute naming and ordering for clarity and consistency --- .github/copilot-instructions.md | 186 ++++++++++++++++++ .../tibber_prices/binary_sensor.py | 28 +-- custom_components/tibber_prices/sensor.py | 12 +- 3 files changed, 202 insertions(+), 24 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 63c555d..e5b6d0f 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -218,6 +218,192 @@ Public entry points → direct helpers (call order) → pure utilities. Prefix p **Translation sync:** When updating `/translations/en.json`, update ALL language files (`de.json`, etc.) with same keys (placeholder values OK). +## Attribute Naming Conventions + +Entity attributes exposed to users must be **self-explanatory and descriptive**. Follow these rules to ensure clarity in automations and dashboards: + +### General Principles + +1. **Be Explicit About Context**: Attribute names should indicate what the value represents AND how/where it was calculated +2. **Avoid Ambiguity**: Generic terms like "status", "value", "data" need qualifiers +3. **Show Relationships**: When comparing/calculating, name must show what is compared to what +4. **Consistency First**: Follow established patterns in the codebase + +### Attribute Ordering + +Attributes should follow a **logical priority order** to make the most important information easily accessible in automations and UI: + +**Standard Order Pattern:** + +```python +attributes = { + # 1. Time information (when does this apply?) + "timestamp": ..., # ALWAYS FIRST: Reference time for state/attributes validity + "start": ..., + "end": ..., + "duration_minutes": ..., + + # 2. Core decision attributes (what should I do?) + "level": ..., # Price level (VERY_CHEAP, CHEAP, NORMAL, etc.) + "rating_level": ..., # Price rating (LOW, NORMAL, HIGH) + + # 3. Price statistics (how much does it cost?) + "price_avg": ..., + "price_min": ..., + "price_max": ..., + + # 4. Price differences (optional - how does it compare?) + "price_diff_from_daily_min": ..., + "price_diff_from_daily_min_%": ..., + + # 5. Detail information (additional context) + "hour": ..., + "minute": ..., + "time": ..., + "period_position": ..., + "interval_count": ..., + + # 6. Meta information (technical details) + "periods": [...], # Nested structures last + "intervals": [...], + + # 7. Extended descriptions (always last) + "description": "...", # Short description from custom_translations (always shown) + "long_description": "...", # Detailed explanation from custom_translations (shown when CONF_EXTENDED_DESCRIPTIONS enabled) + "usage_tips": "...", # Usage examples from custom_translations (shown when CONF_EXTENDED_DESCRIPTIONS enabled) +} +``` + +**Critical: The `timestamp` Attribute** + +The `timestamp` attribute **MUST always be first** in every sensor's attributes. It serves as the reference time indicating: + +- **For which interval** the state and attributes are valid +- **Current interval sensors**: Contains `startsAt` of the current 15-minute interval +- **Future/forecast sensors**: Contains `startsAt` of the future interval being calculated +- **Statistical sensors (min/max)**: Contains `startsAt` of the specific interval when the extreme value occurs +- **Statistical sensors (avg)**: Contains start of the day (00:00) since average applies to entire day + +This allows users to verify data freshness and understand temporal context without parsing other attributes. + +**Rationale:** + +- **Time first**: Users need to know when/for which interval the data applies before interpreting values +- **Decisions next**: Core attributes for automation logic (is it cheap/expensive?) +- **Prices after**: Actual values to display or use in calculations +- **Differences optionally**: Contextual comparisons if relevant +- **Details follow**: Supplementary information for deeper analysis +- **Meta last**: Complex nested data and technical information +- **Descriptions always last**: Human-readable help text from `custom_translations/` (must always be defined; `description` always shown, `long_description` and `usage_tips` shown only when user enables `CONF_EXTENDED_DESCRIPTIONS`) + +**In Practice:** + +```python +# ✅ Good: Follows priority order +{ + "timestamp": "2025-11-08T14:00:00+01:00", # ALWAYS first + "start": "2025-11-08T14:00:00+01:00", + "end": "2025-11-08T15:00:00+01:00", + "rating_level": "LOW", + "price_avg": 18.5, + "interval_count": 4, + "intervals": [...] +} + +# ❌ Bad: Random order makes it hard to scan +{ + "intervals": [...], + "interval_count": 4, + "rating_level": "LOW", + "start": "2025-11-08T14:00:00+01:00", + "price_avg": 18.5, + "end": "2025-11-08T15:00:00+01:00" +} +``` + +### Naming Patterns + +**Time-based Attributes:** + +- Use `next_*` for future calculations starting from the next interval (not "future\_\*") +- Use `trailing_*` for backward-looking calculations +- Use `leading_*` for forward-looking calculations +- Always include the time span: `next_3h_avg`, `trailing_24h_max` +- For multi-part periods, be specific: `second_half_6h_avg` (not "later_half") + +**Counting Attributes:** + +- Use singular `_count` for counting items: `interval_count`, `period_count` +- Exception: `intervals_available` is a status indicator (how many are available), not a count of items being processed +- Prefer singular form: `interval_count` over `intervals_count` (the word "count" already implies plurality) + +**Difference/Comparison Attributes:** + +- Use `_diff` suffix (not "difference") +- Always specify what is being compared: `price_diff_from_daily_min`, `second_half_3h_diff_from_current` +- For percentages, use `_diff_%` suffix with underscore: `price_diff_from_max_%` + +**Duration Attributes:** + +- Be specific about scope: `remaining_minutes_in_period` (not "after_interval") +- Pattern: `{remaining/elapsed}_{unit}_in_{scope}` + +**Status/Boolean Attributes:** + +- Use descriptive suffixes: `data_available` (not just "available") +- Qualify generic terms: `data_status` (not just "status") +- Pattern: `{what}_{status_type}` like `tomorrow_data_status` + +**Grouped/Nested Data:** + +- Describe the grouping: `intervals_by_hour` (not just "hours") +- Pattern: `{items}_{grouping_method}` + +**Price-Related Attributes:** + +- Period averages: `period_price_avg` (average across the period) +- Reference comparisons: `period_price_diff_from_daily_min` (period avg vs daily min) +- Interval-specific: `interval_price_diff_from_daily_max` (current interval vs daily max) + +### Examples + +**❌ Bad (Ambiguous):** + +```python +attributes = { + "future_avg_3h": 0.25, # Future when? From when? + "later_half_diff_%": 5.2, # Later than what? Diff from what? + "remaining_minutes": 45, # Remaining in what? + "status": "partial", # Status of what? + "hours": [{...}], # What about hours? + "intervals_count": 12, # Should be singular: interval_count +} +``` + +**✅ Good (Clear):** + +```python +attributes = { + "next_3h_avg": 0.25, # Average of next 3 hours from next interval + "second_half_3h_diff_from_current_%": 5.2, # Second half of 3h window vs current price + "remaining_minutes_in_period": 45, # Minutes remaining in the current period + "data_status": "partial", # Status of data availability + "intervals_by_hour": [{...}], # Intervals grouped by hour + "interval_count": 12, # Number of intervals (singular) +} +``` + +### Before Adding New Attributes + +Ask yourself: + +1. **Would a user understand this without reading documentation?** +2. **Is it clear what time period/scope this refers to?** +3. **If it's a calculation, is it obvious what's being compared/calculated?** +4. **Does it follow existing patterns in the codebase?** + +If the answer to any is "no", make the name more explicit. + ## Common Tasks **Add a new sensor:** diff --git a/custom_components/tibber_prices/binary_sensor.py b/custom_components/tibber_prices/binary_sensor.py index c165dc8..a2eb180 100644 --- a/custom_components/tibber_prices/binary_sensor.py +++ b/custom_components/tibber_prices/binary_sensor.py @@ -188,7 +188,7 @@ class TibberPricesBinarySensor(TibberPricesEntity, BinarySensorEntity): status = "partial" return { "intervals_available": interval_count, - "status": status, + "data_status": status, } def _get_attribute_getter(self) -> Callable | None: @@ -296,7 +296,7 @@ class TibberPricesBinarySensor(TibberPricesEntity, BinarySensorEntity): "minute": period_start.minute if period_start else None, "time": f"{period_start.hour:02d}:{period_start.minute:02d}" if period_start else None, "duration_minutes": duration_minutes, - "remaining_minutes_after_interval": interval_remaining * MINUTES_PER_INTERVAL, + "remaining_minutes_in_period": interval_remaining * MINUTES_PER_INTERVAL, "periods_total": period_count, "periods_remaining": periods_remaining, "period_position": period_idx, @@ -422,30 +422,26 @@ class TibberPricesBinarySensor(TibberPricesEntity, BinarySensorEntity): aggregated_level = original.get("level") aggregated_rating_level = original.get("rating_level") - # Optimized attribute order: time → core decisions → prices → details → meta + # Follow attribute ordering from copilot-instructions.md summary = { - # Time information "start": first.get("period_start"), "end": first.get("period_end"), "duration_minutes": first.get("duration_minutes"), - # Core decision attributes "level": aggregated_level, "rating_level": aggregated_rating_level, - # Price statistics "price_avg": round(sum(prices) / len(prices), 2) if prices else 0, "price_min": round(min(prices), 2) if prices else 0, "price_max": round(max(prices), 2) if prices else 0, - # Detail information "hour": first.get("hour"), "minute": first.get("minute"), "time": first.get("time"), "periods_total": first.get("periods_total"), "periods_remaining": first.get("periods_remaining"), "period_position": first.get("period_position"), - "intervals_count": len(period_intervals), + "interval_count": len(period_intervals), } - # Add price_diff attributes if present (after details) + # Add price_diff attributes if present (price differences step 4) self._add_price_diff_for_period(summary, period_intervals, first) summaries.append(summary) @@ -479,28 +475,24 @@ class TibberPricesBinarySensor(TibberPricesEntity, BinarySensorEntity): break if current_period_summary: - # Build attributes with optimized order: time → core decisions → prices → details → meta + # Follow attribute ordering from copilot-instructions.md attributes = { - # Time information "timestamp": timestamp, "start": current_period_summary.get("start"), "end": current_period_summary.get("end"), "duration_minutes": current_period_summary.get("duration_minutes"), - # Core decision attributes "level": current_period_summary.get("level"), "rating_level": current_period_summary.get("rating_level"), - # Price statistics "price_avg": current_period_summary.get("price_avg"), "price_min": current_period_summary.get("price_min"), "price_max": current_period_summary.get("price_max"), - # Detail information "hour": current_period_summary.get("hour"), "minute": current_period_summary.get("minute"), "time": current_period_summary.get("time"), "periods_total": current_period_summary.get("periods_total"), "periods_remaining": current_period_summary.get("periods_remaining"), "period_position": current_period_summary.get("period_position"), - "intervals_count": current_period_summary.get("intervals_count"), + "interval_count": current_period_summary.get("interval_count"), } # Add period price_diff attributes if present @@ -533,7 +525,7 @@ class TibberPricesBinarySensor(TibberPricesEntity, BinarySensorEntity): attributes["interval_price_diff_from_daily_max"] = current_interval["price_diff_from_max"] attributes["interval_price_diff_from_daily_max_%"] = current_interval.get("price_diff_from_max_%") - # Meta information at the end + # Nested structures last (meta information step 6) attributes["periods"] = periods_summary return attributes @@ -541,14 +533,14 @@ class TibberPricesBinarySensor(TibberPricesEntity, BinarySensorEntity): return { "timestamp": timestamp, "periods": periods_summary, - "intervals_count": len(filtered_result), + "interval_count": len(filtered_result), } # No periods found return { "timestamp": timestamp, "periods": [], - "intervals_count": 0, + "interval_count": 0, } def _add_price_diff_for_period(self, summary: dict, period_intervals: list[dict], first: dict) -> None: diff --git a/custom_components/tibber_prices/sensor.py b/custom_components/tibber_prices/sensor.py index 5ac9395..b8e5434 100644 --- a/custom_components/tibber_prices/sensor.py +++ b/custom_components/tibber_prices/sensor.py @@ -1265,7 +1265,7 @@ class TibberPricesSensor(TibberPricesEntity, SensorEntity): self._trend_attributes = { "timestamp": next_interval_start.isoformat(), f"trend_{hours}h_%": round(diff_pct, 1), - f"future_avg_{hours}h": round(future_avg * 100, 2), + f"next_{hours}h_avg": round(future_avg * 100, 2), "interval_count": hours * 4, "threshold_rising": threshold_rising, "threshold_falling": threshold_falling, @@ -1276,12 +1276,12 @@ class TibberPricesSensor(TibberPricesEntity, SensorEntity): # Get second half average for longer periods later_half_avg = self._calculate_later_half_average(hours, next_interval_start) if later_half_avg is not None: - self._trend_attributes[f"later_half_avg_{hours}h"] = round(later_half_avg * 100, 2) + self._trend_attributes[f"second_half_{hours}h_avg"] = round(later_half_avg * 100, 2) # Calculate incremental change: how much does the later half differ from current? if current_price > 0: later_half_diff = ((later_half_avg - current_price) / current_price) * 100 - self._trend_attributes[f"later_half_diff_{hours}h_%"] = round(later_half_diff, 1) + self._trend_attributes[f"second_half_{hours}h_diff_from_current_%"] = round(later_half_diff, 1) # Cache the trend value for consistency self._cached_trend_value = trend_state @@ -1429,7 +1429,7 @@ class TibberPricesSensor(TibberPricesEntity, SensorEntity): future_prices = self._get_future_prices(max_intervals=MAX_FORECAST_INTERVALS) if not future_prices: attributes["intervals"] = [] - attributes["hours"] = [] + attributes["intervals_by_hour"] = [] attributes["data_available"] = False return @@ -1494,7 +1494,7 @@ class TibberPricesSensor(TibberPricesEntity, SensorEntity): hour_data["avg_rating"] = sum(ratings) / len(ratings) # Convert to list sorted by hour - attributes["hours"] = [hour_data for _, hour_data in sorted(hours.items())] + attributes["intervals_by_hour"] = [hour_data for _, hour_data in sorted(hours.items())] @property def native_value(self) -> float | str | datetime | None: @@ -1776,7 +1776,7 @@ class TibberPricesSensor(TibberPricesEntity, SensorEntity): interval_data = find_price_data_for_interval(price_info, now) attributes["timestamp"] = interval_data["startsAt"] if interval_data else None if hasattr(self, "_last_rating_difference") and self._last_rating_difference is not None: - attributes["difference_" + PERCENTAGE] = self._last_rating_difference + attributes["diff_" + PERCENTAGE] = self._last_rating_difference if hasattr(self, "_last_rating_level") and self._last_rating_level is not None: attributes["level_id"] = self._last_rating_level attributes["level_value"] = PRICE_RATING_MAPPING.get(self._last_rating_level, self._last_rating_level)