Enhanced MQTT Data for Serial Battery Configuration #573

swingstate · 2023-12-31T11:50:14Z

I'm excited to share my first pull request, which focuses on enriching the MQTT data for batteries in a serial configuration. This enhancement includes not only additional data fields but also improves the decimal accuracy for both live view displayed data and MQTT transmissions.

Changes Made:

New Data Fields for HomeAssistant MQTT Auto Discovery:

Midpoint Voltage:

Added midpoint voltage information to provide a more comprehensive view of the battery's state in a serial configuration.

Midpoint Deviation:

Introduced the midpoint deviation metric, offering insights into the balance and stability of the battery pack.

Additional Metrics:

Consumed Ampere Hours: Tracking the total ampere hours consumed over time.
Instantaneous Power: Providing real-time power consumption data for more immediate insights.
Charged Energy: Monitoring the total energy charged into the battery.
Discharged Energy: Keeping tabs on the energy discharged from the battery.

Decimal Accuracy Improvement:
To enhance precision, the decimal accuracy for both live view and MQTT-transmitted Smartshunt data has been increased.

schlimmchen

Thanks for contributing and congrats to your first pull request 🚀 Please have a look at my review comments.

schlimmchen · 2023-12-31T13:02:42Z

.DS_Store

Remove this file from the changeset (edit the commit that added the file and do not add the file).

schlimmchen · 2023-12-31T13:03:10Z

.gitignore

@@ -8,3 +8,4 @@ platformio-device-monitor*.log
 logs/device-monitor*.log
 platformio_override.ini
 .DS_Store
+.DS_Store


This line is a duplicate. There should be no change to the .gitignore file.

schlimmchen · 2023-12-31T13:03:40Z

lib/.DS_Store

Remove this file from the changeset (leave it as is).

schlimmchen · 2023-12-31T13:03:45Z

lib/Hoymiles/.DS_Store

Remove this file from the changeset (leave it as is).

schlimmchen · 2023-12-31T13:04:51Z

include/BatteryStats.h

-        float _voltage;
-        float _current;
-        float _temperature;
+        float_t _voltage;
+        float_t _current;
+        float_t _temperature;
        bool _tempPresent;
        uint8_t _chargeCycles;
        uint32_t _timeToGo;
-        float _chargedEnergy;
-        float _dischargedEnergy;
+        float_t _chargedEnergy;
+        float_t _dischargedEnergy;
        String _modelName;
+        float_t _midpointVoltage;
+        float_t _midpointDeviation;
+        float_t _instantaneousPower;
+        float_t _consumedAmpHours;


Why are you changing the float types? I suggest leaving them at float and using float for the new member variables.

I have looked into this again and will revert this change.

schlimmchen · 2023-12-31T13:08:21Z

lib/VeDirectFrameHandler/VeDirectShuntController.h

-        int32_t P;                      // Instantaneous power
-        int32_t CE;                     // Consumed Amp Hours
-        int32_t SOC;                    // State-of-charge
+        float_t P;                      // Instantaneous power
+        float_t CE;                     // Consumed Amp Hours
+        float_t SOC;                    // State-of-charge
        uint32_t TTG;                   // Time-to-go
        bool ALARM;                     // Alarm condition active
        uint32_t AR;                    // Alarm Reason
-        int32_t H1;                     // Depth of the deepest discharge
-        int32_t H2;                     // Depth of the last discharge
-        int32_t H3;                     // Depth of the average discharge
+        float_t H1;                     // Depth of the deepest discharge
+        float_t H2;                     // Depth of the last discharge
+        float_t H3;                     // Depth of the average discharge
        int32_t H4;                     // Number of charge cycles
        int32_t H5;                     // Number of full discharges
        int32_t H6;                     // Cumulative Amp Hours drawn
-        int32_t H7;                     // Minimum main (battery) voltage
-        int32_t H8;                     // Maximum main (battery) voltage
+        float_t H7;                     // Minimum main (battery) voltage
+        float_t H8;                     // Maximum main (battery) voltage
        int32_t H9;                     // Number of seconds since last full charge
        int32_t H10;                    // Number of automatic synchronizations
        int32_t H11;                    // Number of low main voltage alarms
        int32_t H12;                    // Number of high main voltage alarms
        int32_t H13;                    // Number of low auxiliary voltage alarms
        int32_t H14;                    // Number of high auxiliary voltage alarms
