BJReplay / ha-solcast-solar

Solcast Integration for Home Assistant
Apache License 2.0
160 stars 31 forks source link

Support site breakdown for detailedForecast #160

Closed andreas-bulling closed 7 hours ago

andreas-bulling commented 1 week ago

I have two systems with two different orientations configured in solcast.

Would be great if the detailedForecast attribute would also support site breakdown so that I can plot both systems separately using apex charts.

BJReplay commented 1 week ago

Ok, so this is, undoubtedly, an enhancement.

I think that it is important to remember that Solcast provides an API that returns exactly the data that you are after.

So, what you are now after is an enhancement to a home assistant integration with the original purpose of showing total forecast solar on the energy dashboard, to repackage an existing API in a new API so that a third API (apex charts) can display it in HA.

That creates a maintenance burden.

Possible? Yes.

Good idea? No.

andreas-bulling commented 1 week ago

?

Just to clarify in case this was unclear: I am talking about two arrays with different orientation within one solcast account. What your documentation calls "site" I believe?!

Other attributes already support site breakdown - according to your documentation. That is, to retrieve e.g. "this hour" and "next hour" values for both arrays.

I am proposing to support the same for the "detailedForecast" property/attribute as this seems to be among the few left that doesn't already support this.

BJReplay commented 1 week ago

I am proposing to support the same for the "detailedForecast" property/attribute as this seems to be among the few left that doesn't already support this.

Feel free to submit a PR for this change.

andreas-bulling commented 1 week ago

I am a user not a programmer. And as a user I'd find this feature helpful. And I could imagine many other users as well. Hence I submitted a feature request.

If you don't like it, or think it's not doable, then fine. Close the issue and move on.

But telling users with ideas for new features to implement them themselves and opening a PR is a bit... well, not helpful.

BJReplay commented 1 week ago

But telling users with ideas for new features to implement them themselves and opening a PR is a bit... well, not helpful.

Telling the authors of a free tool who have already told you that they think your feature request is:

that you think that it is one of the few things missing is failing to understand that the reason why it is missing is precisely because it is

and their suggestion that if you would like it supported you should submit a PR is their way of telling you if you don't like it, fork the repo and implement it yourself is their way of telling you if you think it is easy, show us how, and we'll include your work

No, it's not helpful, but it's more polite that closing without comment, telling you to *&%# off, or deleting the repo.

gcoan commented 1 week ago

I'll chip my comments in, and ignore or take on board as appropriate.

I have two different site orientations (East and West), spread over 3 inverter arrays, and was I think the first one to ask whether it was possible for a breakdown in the forecast generation so I could see the site-level specifics. This led to the site breakdown being added to many sensors and then the estimate10/90 figures for those sensors.

So what do I use?

Having asked for this extra information and having configured it in the integration I find I actually use very little of it.

Probably the most useful is the peak forecast per site and peak forecast time as these I can use to manage clipping that occurs on sunny days, and knowing the different peaks and peak sizes I can control which batteries are charging when. I do use this data.

The per-site forecast power now (50% and 10%) is occasionally useful to check against what I am generating, and how it is on track or not with the forecasts, but I don't use it that often.

And by having the forecast power now I did consider whether I'd like the per-site per half-hour forecast breakdown for the day as requested in this FR, but concluded that it would be a lot of data filling up the database that at the end of the day I would hardly use. A forecast is only a forecast and it's never 100% right, and it isn't something I could take any action on even if I knew it.

So for me I would be interested in this data, would look at it a few times, but I don't think I would want to retain it so would need it as a configurable option to be able to turn off.

autoSteve commented 1 week ago

I have a slightly different opinion on this, although I do concur with @BJReplay regarding the effort required to implement things that hardly anyone will use, and the burden of testing them into the future.

If this does get implemented then it will definitely be disabled by default. It will also require translations into different languages, and require further testing. It will probably be me doing all of this in my spare time for love/like/meh. (Only one integration user has ever sponsored me on GitHub - for five bucks - but that's okay. I have thick skin and use this integration myself, so I want it right.)

My differerence of opinion to BJ?

Expose all information, and let folks choose what to consume.

I would not use what you are asking for @andreas-bulling, but that doesn't mean it should not be available.

Yes, the Solcast API provides the info needed for this use case but there are limited resources for hobbyist accounts, so implementing a REST call as an HA automation would require additional API calls that are already being made by this integration (and an automation could absolutely be done to achieve what you want, and there are examples on the web as to how to do so).

This integration gets Solcast data in a VERY robust way, with back-off retries and validation of data received, doing it in such a way that the Solcast API does not get swamped with requests by thousands of people at once. Implementing an automation doing REST alongside this integration would not do it with elegant retries, and would only raise the pressure on Solcast.

autoSteve commented 1 week ago

And this request gets curlier.

For you, @andreas-bulling I have a proof of concept running. This was logged for each daily forecast sensor while site breakdown for detailed half-hourly and hourly forecasts was enabled... This is a lot of data.

I am investigating whether this can even be suppressed. The integration currently switches off recorder storage of detailedForecast and detailedHourly data, but the sites breakdown adds a curve-ball given the attribute names are not constants.

WARNING (Recorder) [homeassistant.components.recorder.db_schema] State attributes for sensor.solcast_pv_forecast_forecast_today exceed maximum size of 16384 bytes. This can cause database performance issues; Attributes will not be stored
gcoan commented 1 week ago

Don't worry about that warning, we get it on quite a few predbat entities. It's just a warning that recorder can't handle them.

We suppress them for predbat warnings

autoSteve commented 1 week ago

Hey @andreas-bulling.

There is an alpha release at https://github.com/autoSteve/ha-solcast-solar/.

If you want to try it out then head in to HACS.

image

Release notes at https://github.com/autoSteve/ha-solcast-solar/releases/tag/v4.1.7.

I look forward to your feedback.

autoSteve commented 1 week ago

Don't worry about that warning, we get it on quite a few predbat entities.

It may be worth pointing out to the PredBat folks this nugget. Introduced late 2023.

from homeassistant.const import MATCH_ALL

class ExampleEntity(Entity):
    """Implementation of an entity."""

    _unrecorded_attributes = frozenset({MATCH_ALL})

This will automatically remove all entity attributes from recording except device_class, state_class, unit_of_measurement, and friendly_name. No logs will be spammed, recorder will enjoy the silence, and I believe your DB should get smaller...

Even if this enhancement / PR is a bust due to lack of interest (@andreas-bulling? Having issues downloading the alpha release?) I am going to implement this gem for a future release. From my POV, the 🤔😭 part about the implementation is that it can only be declared as a frozen set, so no building names to include/exclude based on variable content in a class constructor.

It is scorched earth as MATCH_ALL, so any attributes that really do want recording should utilise an explicit include or exclude. For Solcast we don't care about any of them, just the sensor states. I'm tipping PredBat is the same.

autoSteve commented 6 days ago

What news, @andreas-bulling? I saw the alpha got downloaded.

springfall2008 commented 2 days ago

Don't worry about that warning, we get it on quite a few predbat entities.

It may be worth pointing out to the PredBat folks this nugget. Introduced late 2023.

from homeassistant.const import MATCH_ALL

class ExampleEntity(Entity):
    """Implementation of an entity."""

    _unrecorded_attributes = frozenset({MATCH_ALL})

Nice but this is for an integration inside HA, not sure if this is supported for the HA Rest API - I will look

autoSteve commented 8 hours ago

@andreas-bulling, I would greatly appreciate an update.

Please do not just request, get your nose out of joint when it is initially questioned, have me mount a counter-opinion to that of @BJReplay, then actually develop the code to satisfy the requested feature, only to then then have you ghost 👻 me. It is insulting.

Testing of software needs to touch many variations of configuration to declare it likely bug free. So far I have just me.

A functional alpha is still available at autoSteve/ha-solcast-solar. It shows two downloads. I am one of them.

autoSteve commented 7 hours ago

Actually, on second thoughts I am going to close this FR due to lack of interest and involvement, @andreas-bulling. You initially closed it, so... re-closing.

I have broader plans for the feature that you have requested. It is going to be implemented and further developed for a potential future release to support what I consider a niche use case. I think I actually have a use for this detail personally as a science experiment mostly, so that motivates me, and I think this may also help a few others to achieve what they might need, or could discover that they want. This goes well beyond wanting to just replicate a chart that you could get over at solcast.com in the HA interface.

So thanks for sewing the idea, and this might see release at some stage.