[energidataservice] Add support for hourly spot prices as calculated average - #19553
Conversation
ee02cd5 to
99ba903
Compare
5f870b1 to
eebba21
Compare
ba419b0 to
fe38d90
Compare
fe38d90 to
df2b070
Compare
There was a problem hiding this comment.
Pull request overview
Adds an optional Thing-level configuration to expose spot prices as an hourly average (derived from quarter-hourly values), affecting both the spot-price channel and price calculations performed by Thing actions.
Changes:
- Introduces a new Thing parameter
hourlySpotPrices(UI config + i18n + README). - Extends
SpotPriceSubscriptionto include an hourly-average variant. - Updates
ElectricityPriceProviderand caches to support recalculating and serving hourly-average spot prices.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| bundles/org.openhab.binding.energidataservice/src/main/resources/OH-INF/i18n/energidataservice.properties | Adds i18n label/description for the new hourlySpotPrices Thing parameter. |
| bundles/org.openhab.binding.energidataservice/src/main/resources/OH-INF/config/service.xml | Adds the hourlySpotPrices boolean Thing configuration parameter. |
| bundles/org.openhab.binding.energidataservice/src/main/java/org/openhab/binding/energidataservice/internal/provider/subscription/SpotPriceSubscription.java | Adds hourly-average flag to distinguish subscription variants. |
| bundles/org.openhab.binding.energidataservice/src/main/java/org/openhab/binding/energidataservice/internal/provider/cache/SubscriptionDataCache.java | Extends cache API to support replacing cached key/value pairs. |
| bundles/org.openhab.binding.energidataservice/src/main/java/org/openhab/binding/energidataservice/internal/provider/cache/ElectricityPriceSubscriptionCache.java | Implements replacing the internal cached price map. |
| bundles/org.openhab.binding.energidataservice/src/main/java/org/openhab/binding/energidataservice/internal/provider/ElectricityPriceProvider.java | Adds hourly-average recalculation logic and cache dependency handling. |
| bundles/org.openhab.binding.energidataservice/src/main/java/org/openhab/binding/energidataservice/internal/handler/EnergiDataServiceHandler.java | Creates spot-price subscriptions using the new Thing config flag. |
| bundles/org.openhab.binding.energidataservice/src/main/java/org/openhab/binding/energidataservice/internal/config/EnergiDataServiceConfiguration.java | Adds new config field mapping hourlySpotPrices. |
| bundles/org.openhab.binding.energidataservice/README.md | Documents the new Thing configuration parameter. |
Comments suppressed due to low confidence (1)
bundles/org.openhab.binding.energidataservice/src/main/java/org/openhab/binding/energidataservice/internal/provider/subscription/SpotPriceSubscription.java:54
hashCode()still only hashespriceAreaandcurrencyeven though equality now also depends onhourlyAverage. Consider includinghourlyAverageinhashCode()as well to keep it aligned withequals()and avoid unnecessary hash collisions in maps/sets keyed by this subscription.
@Override
public int hashCode() {
return Objects.hash(priceArea, currency);
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
df2b070 to
64139e9
Compare
|
@jlaur if not mistaken you liked to have this into 5.2. Ping when you are ready for a final review. |
Thanks, I should try to prioritize this soon. The main reason why it has been "work in progress" for so long is that the current cache implementation is not prepared for this change. In the current implementation, the calculated hourly prices are added as a "sidecar", but this complicates things a bit, so I already started a rewrite. Secondly, Copilot found a real issue with possible loss of events. |
c3dccbd to
0bc8e4f
Compare
OK, delegated to Claude, result looks good. Will let it run until tomorrow's day-ahead prices becomes available, then I should be able to confirm and publish the PR in its current form (without major cache refactoring). |
|
@lsiepel - this is now ready for review. I don't know when I'll find the time to rewrite the cache, but in any case that will have to be in another PR. The current "sidecar" approach has been tested for quite a while now, and the event issue is fixed as well, and has been successfully tested now for some days. |
Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
0bc8e4f to
136b79a
Compare
Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk> Signed-off-by: olemr <olemr@olemr.com>
Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk> Signed-off-by: Ciprian Pascu <contact@ciprianpascu.ro>
This is configurable on Thing level, and will affect the
spot-pricechannel as well as calculated results from Thing actions.