-        int32_t H15;                    // Minimum auxiliary (battery) voltage
-        int32_t H16;                    // Maximum auxiliary (battery) voltage
-        int32_t H17;                    // Amount of discharged energy
-        int32_t H18;                    // Amount of charged energy
+        float_t H15;                    // Minimum auxiliary (battery) voltage
+        float_t H16;                    // Maximum auxiliary (battery) voltage
+        float_t H17;                    // Amount of discharged energy
+        float_t H18;                    // Amount of charged energy
+        float_t VM;                     // Mid-point voltage of the battery bank
+        float_t DM;                     // Mid-point deviation of the battery bank


You must not change the type of the member variables of this struct. The Shunt never transmits floating point values through the wire. It always transmits integers. And we shall save them as such. This is important, as floating point values are not always interchangable with real integers. Conversion to floating point values (because the integers are tens or hundreds or whatever multiples of a particular unit) happens later (BatteryStats).

Okay, understood. The latter is what I am after, respectively I want to use float since they (from my understanding) provide higher accuracy.

I will revert the changes here and look into increasing accuracy / BatteryStats separately, once this PR is done.

schlimmchen · 2023-12-31T14:21:12Z

src/BatteryStats.cpp

@@ -41,7 +41,7 @@ void BatteryStats::getLiveViewData(JsonVariant& root) const
    root[F("manufacturer")] = _manufacturer;
    root[F("data_age")] = getAgeSeconds();

-    addLiveViewValue(root, "SoC", _SoC, "%", 0);
+    addLiveViewValue(root, "SoC", _SoC, "%", 1);


This is too generic. The JK BMS (and maybe the Pylontech as well) do not report tens of percents. I suggest you introduce a protected "SoC resolution" variable in the base class BatteryStats, initialize it to zero, and overwrite it to be 1 in VictronSmartShuntStats.

maybe later, as this goes beyond my skills. I'll revert this one first.

schlimmchen · 2023-12-31T14:22:08Z

src/BatteryStats.cpp

}
+

Please avoid adding empty lines like this one.

schlimmchen · 2023-12-31T14:25:21Z

src/MqttHandlePylontechHass.cpp

+    publishSensor("SmartShunt battery midpoint", NULL, "midpointVoltage", "voltage", "measurement", "V");
+    publishSensor("SmartShunt battery midpoint Deviation", NULL, "midpointDeviation", "battery", "measurement", "%");
+    publishSensor("SmartShunt Instantaneous Power", NULL, "instantaneousPower", "power", "measurement", "W");
+    publishSensor("SmartShunt Consumed AmpHours", NULL, "consumpedAmpHours", "energy", "measurement", "Ah");
+    publishSensor("SmartShunt Total Charged Energy", NULL, "chargedEnergy", "energy", "total_increasing", "kWh");
+    publishSensor("SmartShunt Total DisCharged Energy", NULL, "dischargedEnergy", "energy", "total_increasing", "kWh");


We may leave it like that for now, however, this is not the way forward. You are now publishing "SmartShunt" sensors to HomeAssistent, even if the actual battery is a Pylontech or a JK BMS. This is not yet fixed in general. The whole file needs to be refactored such that only the appropriate sensors are published. Your change makes it worse, though...

I am aware of this flaw and piggybacking on the Pylontech, since I haven't found any other way to get the sensor published.

Though I do not agree that this is making things worse as this is just using existing code strategies / structures and extending on it.

merge

swingstate · 2024-01-10T08:27:45Z

@schlimmchen I made couple of adjustments based on your feedback and deleted the .DS files (at least I think I deleted them). Can you please take a look.

I merged the last updated from the main repo into my dev and now have 2 onBattery PCBs running with the latest (main + my changes) updates. So far things look good.

schlimmchen · 2024-02-05T20:18:04Z

@swingstate This is much better, thanks!

This changeset now removes three of those (other) .DS_Store files that were previously there. The goal is to have a changeset that only does what is needed to get the feature going.

Regarding the issue that you publish your values alongside the Pylontech sensors you wrote:

Though I do not agree that this is making things worse as this is just using existing code strategies / structures and extending on it.

I disagree. People with Pylonthech batteries would see sensors that don't even show values for their Battery, as they are specific to the SmartShunt.

