Better Support for Solarflow Batteries without Cloud

[vaterlangen]

This MR adds a new battery provider (index 7) to support Zendure Solarflow batteries using local MQTT broker.

Preconditions

In order to use this integration, the Solarflow system has to be "disconnected from cloud" (aka. configured to use the local MQTT broker instead of the pre configured vendor cloud). Please refer https://github.com/reinhard-brandstaedter/solarflow-bt-manager for details. If one can adjust answers on DNS queries (e.g. using OpenWRT as router), Solarflow can also be "disconnected from cloud" by returning the local broker IP for requests to mqtteu.zen-iot.com. The broker should be configured without authentication - or one has to gather the username passoword used by Solarflow.

Features

• Query battery status from Solarflow
• Display stats on LiveView
• Publish stats via MQTT
• HASS auto-discovery
• Auto detection of AB1000 and AB2000 packs (currently only tested with my AB2000)
• Time-sync for Solarflow
• Sending settings to Solarflow
• Set Zendure output limit fixed or via schedule (relative to sunrise/fall)
• Configurable full charge intervals for battery (or better: BMS) conditioning

Main Page with battery data read from Solarflow

Configuration of new battery provider

Testing

Tested with my local setup consisting of

• 1x HUB1200 / HUB2000
• 2x AB2000
• 1x HMT-2250
• Mosquitto as local MQTT broker

[vaterlangen]

Removed accidentally pushed stuff from branch

[AndreasBoehm]

I am wondering why we need this PR if there is already support for Zendure Batteries after this PR (#1127) has been merged? Or is that a different way to setup the battery?

[vaterlangen]

With this PR, all battery data is read and displayed (as shown in the screenshot) - not only voltage and soc as implemented by #1127.It iis intended for Solarflow instances, that are not connected to the vendor cloud (but using local MQTT broker, as this requires sending some commands to the device, too) and not using the vendor App. On the settings page, one can already adjust some parameters, as maximum output power and soc limits.

As I discovered a better way to read data from the device, I'm currently reworking the code.

[vaterlangen]

Initial post completely updated and ready for review

[schlimmchen]

Let's get this ready, shall we?

I took the liberty to clean this PR up. Since we let it stew for so long, @vaterlangen had to merge hoylabs/development several times. Rebasing is now impossible, but merging is not an option.

git diff hoylabs/development vaterlangen/feature/zendure_solarflow | git apply -

That command is what I used and I have very high confidence in git that it did exactly what I wanted.

However, I am unable to force-push to vaterlangen/feature/zendure_solarflow, even though maintainers are supposed to be allowed to edit this PR and even though I was able to edit the branch before, around 3 months ago. @vaterlangen Did you do something to prevent me from changing this branch again?

I then started a more in-depth review. There are some oddities:

• std::optional<std::shared_ptr<>> does not make any sense to me. Use nullptr if no object can be returned.
• You seem to be eager to use move semantics to avoid copying stuff, to a point that makes we wonder "does this even improve anything?", but then there are lines that obviously can be improved regarding the same thing.
• Please don't add dead code, i.e., commented-out code. Ah, I see you want to make values auto-discoverable for all packs, but packs are a volatile thing... Well... Let's sleep one more night over this. Definitily getPackDataList() must return a const&, not a copy.
• You often (always?) are missing a space between a closing bracket and an opening curly brace... This is bugging me hard^^
• You use structured binding, something new to me. Nice to learn about this, I have not had the opportunity to catch up on C++17 stuff, let alone C++23. However, then you proceed to use only the index to retrieve the element you already have as entry, which by the way is a copy. The problem with finding stuff like this is the following: You used a concept that makes sense in that context, but you obviously have not fully understood them, but applied them anyways. That in turn makes me doubt the rest of the code... I see why you did this: you want to modify entry, which did not work, because your structured binding expression yields a copy.
• The readability of your code can be significantly improved by un-indenting your code

I started refactoring/updating the code to bring it to a level that I am happy to merge. I did not push that, yet, and will continue shortly.

[vaterlangen]

@schlimmchen Thanks for driving this forward!

No, I did not change anything on the repo settings. I just disabled and re-enabled maintainer edits. Please try again.

Thanks for your comments:

• oh, yes. That's right - optional is just generating overhead...
• that was the same I thought...but not coming with a solution
• Sorry for bugging you, that was not intended
• The loop was originally designed using both, index and object - as it is now using the object only, this can be done by a range-based loop
• And again, you're right. "Some" spots could be optimized this way

Thanks for cleaning my mess up...

I also addressed some points an pushed them to feature/zendure_solarflow_upd - just in case I can reduce your work by this