search

hoylabs / OpenDTU-OnBattery

Software for ESP32 to talk to Hoymiles/TSUN/Solenso Inverters, VE.Direct devices, battery management systems, and related peripherals
GNU General Public License v2.0
301 stars 63 forks source link

Jbd bms support #1218

Open MoleBre opened 1 month ago

MoleBre commented 1 month ago

First of all: also my thank to all the developers. You are doing a great job. This project is awesome. Please be patient with my skills. Its my first time contributing to a project on github and it took me some time to understand the code more or less :-).

I copied the the jk-bms code and adapted it to the jbd-bms uart communication protocol as specified by the manufacturer.

The code is working with my setup via the rs485 interface of my jbd bms:

This pull request is related to issue #467.

BatterySettings LifeView-LifeData LifeView-LifeData-JBD-BMS

schlimmchen commented 1 month ago

Thanks for this contribution!

I looked at this some time ago and noticed that it needs some polishing. Minor stuff (commented code should be cleaned up, spaces/indention).

The other thing that is a little bit offputting is this:

I copied the the jk-bms code

However, I am not sure if I am willing to put in the time and effort to generalize and share code. It would be the right thing to do, though. I can also see that there are significant differences, so clearly, there will be a lot of new code. We could generalize the DataPointContainer using a template (the DataPoint's std::variant go into the template parameters, and the DataPointContainer needs at least the label as template parameter...). I think that would be a good idea.

You marked this as draft a couple of hours ago. Why is that? because of the conflicts in webapp_dist? Just clean webapp_dist and you are good, at least regarding the conflicts.

MoleBre commented 1 month ago

Hi Schlimmchen. Ich antworte mal kurz auf Deutsch.

Vielen Dank erstmal für deine Rückmeldung!

Den PR-Status "Draft" und "Open" hatte ich falsch verstanden. Ich habe es jetzt wieder auf "Open" zurückgeändert.

Ja, da hast du recht. Den Code aufzudoppeln ist nicht schön. Anders hätte ich deinen Jk-Bms Code jedoch anpassen müssen, damit ich ihn für das Jbd-Bms wiederverwenden kann. Das habe ich mich bei meinem ersten PR nicht getraut. Ich habe hier kein Jk-Bms zum testen und ich wollte keine Bugs in euren bestehenden Code einbauen. Das Risiko war mir zu hoch.

Ich habe die Test-Kommentare gelöscht und den Commit mit den Webapp-Files zurückgenommen. Die falschen Einrückungen konnte ich aber bei mir in VS Code nicht finden.

Gebe mir gerne Bescheid, wenn ich noch etwas anpassen soll.

BG.

schlimmchen commented 3 weeks ago

Rebased onto current development and de-deplicated DataPointsContainer.

schlimmchen commented 3 weeks ago

@MoleBre Ich wollte mergen, als mir aufgefallen ist, dass die HASS autodiscovery fehlt. Kannst du da bitte zumindest die wichtigsten Werte nacharbeiten? Siehe MqttHandleBatteryHass.cpp.

schlimmchen commented 3 weeks ago

Zwei Drei Dinge sind hier noch zu tun: (1) HASS autodiscovery und (2) der Battery Provider Index muss 6 werden um #1199 auszuweichen und (3) es muss nach dem Mergen von #1199 nochmal ge-rebase't werden, wobei Konflikte zu beheben sein werden.