Allow for separate Opower costs and consumption data #132133
Conversation
|
Hey there @tronikos, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
|
Do you see this https://github.com/tronikos/opower/blob/d3a76537984cf02784c366e3004354f577aeb6eb/src/opower/opower.py#L411 in the logs? If yes, should we change the library code to indicate to the caller this happened to avoid inferring this here? |
| @@ -245,6 +329,31 @@ def _update_with_finer_cost_reads( | |||
| break | |||
| cost_reads += finer_cost_reads | |||
|
|
|||
| def _check_is_cost_missing( | |||
There was a problem hiding this comment.
Can this just be:
def _check_is_cost_missing(cost_reads: list[CostRead]) -> bool:
if not cost_reads:
return False
return sum(cost_read.provided_cost for cost_read in cost_reads) == 0And only call it with daily_cost_reads and hourly_cost_reads?
You will also set: cost_reads_billing = cost_reads.copy() right after we fetch bill data.
| @@ -218,7 +302,7 @@ async def _insert_statistics(self) -> None: | |||
|
|
|||
| async def _async_get_cost_reads( | |||
| self, account: Account, time_zone_str: str, start_time: float | None = None | |||
| ) -> list[CostRead]: | |||
| ) -> tuple[list[CostRead], list[CostRead] | None]: | |||
| @@ -156,32 +169,100 @@ async def _insert_statistics(self) -> None: | |||
| cost_sum = cast(float, stats[cost_statistic_id][0]["sum"]) | |||
| consumption_sum = cast(float, stats[consumption_statistic_id][0]["sum"]) | |||
| last_stats_time = stats[consumption_statistic_id][0]["start"] | |||
| if cost_reads_billing: | |||
There was a problem hiding this comment.
The code below is very similar to the code above. Can you avoid duplicate code by extracting it to a function?
| consumption_sum, | ||
| last_stats_time, | ||
| cost_reads, | ||
| [StatisticType.COST, StatisticType.CONSUMPTION] |
There was a problem hiding this comment.
The code is now hard to maintain with all the branching. I feel like it could be simplified if you have 2 lists of cost reads: one for usage, one for cost. For accounts like mine the 2 lists will be the same. For accounts like yours the second list will have the monthly data. Then you can fetch and update statistics in the recorder separately. This unfortunately means for accounts like mine the recorder interactions will now be slower but I think it's better to have less complex code. Could you try that in a separate PR to see how the code would look like?
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
No, PSE doesn't seem to properly return that the cost is missing. It's just 0 unfortunately... |
To clarify, the linked code re-fetches data hitting the usage only endpoint if it gets no data from the cost endpoint and fills in 0 for cost. Can you confirm with a debugger you aren't hitting that code path? Or can you run |
Yup, I just checked it again to be sure, and it's running that if statement, but not entering it. The cost is just being returned as 0 from Opower I believe. |
Proposed change
I'm unsure about other utilities, but PSE only reports costs in the billing view, not in the daily or hourly views. Previously, the finer windows overrode the billing view windows, and costs were set back to 0. This PR attempts to check if the utility has this behavior (fine views have 0 cost), and then fetches and stores the cost data in a separate set of statistics.
Type of change
Additional information
This PR fixes or closes issue: fixes tronikos/opower#56
This PR is related to issue: #99674
Due to the way HomeAssistant handles cost/usage data with mismatched time windows, the energy dashboard will only show costs on the first day of the billing period. The cost shown is the full cost of the entire billing period, despite only showing on that day. All other days show a cost of $0.
Unlike #128964, the data here is presented exactly as returned by the Opower API, with no interpolation. However this may require additional work on the HomeAssistant energy dashboard side to present the data in a meaningful way to the user.
Checklist
ruff format homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all.To help with the load of incoming pull requests: