[solarforecast] Initial contribution

[weymann]

Solar Forecast binding for two diferent free photovoltaic forecast services

• forecast.solar
• Solcast Hobbiest Plan

Binding is provided on Marketplace.

The binding is running pretty stable on my side but I need your opinion regarding the Actions interface
Is LocalDateTime'sufficient or is ZonedDateTime the way to go?

[jlaur]

@weymann - thanks for this contribution! I have now slightly started a review. Would it be possible for you to do one self-review iteration with #13044 in mind to avoid some redundancy? For example i18n for updateStatus(), usage of Instant over LocalDateTime to avoid clock issues, etc.

[weymann]

I'll do as you suggested. Was developed in parallel to Mercedes binding so I can imagine some similar findings.
Put "WIP" in headline and I'll remove it as soon as I'm done.

[weymann]

WIP removed

• Logging adapted
• i18n for status upates and rule actions updated
• usage of Instant for internal measurements
• package rework

[jlaur]

Thank you for your contribution, and sorry for the long delay! I almost made it for the one year anniversary. I have provided my feedback for my first iteration review.

The binding is running pretty stable on my side but I need your opinion regarding the Actions interface
Is LocalDateTime'sufficient or is ZonedDateTime the way to go?

I would actually suggest Instant, but I might be alone with this opinion. LocalDateTime seems not ideal since there can be ambiguity, for example during DST. It can suddenly change when time-zone settings are changed. Therefore ZonedDateTime would be better. The reason I'm proposing Instant is because you actually don't need the time-zone to be provided, only the exact moment in time. You can always get the current time-zone through TimeZoneProvider.

Historically ZonedDateTime is better supported, for example it's currently the native type for DateTimeType. However, Instant has also recently gotten some attention in openhab-js. See also openhab/openhab-core#2898

[weymann]

@jlaur
I looked into the discussion regarding Instant or ZonedDateTime as base for DateTimeType. Deep discussion regarding compatibility and so on.

Switch now perspective to an openHAB user with no deep technical background. And we want to attract new users, right?
The user wants to know some "simple" values

• energy production of tomorrow: LocalDate.now.plusDays(1)
• power prediction one hour ahead: LocalDateTime.now.plusHours(1)
• power prediction at 16:13: LocalDateTime:now.withHour(16).withMinute(13)

In my opinion I can very easily explain the Interface with plusX, withX, minusX functions on LocalDateTime.
With Instant it would look very different.

[jlaur]

I looked into the discussion regarding Instant or ZonedDateTime as base for DateTimeType. Deep discussion regarding compatibility and so on.

Switch now perspective to an openHAB user with no deep technical background. And we want to attract new users, right? The user wants to know some "simple" values

• energy production of tomorrow: LocalDate.now.plusDays(1)
• power prediction one hour ahead: LocalDateTime.now.plusHours(1)
• power prediction at 16:13: LocalDateTime:now.withHour(16).withMinute(13)

All of those classes have their reasons to exist, we just need to use the right classes for the right purposes. First, let's not mix dates and datetimes.

I'm not sure exactly what you want to discuss here, since there are many subtopics. But if we focus on what I assume is thing action examples/rule code: What exactly do you expect from LocalDateTime.now.plusHours(1)? This will use the system default time-zone, not the time-zone configured in openHAB. See public static LocalDateTime now(). So when using an openHAB system in another time-zone, this will simply not work, since TimeZoneProvider is not considered.

How will you, using LocalDateTime, manage ambiguous moments in time when switching from DST to non-DST, i.e. when offsets change in a backward direction?

In my opinion I can very easily explain the Interface with plusX, withX, minusX functions on LocalDateTime. With Instant it would look very different.

Instant also have public Instant plus(long amountToAdd, TemporalUnit unit)

[weymann]

After investigating Energi Data Service binding usage of Instant is clear for me now.

• actions interface changed also to Instant usage
• first internal tests running fine
• release candidate updated in community / marketplace page

[openhab-bot]

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/solar-forecast-pv/137681/123

[mstormi]

@jlaur Jacob do you mind finishing this review so we could still get this into 4.1? Thanks.

[jlaur]

@weymann - I checked for open comments the other day and found one that I had missed previously. Can you recheck this? #13308 (comment)

[soenkekueper]

Now my bridge goes online, but my two planes stay offline with COMMUNICATION_ERROR and HTTP Status Code 401.

Yes, known issue. Last 2 commits contains fixes after testing the version on my side. Bote services are up and running now.

Hey,

thanks for the fast fixing.
I've just installed the latest version
336 │ Active │ 80 │ 4.2.0.202404281527 │ openHAB Add-ons :: Bundles :: SolarForecast Binding

Doesn't work yet :/
The Bridge goes offline and states:
Exception during update: solarforecast:sc-plane:1412a57284:a7bb1eafb0 # Forecast invalid time range
so the two defined planes stay offline:bridge.

The log states:

2024-04-28 19:36:53.048 [TRACE] [.solcast.handler.SolcastPlaneHandler] - Get new forecast Expiration: 2024-04-28T17:36:23.504336188Z, Data: {}
2024-04-28 19:36:55.343 [DEBUG] [.solcast.handler.SolcastPlaneHandler] - Solcast Hauptdach Call https://api.solcast.com.au/rooftop_sites/d4dc-xxxx-yyyy-e4fa/estimated_actuals?format=json failed 403
2024-04-28 19:36:55.366 [TRACE] [.solcast.handler.SolcastPlaneHandler] - Get new forecast Expiration: 2024-04-28T17:36:44.903449277Z, Data: {}
2024-04-28 19:36:57.252 [DEBUG] [.solcast.handler.SolcastPlaneHandler] - Solcast Anbau Call https://api.solcast.com.au/rooftop_sites/b243-xxxx-yyyy-09bf/estimated_actuals?format=json failed 403

Is it right, that the expiration time is 30 seconds past the current time? Which timestamp is ment here?

So should i continue testing at the moment or do you have some further known issues, you're working on?

BR Sönke

[jlaur]

@weymann - we are getting close. Two comments still open from this: #13308 (review). There are also some compiler warnings to address. And I'm wondering if this needs to be investigated/resolved before merging: #13308 (comment)?

[weymann]

Solcast Hauptdach Call https://api.solcast.com.au/rooftop_sites/d4dc-xxxx-yyyy-e4fa/estimated_actuals?format=json failed 403

Did you register for Hobbyist Plan and is it verified by Solcast ?
As you can see in the response table return code 403 says that you're not allowed to access this API.

[soenkekueper]

Yes i've a valid registration and the plugin was running some time ago and i did not change the configuration.

So to be sure, i've just generated a new api-key, now its running fine.

Thanks for your work!

[weymann]

to address. And I'm wondering if this needs to be investigated/resolved before merging: #13308 (comment)?

This was config issue, not code. Reported as fixed in latest comment.

Issue weymann/OH3-SolarForecast-Drops#5 (comment) was reported in my drop repository which needs to be fixed

[jlaur]

Some minor nitpicking. Let me know when weymann/OH3-SolarForecast-Drops#5 is fixed and you are ready for merging the PR.

[weymann]

Some minor nitpicking. Let me know when weymann/OH3-SolarForecast-Drops#5 is fixed and you are ready for merging the PR.

Tested commit on my site 3 days with result ok
Reporter of this issue tested also the change with result ok and closed the issue as solved

Ready to merge!

[jlaur]

Thanks again for the contribution! LGTM.

You can now add your binding's logo to the openHAB website and the UI. See https://next.openhab.org/docs/developer/addons/#add-your-add-on-s-logo-to-the-openhab-website-and-the-ui