softwarecrash / Daly2MQTT

ESP8266 Connector to get Daly / XENES / HI-BMS / BullTron BMS Data into MQTT systems
Other
138 stars 30 forks source link

Add pack voltage thresholds #148

Closed s10l closed 6 months ago

s10l commented 6 months ago

I would like to contribute a little to this excellent project.

I am currently missing the setting of the BMS configuration as well as the support of the active balancer (must admit, I have not tried it). I will open a few pull requests over a longer period of time to add these features.

This PR is used to get the voltage thresholds for the pack and publish them via MQTT.

The level-2 values for the cell voltage thresholds are also published via MQTT. I noticed that the level-2 values are the values I set via the SmartBMS app.

cell_hVt and cell_lVt remains unchanged.

...
      "cell_hVt": 4.15,
      "cell_lVt": 2.8,
      "cell_hVt2": 3.55,
      "cell_lVt2": 2.8,
      "pack_hVt": 66.4,
      "pack_lVt": 44.8,
      "pack_hVt2": 56,
      "pack_lVt2": 47,
...
all-solutions commented 6 months ago

First of all: the balancers speak the same protocol as the BMS. So the support is there. I have one running myself.

And now to the basic question: why do you need the thresholds in MQTT? There will NOT be a configuration of these values via Daly2MQTT. There is too much behind it that could be misconfigured.

s10l commented 6 months ago

Thank you for clarifying that the active balancer is already usable. 👍

Thresholds are already being sent via MQTT (https://github.com/softwarecrash/Daly2MQTT/commit/50e3af6909294bedac70e9a5ae67b6f8cdca70b1). Unfortunately, I could not use them because the level-2 value, the relevant one, was not sent.

It is convenient for me to validate the readout via MQTT. I cannot say whether others would like to use the data. If these values are not desired, they could be hidden behind a switch and thus only sent if desired.

I am currently doing this with the intention of also supporting the setting of the BMS parameters via Daly2MQTT, as this is already implemented for the SoC. But if this is not desired at all... I'll probably have to think of something else.

softwarecrash commented 6 months ago

Hello, Thank you for the PR but it's not needed to do a force. So the threshold values Level 2 can be implemented.

The other thing... To build in functions to manipulate thresholds and other values over mqtt will NOT be made, why? A lot of users have no idea what that does and then you see burning things... So we have long discuse about this, with the conclusion that soc and fet control is enough.

The most values are set one by provisoning the battery and then never changed, so i think that's a realy bad idea to offer the possibility to manipulate all values over mqtt. On the other hand its a security risk. I hope you will understand it.

s10l commented 6 months ago

I have to admit that these are plausible reasons.

Would the output of the current BMS configuration in the web UI be a desirable feature that could be merged?

s10l commented 6 months ago

I overlooked the fact that the license has been changed in the meantime.

I kindly ask for permission to make modifications to the source code for pull requests. Thank you :)

all-solutions commented 6 months ago

The license change was made some time ago. PRs can still be made.

The license change is intended to prevent our months of work from being "taken" and used for profit. We were just about to release your PR.

Please note that the PR does not give you any rights to redistribute the complete software yourself. If you agree with this, you are welcome to push the PR again.