Add pvpc electricity prices integration #32092
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #32092 +/- ##
======================================
Coverage ? 94.76%
======================================
Files ? 770
Lines ? 55686
Branches ? 0
======================================
Hits ? 52769
Misses ? 2917
Partials ? 0
Continue to review full report at Codecov.
|
springstan
changed the title
springstan
changed the title
azogue
force-pushed
the
feature/electric-prices-spain
branch
from
c484d39
to
9eadfc2
Compare
Thanks to you for reviewing it, @bdraco :) About the pypi policy, I knew about it, but for this time I didn't consider the possibility, as:
So I thought about the possibility of moving out some logic, but these reasons made me avoid it, as the external package would be just an async method with a lot of arguments, like this: async def download_pvpc_prices(
day: date,
tariff: str,
loop: asyncio.AbstractEventLoop,
websession: aiohttp.ClientSession,
logger: logging.Logger,
timeout: float = 5,
data_source_available: bool = True,
) -> Dict[datetime, float]:
"""
PVPC data extractor.
Make GET request to 'api.esios.ree.es' and extract hourly prices for
the selected tariff from the JSON daily file download
of the official _Spain Electric Network_ (Red Eléctrica Española, REE)
for the _Voluntary Price for Small Consumers_
(Precio Voluntario para el Pequeño Consumidor, PVPC).
Prices are referenced with datetimes in UTC.
"""
url = _RESOURCE.format(day=day)
key = _TARIFF_KEYS[tariff]
try:
with async_timeout.timeout(timeout, loop=loop):
resp = await websession.get(url)
if resp.status < 400:
data = await resp.json()
ts_init = (
datetime.strptime(data["PVPC"][0]["Dia"], "%d/%m/%Y")
.astimezone(_REFERENCE_TZ)
.astimezone(UTC)
)
return {
ts_init
+ timedelta(hours=i): round(
float(values_hour[key].replace(",", ".")) / 1000.0,
_PRECISION
)
for i, values_hour in enumerate(data["PVPC"])
}
except KeyError:
logger.debug("Bad try on getting prices for %s", day)
except asyncio.TimeoutError: # pragma: no cover
if data_source_available:
logger.warning("Timeout error requesting data from '%s'", url)
except aiohttp.ClientError: # pragma: no cover
if data_source_available:
logger.warning("Client error in '%s'", url)
return {}So I discarded the idea. But if it is strictly necessary I have no problems doing it. So, what's your advice? Should I move the method above to its own Another big doubt is with tests. As the extraction logic is here, I added some fixture files to test the availability logic (HA specific) and also the special DST days when the prices array has 23 or 25 values. |
You can pass the
It likely makes sense to use
Ideally, the module provides a class where you pass in the Then you only have to pass the most minimal amount to your data fetch function If you make an additional class .. maybe something like You can do something like This would make it easier to mock in the testing as well as you could pass your fixture though PvPcPriceData.
Its ok to trap the exceptions in HomeAssistant so we raise ConfigEntryNotReady when we get timeouts.
Absolutely. If there are changes in the API, we only have to bump the library instead of going through the PR process. This makes it far more maintainable.
|
|
Hi @bdraco, I'm already on it, and making a Tests could be highly reduced, as all logic is externalized now. I'll push the changes the moment I publish the new repo. If it is not sufficient, I'll move to the coordinator. Thanks again for taking the time to review. |
|
Now I remember why I discarded the coordinator: it works for the usual periodic fetching with a |
No need to use the coordinator if it's not the right fit for what you are doing. |
to add a sensor with the current hourly price of electricity in Spain. Configuration is done by selecting one of the 3 reference tariffs, with 1, 2, or 3 billing periods. * Features config flow, entity registry, RestoreEntity, options flow to change tariff, manual yaml config as integration or sensor platform * Cloud polling sensor with minimal API calls (3/hour at random times) and smart retry; fully async * Only 1 state change / hour (only when the price changes) * At evening, try to download published tomorrow prices, to always store prices info for a window of [3, 27] hours in the future. * Include useful state attributes to program automations to be run at best electric prices. * Add spanish and english translations. * Requires `xmltodict` to parse official xml file with hourly prices for each day.
Instead, create time change listeners in async_added_to_hass and call async_generate_entity_id before async_add_entities
with entity definition as integration and also as a sensor platform
- to deal with days with DST changes - and work with different local timezones
|
Ready for another review pass or still in progress ? |
Ready! |
- Remove PLATFORM_SCHEMA and async_setup_platform - Generate config_entry.unique_id with tariff instead of entity_id, in flow step. - Remove TariffSelectorConfigFlow - Adapt tests to maintain full coverage
and rename SENSOR_SCHEMA to SINGLE_SENSOR_SCHEMA to avoid confusion
- No need for a test_setup now, as platform setup is removed and integration setup is already used in `test_availability` - Simplified `_process_time_step`: only one async_fire(EVENT_TIME_CHANGED)/hour
when source is not available.
|
Hi @bdraco, you can check it again :) |
|
Hi @bdraco
I've been updating them during the review, removing references to:
So I think they're up to date. Are you thinking about something specific? |
and call to async_update after 1st download or when recovering access, so async_write_ha_state is not called twice on those.
|
Looks good, retesting now |
|
|
|
Hi @bdraco, your strange attributes are because of your timezone :) As the hourly prices are downloaded in daily packets localized on Europe/Madrid timezone, when they get re-localized the local hours drift. At evening, prices for the next day are published and the integration downloads them, so at night, this is how it looks:
The only other existing timezone for the locations where the prices apply is 'Atlantic/Canary', for the Canary Islands, which are 1 hour behind the peninsular timezone. For them only one "price last day 23h" appears, corresponding to the 00:00 hour for the main timezone. |
|
Hi @MartinHjelmare, I'll do a small PR asap to address your comments, ok? |
I've been using a custom_component sensor for the hourly price of electricity in Spain for almost 2 years, but after seeing it as a feature request in the forum, I decided to clean and update it up to current standards to propose it as new integration:
pvpc_hourly_pricingDescription
1, 2, or 3 billing periods for each one.
and options flow to change tariffsensor platform entity or bycomponent. (if some of these are deprecated there is no problem to remove those, I just wanted to cover all cases first)Breaking change
Proposed change
Type of change
Example entry for
configuration.yaml:The simplest way of using it is to add it from "Integrations" (can be used up to 3 times to set sensors with different tariffs), but there is also the possibility of manual yaml setup, by component entities.
Additional information
Checklist
black --fast 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..coveragerc.The integration reached or maintains the following Integration Quality Scale:
I hope!
If not, there is no problem to change it until it reaches platinum score :)