Since you prepared the PR, at least one more thing changed: The use of the "F" macro. That would have to be removed. And you would have to rebase your work onto the current state of the development branch.

I finished the Home Assistent integration for the JK BMS in #640. Your work and mine now conflict. However, the work I did will solve the issue with the missing distinction of the battery provider.

If you are okay with it, I can offer to help this change along and prepare a respective new PR that is based on #640. I would use the work you did to put it in the right new places and prepare clean commits. I could try to preserve your authorship, however I cannot promise that it will work as I intend.

the Victron SmartShunt communicates the SoC value in promille. this should be displayed in the web UI accordingly. this is a good excuse to fully move ownership of the SoC value to the BatteryStats base class and add a precision indicator variable. this is required to be set each time a derived class (a battery provider) wants to update the SoC value. the precision is then used when populating the JSON data for the web UI (live view). related to helgeerbe#573.

the Victron SmartShunt communicates the SoC value in permille. this should be displayed in the web UI accordingly. this is a good excuse to fully move ownership of the SoC value to the BatteryStats base class and add a precision indicator variable. this is required to be set each time a derived class (a battery provider) wants to update the SoC value. the precision is then used when populating the JSON data for the web UI (live view). related to helgeerbe#573.

swingstate · 2024-02-20T18:45:35Z

I will not capacity to finish and test this in the near future. Maybe you @schlimmchen or @LukasK13 can look into this and/or merge this with the effort Lukas is looking into for the MQTT Powerlimiter settings.

ottelo9 · 2024-03-22T09:58:43Z

If you need someone to test it (if ready), I could do it. I have a Pylontech battery.

swingstate and others added 4 commits December 19, 2023 16:38

Extended Ve.Direct MQTT Details

1ab5300

Added Smartshunt MQTT Autodiscovery details

385729a

Fixed & Expanded MQTT HA Autodiscovery

e0366b7

MQTT topic bugfix

b004253

schlimmchen requested changes Dec 31, 2023

View reviewed changes

helgeerbe marked this pull request as draft January 1, 2024 16:07

swingstate and others added 4 commits January 9, 2024 10:17

Delete .DS_Store

7de1990

Merge pull request #1 from helgeerbe/development

b7aafb0

merge

adjustments based on PR feedback

3927c6e

removed duplicate entry

6a0d853

schlimmchen mentioned this pull request Feb 18, 2024

DPL Voltage Features #679

Merged

ottelo9 mentioned this pull request Mar 22, 2024

[Request] measure (calculate) energy going in to and out of the battery for e.g. Home Assistant Energy Dashboard #783

Closed

schlimmchen linked an issue May 4, 2024 that may be closed by this pull request

[Request]More Data on the Startpage from the Battery - Voltage Diffence between 2 x 12V LiFePo4 #954

Open

schlimmchen mentioned this pull request May 4, 2024

[Request]More Data on the Startpage from the Battery - Voltage Diffence between 2 x 12V LiFePo4 #954

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhanced MQTT Data for Serial Battery Configuration #573

Enhanced MQTT Data for Serial Battery Configuration #573

swingstate commented Dec 31, 2023

schlimmchen left a comment

schlimmchen Dec 31, 2023

schlimmchen Dec 31, 2023

schlimmchen Dec 31, 2023

schlimmchen Dec 31, 2023

schlimmchen Dec 31, 2023

swingstate Jan 9, 2024

schlimmchen Dec 31, 2023

swingstate Jan 9, 2024

swingstate Jan 9, 2024

schlimmchen Dec 31, 2023

swingstate Jan 9, 2024

schlimmchen Dec 31, 2023

schlimmchen Dec 31, 2023

swingstate Jan 9, 2024 •

edited

swingstate Jan 9, 2024

swingstate commented Jan 10, 2024

schlimmchen commented Feb 5, 2024

swingstate commented Feb 20, 2024

ottelo9 commented Mar 22, 2024

Enhanced MQTT Data for Serial Battery Configuration #573

Are you sure you want to change the base?

Enhanced MQTT Data for Serial Battery Configuration #573

Conversation

swingstate commented Dec 31, 2023

Changes Made:

schlimmchen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

swingstate Jan 9, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

swingstate commented Jan 10, 2024

schlimmchen commented Feb 5, 2024

swingstate commented Feb 20, 2024

ottelo9 commented Mar 22, 2024

swingstate Jan 9, 2024 •

edited