helgeerbe / OpenDTU-OnBattery

Software for ESP32 to talk to Hoymiles Inverters and Victrons MPPT battery chargers (Ve.Direct)
GNU General Public License v2.0
298 stars 63 forks source link

[Request] charging to SoC target with defined power after "Immediate charging requested" by Pylontech #723

Closed gitisgreat2023 closed 5 months ago

gitisgreat2023 commented 6 months ago

Is your feature request related to a problem? Please describe.

When the SoC, for whatever reason, drops so low that an immediate charge is requested by the Pylontech battery, to avoid damage, currently nothing happens. Though a controllable AC charger is present and could charge the battery again to safe level.

Describe the solution you'd like

When "Immediate charging requested" switches from "No" to "Yes" the AC charger should charge to an specified SoC Target with a to be specified power. Also it would be good if any of these situations would be logged.

Describe alternatives you've considered

Use an external script (like NodeRed) which monitors via MTTQ the "Immediate charging requested" state and sets via MTTQ the charge power until the target SoC is reached.

Additional context

Currently I use a minimal SoC of 20%. Implementing this automatically charging feature would enable to put for example a SoC of 15% as minimum, of even 10%. With the logging one could decide to top balance (charging 100%) or put the minimal SoC higher to avoid such immediate charge request.

schlimmchen commented 6 months ago

@MalteSchm Is this something you'd like to takle?

MalteSchm commented 6 months ago

My system tells me: yes I should address that... Here is why:

I just checked my dashboard and found all alarms red. It seemed that the inverter operated at full power with communication between openDTU and the inverter being broken. Soft and hard restarts of the ESP were unsuccessful. I got the system back by issuing an inverter reset command.

I think that resetting the inverter when communication is broken for a long time and starting up the PSU when charging is requested would be countermeasures that should be implemented.

So in general: the battery SoC drops to that level it is likely that the inverter runs mad. Implementing only the PSU solution is not sufficient IMHO. I think inverter restart and an ESP reset should be carried out in this case

schlimmchen commented 6 months ago

Meh... You, too? There are other users who recently started reporting that their inverter is somehow stuck and must be revived through a reboot command, as even with my more recent changes that repeat limit updates and power requests their inverter somehow manages to keep in standby... This is weird...

The DPL could (quite easily with the recent changes) detect if the inverter is not reaching the desired state for a longer time and trigger issuing a reboot message. Should we do that? Can you open a respective issue if you think that's desirable?

and an ESP reset

That would be a last resort, i.e., after issuing the reboot command did not do the trick.

gitisgreat2023 commented 6 months ago

@MalteSchm That's sounds a quite scary, an inverter going rogue... how low went the SoC on your Pylontech, did he shutdown due to undervoltage? Any idea why the inverter got in that situation?

To be prepared: "I got the system back by issuing an inverter reset command.": how do I do this? This is not possible in the WebUI, right?

MalteSchm commented 6 months ago

@schlimmchen

Yes I had that twice now. There could have been other cases that went without me noticing. The inverter is reset in regular intervals at night so the issue will resolve over time. The battery stays fully discharged during this period, potentially even disconnected as the BMS will cut the cord at some point.

I think changing the DPL this way is desirable yes. And I also think we should trigger an ESP reset as a last resort

@gitisgreat2023

First time: 0%, yesterday 2%. I think the inverter did shut off at this stage but: Once the PSU started charging the inverter also started up again. You can reset ("Neustart") the inverter in the web-ui. This is accessible from the buttons in the main page / dashboard. Right next to the buttons to turn the inverter off and on

MalteSchm commented 6 months ago

I just checked my dashboard and found all alarms red. It seemed that the inverter operated at full power with communication between openDTU and the inverter being broken. Soft and hard restarts of the ESP were unsuccessful. I got the system back by issuing an inverter reset command.

I think that resetting the inverter when communication is broken for a long time and starting up the PSU when charging is requested would be countermeasures that should be implemented.

