Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds a new dynamic tariff provider called tariff_zones that supports simple two-tier (day/night) fixed pricing. This is designed to support zone-based pricing plans like Octopus Energy. The implementation introduces a new tariff provider class, integrates it into the dynamic tariff factory, updates the configuration file with new parameters, and updates documentation.
Changes:
- Added new
Tariff_zonesclass for zone-based tariff pricing with configurable day/night hours and prices - Integrated the new provider into the dynamic tariff factory with configuration validation
- Updated README to mention zone-based pricing options
- Added example configuration parameters for the new tariff_zones provider
- Added a test case for charge calculation logic (though not specific to the new tariff provider)
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| src/batcontrol/dynamictariff/tariffzones.py | New tariff provider implementation with two-tier pricing and wrap-around hour support |
| src/batcontrol/dynamictariff/dynamictariff.py | Factory integration for tariff_zones provider with configuration validation |
| config/batcontrol_config_dummy.yaml | Added example configuration parameters for tariff_zones provider |
| README.MD | Updated prerequisites to mention zone-based pricing options |
| tests/batcontrol/logic/test_default.py | Added duplicate/modified test case for charge calculation |
| def _get_prices_native(self) -> dict[int, float]: | ||
| """Build hourly prices for the next 48 hours, hour-aligned. | ||
|
|
||
| Returns a dict mapping interval index (0 = start of current hour) | ||
| to price (float). | ||
| """ | ||
| raw = self.get_raw_data() | ||
| # allow values from raw data (cache) if present | ||
| tariff_zone_1 = raw.get('tariff_zone_1', self.tariff_zone_1) | ||
| tariff_zone_2 = raw.get('tariff_zone_2', self.tariff_zone_2) | ||
| zone_1_start = int(raw.get('zone_1_start', self.zone_1_start)) | ||
| zone_1_end = int(raw.get('zone_1_end', self.zone_1_end)) | ||
|
|
||
| now = datetime.datetime.now().astimezone(self.timezone) | ||
| # Align to start of current hour | ||
| current_hour_start = now.replace(minute=0, second=0, microsecond=0) | ||
|
|
||
| prices = {} | ||
| # produce next 48 hours | ||
| for rel_hour in range(0, 48): | ||
| ts = current_hour_start + datetime.timedelta(hours=rel_hour) | ||
| h = ts.hour | ||
| if zone_1_start <= zone_1_end: | ||
| is_day = (h >= zone_1_start and h < zone_1_end) | ||
| else: | ||
| # wrap-around (e.g., zone_1_start=20, zone_1_end=6) | ||
| is_day = not (h >= zone_1_end and h < zone_1_start) | ||
|
|
||
| prices[rel_hour] = tariff_zone_1 if is_day else tariff_zone_2 | ||
|
|
||
| logger.debug('tariff_zones: Generated %d hourly prices', len(prices)) | ||
| return prices |
There was a problem hiding this comment.
The new tariff_zones provider lacks comprehensive test coverage. All other tariff providers in this codebase have dedicated test files (e.g., test_tibber.py, test_evcc.py, test_energyforecast.py). A test file should be created at tests/batcontrol/dynamictariff/test_tariffzones.py to verify the tariff calculation logic, especially the wrap-around scenario when zone_1_start is greater than zone_1_end.
| zone_1_start = int(config.get('zone_1_start', 7)) | ||
| zone_1_end = int(config.get('zone_1_end', 22)) | ||
| selected_tariff = Tariff_zones( | ||
| timezone, | ||
| min_time_between_api_calls, | ||
| delay_evaluation_by_seconds, | ||
| target_resolution=target_resolution | ||
| ) | ||
| # store configured values in instance | ||
| selected_tariff.tariff_zone_1 = tariff_zone_1 | ||
| selected_tariff.tariff_zone_2 = tariff_zone_2 | ||
| selected_tariff.zone_1_start = zone_1_start | ||
| selected_tariff.zone_1_end = zone_1_end |
There was a problem hiding this comment.
The tariff_zones provider lacks input validation for zone_1_start and zone_1_end. These values should be validated to ensure they are within the valid hour range (0-23). Invalid values could lead to unexpected behavior in the price calculation logic. Consider adding validation either in the factory method (create_tarif_provider) or in the Tariff_zones init method.
| ) | ||
| # read values and optional price parameters | ||
| tariff_zone_1 = float(config.get('tariff_zone_1')) | ||
| tariff_zone_2 = float(config.get('tariff_zone_2')) |
There was a problem hiding this comment.
The tariff values are extracted as floats, but there's no validation to ensure they are positive numbers or reasonable values. Negative or extremely large tariff values could cause issues in the battery control logic. Consider adding validation to ensure tariff_zone_1 and tariff_zone_2 are positive floats within a reasonable range.
| tariff_zone_2 = float(config.get('tariff_zone_2')) | |
| tariff_zone_2 = float(config.get('tariff_zone_2')) | |
| # validate tariff values to be positive and within a reasonable range | |
| max_allowed_tariff = 10.0 | |
| if not (0.0 < tariff_zone_1 <= max_allowed_tariff): | |
| raise RuntimeError( | |
| '[DynTariff] Invalid tariff_zone_1 value ' | |
| f'({tariff_zone_1}). Expected a positive value ' | |
| f'<= {max_allowed_tariff}.' | |
| ) | |
| if not (0.0 < tariff_zone_2 <= max_allowed_tariff): | |
| raise RuntimeError( | |
| '[DynTariff] Invalid tariff_zone_2 value ' | |
| f'({tariff_zone_2}). Expected a positive value ' | |
| f'<= {max_allowed_tariff}.' | |
| ) |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@OliJue Kannst du bitte das Naming der Klassen an den Python standard anpassen und dir die anderen Kommentare nochmal anschauen? |
|
@MaStr Ich habe einige Vorschläge übernommen, ich hoffe, dass sieht jetzt besser aus. Was ist nicht schaffe / gemacht habe ist ein Error-Handling für die Variablen aus der Config-Yaml und komplette Test cases für. |
|
Danke dir für deine Mühe und Arbeit. ❤️ Das gefällt mir schonmal ziemlich gut. Ich schaue mir das nochmal im Detail an, wenn ich die Tage bisschen Luft habe. Ein bisschen Eingabe-Validierung brauchen wir noch. Ich melde mich |
Currently only for two zones, but already prepared for multiple zones.