[fronius] Support setting backup reserved battery capacity

[florian-h05]

Description

This PR adds support to the Fronius Symo inverter Thing actions to set the reserved battery capacity for backup power supply.

Testing

Tested with my Fronius Symo inverter.

[florian-h05]

@jlaur Can I add myself to the CODEOWNERS of the Fronius binding?

[jlaur]

Can I add myself to the CODEOWNERS of the Fronius binding?

It would be nice if @trokohl could confirm/approve this.

@trokohl, I can see you are still active on GitHub, but haven't created PR's in this repository since openHAB 2.4 and last reviewed a PR in 2021 for openHAB 3.2.

Is it okay with you if @florian-h05 is added as maintainer for this binding? Do you want to stay as a maintainer also in this case?

[florian-h05]

It would be nice if @trokohl could confirm/approve this.

Sure 👍
I would suggest to do the CODEOWNERS change in a separate PR if it is approved,
and keep this PR here as is so it can be reviewed soon.

[jlaur]

I would suggest to do the CODEOWNERS change in a separate PR if it is approved

Yes, I also think that would be better.

[florian-h05]

@jlaur Could you please do me a favour and review this soon? Can't wait to use it to optimise my solar production self-consumption ...

[jlaur]

Could you please do me a favour and review this soon?

I'm not sure why you requested a review from me specifically, since I don't remember looking into this binding previously. But I can still try to have a look, sure. 🙂 From a brief look, my first question before going deeper is: Why provide this as a thing action rather than a channel?

Can't wait to use it to optimise my solar production self-consumption

I think you can already by installing the JAR? 😉

[florian-h05]

Sorry, I thought you reviewed my last large Fronius PR, but it was @lsiepel.

Why provide this as a thing action rather than a channel?

I have actually thought about this myself, but came to the conclusion that it is better provided as a thing action as I don‘t get state for this from the inverter (technically this should be possible, but I don‘t think we should spam the config endpoints that are not intended for API consumption with regularly update requests).

I think you can already by installing the JAR? 😉

Yeah, but it‘s always better to have everything in the distro …

[jlaur]

Why provide this as a thing action rather than a channel?

I have actually thought about this myself, but came to the conclusion that it is better provided as a thing action as I don‘t get state for this from the inverter (technically this should be possible, but I don‘t think we should spam the config endpoints that are not intended for API consumption with regularly update requests).

It's perfectly possible to have a write-only channel, although not very common. If you don't want it to have a state at all, even when receiving a command, it could use the veto auto-update policy.

Perhaps a write-only channel could be first step, it should be at least as easy to use as a thing action? That would make it possible to set it from various standard UI's, from the console, etc. And of course still from rules. Looking from the other side, what is the advantage of a thing action compared to a write-only channel?

As said, I don't know the binding well, but looked it up now, and can see it's an inverter with a local API. Do you really think it would be an issue to fetch the initial value when the handler is initialized, and the perhaps poll it with as low frequency as it makes sense? How often would it normally change? Is the channel could (at some point) be made readable as well, I think having it as a channel would be far more convenient than as an action.

Just consider it. I think it's better to have this discussion now than later (and now you probably regret having requested my review by mistake 😄).

[florian-h05]

It's perfectly possible to have a write-only channel, although not very common. If you don't want it to have a state at all, even when receiving a command, it could use the veto auto-update policy.

Feels kind of awkward to be honest.

Looking from the other side, what is the advantage of a thing action compared to a write-only channel?

Having this as a Thing action would be consistent and inline with the other battery control capabilities.

As already said, it is technically possible to read the value, but this value is nothing to be changed by a user on the fly, it’s rather something to be controlled by rules based on solar production forecast. Normally, you set this setting once during installation of the inverter and never touch it again — the reason I want to programmatically modify it from rules!, is that during winter I want to optimise self-consumption by having a lower emergency reserve capacity, but if the forecast is bad, have more backup capacity available. This is nothing that a user would do manually, and as a thing action it is then easier to use from rules than having to link an item to a write only channel.

[lsiepel]

Will review this PR, but:

I think it's better to have this discussion now than later (and now you probably regret having requested my review by mistake 😄).

haha, and as you stareted this discussion, i won't take over, so you might regret bringing this up ;-)

Anyway, ping me when all is set to proceed.

[jlaur]

Having this as a Thing action would be consistent and inline with the other battery control capabilities.

As already said, it is technically possible to read the value, but this value is nothing to be changed by a user on the fly, it’s rather something to be controlled by rules based on solar production forecast. Normally, you set this setting once during installation of the inverter and never touch it again — the reason I want to programmatically modify it from rules!, is that during winter I want to optimise self-consumption by having a lower emergency reserve capacity, but if the forecast is bad, have more backup capacity available. This is nothing that a user would do manually, and as a thing action it is then easier to use from rules than having to link an item to a write only channel.

OK, let's keep this as a Thing action then. @lsiepel - ready to proceed. 😉

[florian-h05]

@jlaur I addressed your review, please have another look.
I think this weird PercentType construction and intValue() order came because I started with one and added the other one later without rechecking if it makes sense that way.

[jlaur]

Thanks, LGTM!