Created https://github.com/helgeerbe/OpenDTU-OnBattery/issues/748

gitisgreat2023 commented 6 months ago

@MalteSchm 0% and 2% ain't good... yes we definitely need a feature coping with this. We should under all circumstances avoid that the inverter goes rogue and the PSU starts charging, creating a high power circular flow. If one is on holidays that could get dangerous.

First time: 0%, yesterday 2%. I think the inverter did shut off at this stage but: Once the PSU started charging the inverter also started up again. You can reset ("Neustart") the inverter in the web-ui. This is accessible from the buttons in the main page / dashboard. Right next to the buttons to turn the inverter off and on

I'm stupid I guess: next to the turn on/off there's the grid info: image

image

Or do you meant the on/off is the inverter reset?

MalteSchm commented 6 months ago

@MalteSchm 0% and 2% ain't good... yes we definitely need a feature coping with this. We should under all circumstances avoid that the inverter goes rogue and the PSU starts charging, creating a high power circular flow. If one is on holidays that could get dangerous.

First time: 0%, yesterday 2%. I think the inverter did shut off at this stage but: Once the PSU started charging the inverter also started up again. You can reset ("Neustart") the inverter in the web-ui. This is accessible from the buttons in the main page / dashboard. Right next to the buttons to turn the inverter off and on

I'm stupid I guess: next to the turn on/off there's the grid info: image

image

Or do you meant the on/off is the inverter reset?

Click on this on/off button. There is more...

MalteSchm commented 6 months ago

@schlimmchen How do I get the PylontechBatteryStats needed for _chargeImmediately

This did not work: auto stats = static_cast<PylontechBatteryStats>(Battery.getStats());

schlimmchen commented 6 months ago

You can't. Battery returns a std::shared_ptr<BatteryStats>, i.e., a pointer to the base class BatteryStats, and if you were to cast this to a PylontechBatteryStats, you would need some fancy std::dynamic_cast I guess... The reason I don't know is because I avoid stuff like this at all cost. We shouldn't use it or do it like that. And if we were, we would need to be sure that the instance is actually an instance of the derived PylontechBatteryStats before we attempt such a cast.

Your request is valid, of course, so lets work out a solution.

The goal is to know the value of _chargeImmediately in HuaweiCanClass. The value is generated in PylontechCanReceiver and then saved in PylontechBatteryStats.

We can't ask PylontechCanReceiver, as there is no public instance. Class Battery manages that instance in a private std::unique_ptr<BatteryProvider>.

That might sound cumbersome, but this kind of encapsulation, separation of concern, and ownership rules, saves us headaches in other contexts.

So I think the best way is to define a new method bool needsCharging() in the BatteryStats base class. In particular, this gives the other BatteryStats derived classes a chance to implement the same logic based on their battery provider implementation, i.e., the JK BMS can also implement this method and return true to facilitate this feature. Same for the SmartShunt, which might also want to trigger this feature.

This answer war quite verbose, but maybe you appreciate knowing my train of thought.

Please rebase onto 56353e4f :blush:

gitisgreat2023 commented 6 months ago

@MalteSchm 0% and 2% ain't good... yes we definitely need a feature coping with this. We should under all circumstances avoid that the inverter goes rogue and the PSU starts charging, creating a high power circular flow. If one is on holidays that could get dangerous.

First time: 0%, yesterday 2%. I think the inverter did shut off at this stage but: Once the PSU started charging the inverter also started up again. You can reset ("Neustart") the inverter in the web-ui. This is accessible from the buttons in the main page / dashboard. Right next to the buttons to turn the inverter off and on

I'm stupid I guess: next to the turn on/off there's the grid info: image image Or do you meant the on/off is the inverter reset?

Click on this on/off button. There is more...

Thanks! As an interim solution, until these features are built in and released I now are able to access remotely my OpenDTU-on-Battery, for example when I'm not at home. I activated on my Fritz!Box VPN and know I can easily control my setup remotely as well by phone. Note: if the phone disconnects from WiFi and no LTE appears (for example on my Google Pixel) while trying to connect to the Fritz!Box VPN server, then force on the mobile data settings of the phone ipv4, see for example here.

Linos88 commented 6 months ago

I also like the original idea with forced loading. This function can also be useful for a long service life in winter. Instead of using the SoC in the limit range, the voltage would be more suitable. The SoC can stop if there is only a small standby consumption and the voltage still continues to fall

Manos1966 commented 6 months ago

@MalteSchm Do you have the dtata stored from the time your inverter went rogue? Can it be that the Efficiency % rating of the inverter went from 96% down to something around 60%?

This is the reason why I have a SHELLY 1PM connected to the 230V side of the inverter.... šŸ˜‰

ottelo9 commented 6 months ago

@MalteSchm Do you have the dtata stored from the time your inverter went rogue? Can it be that the Efficiency % rating of the inverter went from 96% down to something around 60%?

This is the reason why I have a SHELLY 1PM connected to the 230V side of the inverter.... šŸ˜‰

Really good idea. I'll probably install a Zigbee relay right now.

CaCu15 commented 6 months ago

You can't. Battery returns a std::shared_ptr<BatteryStats>, i.e., a pointer to the base class BatteryStats, and if you were to cast this to a PylontechBatteryStats, you would need some fancy std::dynamic_cast I guess... The reason I don't know is because I avoid stuff like this at all cost. We shouldn't use it or do it like that. And if we were, we would need to be sure that the instance is actually an instance of the derived PylontechBatteryStats before we attempt such a cast.

Your request is valid, of course, so lets work out a solution.

The goal is to know the value of _chargeImmediately in HuaweiCanClass. The value is generated in PylontechCanReceiver and then saved in PylontechBatteryStats.

We can't ask PylontechCanReceiver, as there is no public instance. Class Battery manages that instance in a private std::unique_ptr<BatteryProvider>.

That might sound cumbersome, but this kind of encapsulation, separation of concern, and ownership rules, saves us headaches in other contexts.

So I think the best way is to define a new method bool needsCharging() in the BatteryStats base class. In particular, this gives the other BatteryStats derived classes a chance to implement the same logic based on their battery provider implementation, i.e., the JK BMS can also implement this method and return true to facilitate this feature. Same for the SmartShunt, which might also want to trigger this feature.

This answer war quite verbose, but maybe you appreciate knowing my train of thought.

Please rebase onto 56353e4 šŸ˜Š

@schlimmchen:

I would like to extend this approach with the following suggestion: Currently the automatic charge control (Huawei automatic charge control) does NOT consider the feedback from the BMS during charging, especially the "charge currrent limitation" as provided by the Pylontech BMS (don't know abut the other BMSs). In order to support this, it would be good to have an additional method float getChargeCurrentLimitation() that returns the respective value returned by the BMS (and currently by REST call api/batterylivedata/status). This would make this value available to the automatic power control in HuaweiCanClass::loop() to limit the charge like this:

        // Set the actual output limit
        float efficiency =  (_rp.efficiency > 0.5 ? _rp.efficiency : 1.0); 
        float outputCurrent = efficiency * (newPowerLimit / _rp.output_voltage);

        // new code
        // assuming getChargeCurrentLimitation returns something > than any current supported by the Huawei, e.g. 100A 
        // if not supported by BMS 
        float chargeCurrentLimitation = Battery.getStats().getChargeCurrentLimitation(); 
        outputCurrent = std::min(outputCurrent, chargeCurrentLimitation);
MalteSchm commented 6 months ago

@CaCu15

I already have that implemented but still need to test it

MalteSchm commented 6 months ago

Draft PR in https://github.com/helgeerbe/OpenDTU-OnBattery/pull/767

schlimmchen commented 5 months ago

I consider this closed as completed since #767 was merged into development and will be released eventually.

github-actions[bot] commented 4 months ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new discussion or issue for related concerns.