Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Daly bms sleep mode causing incorrect data in MP2 gx webinterface #193

Closed
LukasBenner opened this issue Sep 5, 2022 · 12 comments · Fixed by #468
Closed

Daly bms sleep mode causing incorrect data in MP2 gx webinterface #193

LukasBenner opened this issue Sep 5, 2022 · 12 comments · Fixed by #468
Labels
bug Something isn't working

Comments

@LukasBenner
Copy link

LukasBenner commented Sep 5, 2022

Describe the bug
The battery is fully loaded and the daly bms stops charging.
Shortly after it goes into sleep mode and doesn't respond on the uart interface.
In the WebInterface of the MP2 GX and on the VRM Portal wrong values are shown.
e.g the current is displayed with 20 amps but is 0 in real.

The driver log ("/var/log/dbus-serialbattery.ttyUSB0/current") shows "No reply - returning"

image

To Reproduce
Steps to reproduce the behavior:

  1. Stop charging so the daly bms goes into sleep mode.
  2. Open the log from above.

Expected behavior
I'd expect the web interface of the MP2 GX to show the correct data.
I'd expect the software to differentiate between the daly bms being in sleep mode
and the daly bms not responding.

VenusOS (please complete the following information):

  • Device type MP2 GX
  • Firmware Version v2.90

Battery/BMS (please complete the following information):

  • BMS/Battery type: Daly
  • Cells: 16
  • Interface: USB-RS232 UART

Additional context
Add any other context about the problem here.

@Louisvdw Louisvdw added the bug Something isn't working label Sep 6, 2022
@LukasBenner
Copy link
Author

The same behavior can be observed when completely disconnecting the BMS.
This is kind of a critical bug because the soc regulates the parameters for charging and discharging.
Although the Daly BMS will cut the battery when fully charged or discharged, the allowed current for charging and discharging is not updated.

I just connected the BMS to a cerbogx of a friend of mine to test the software. I'd expect my installation to stop charging while the BMS is not connected. Unfortunately it just keeps charging. Went from 61% to 69% percent without any error or something.

@mr-manuel
Copy link
Collaborator

@transistorgit maybe you know, if this is still an issue?

@transistorgit
Copy link
Contributor

Yes, I think so. There is no means of state handling employed. If the bms stops answering, the last values will be there forever.
It should be easy to solve with a timeout in refresh_data. If there isn't any new data for 1 minute, set AllowToCharge/Discharge to false.

How is it done in other bms? Wonder if it shouldn't be a base class function?

@mr-manuel
Copy link
Collaborator

From what I know, it's all the same for all BMS. There is actually no timeout.

A timeout in the battery class would be a good idea. The default Victron timeout are 60 seconds as you also said.

Even, if I unplug the USB to serial adapter it still shows the BMS.

2023-05-01 18:53:21.222963500 ERROR:SerialBattery:[Errno 2] could not open port /dev/ttyUSB0: [Errno 2] No such file or directory: '/dev/ttyUSB0'
2023-05-01 18:53:22.224139500 ERROR:SerialBattery:[Errno 2] could not open port /dev/ttyUSB0: [Errno 2] No such file or directory: '/dev/ttyUSB0'
2023-05-01 18:53:23.225307500 ERROR:SerialBattery:[Errno 2] could not open port /dev/ttyUSB0: [Errno 2] No such file or directory: '/dev/ttyUSB0'
2023-05-01 18:53:24.226727500 ERROR:SerialBattery:[Errno 2] could not open port /dev/ttyUSB0: [Errno 2] No such file or directory: '/dev/ttyUSB0'
2023-05-01 18:53:25.227252500 ERROR:SerialBattery:[Errno 2] could not open port /dev/ttyUSB0: [Errno 2] No such file or directory: '/dev/ttyUSB0'
2023-05-01 18:53:26.229309500 ERROR:SerialBattery:[Errno 2] could not open port /dev/ttyUSB0: [Errno 2] No such file or directory: '/dev/ttyUSB0'

Could you implement your suggestion and open a PR for the jkbms_ble branch?

@mr-manuel
Copy link
Collaborator

Theoretically there should already be a check, but it seems that it's not working.

def publish_battery(self, loop):
# This is called every battery.poll_interval milli second as set up per battery type to read and update the data
try:
# Call the battery's refresh_data function
success = self.battery.refresh_data()
if success:
self.error_count = 0
self.battery.online = True
else:
self.error_count += 1
# If the battery is offline for more than 10 polls (polled every second for most batteries)
if self.error_count >= 10:
self.battery.online = False
# Has it completely failed
if self.error_count >= 60:
loop.quit()
# This is to mannage CVCL
self.battery.manage_charge_voltage()
# This is to mannage CCL\DCL
self.battery.manage_charge_current()
# publish all the data from the battery object to dbus
self.publish_dbus()
except:
traceback.print_exc()
loop.quit()

@transistorgit
Copy link
Contributor

just did a real test with my dalys with branch jkbms_ble.
In both cases, unplugging USB or unplugging the serial connector at the BMS, the remote console shows correctly "Not connected" after a minute.
Until 1 minute is over the values stay frozen, that can be optimized.

@transistorgit
Copy link
Contributor

transistorgit commented May 3, 2023

created pull request mr-manuel/venus-os_dbus-serialbattery#4
is that right? or should it go into luisvdw's repo?

hope somebody can test with other BMS types also. Just tested with Daly, but should be the same on any BMS as the changes are in battery.py mainly.

@mr-manuel
Copy link
Collaborator

mr-manuel commented May 4, 2023

Better open a PR in my repo until we finished testing :-) I test it today with a JKBMS.

@mr-manuel
Copy link
Collaborator

mr-manuel commented May 4, 2023

@transistorgit I did some testing and I have this behaviour: After 10 seconds the BMS is shown as Not connected and all values are --. There was a TypeError and therefore the script crashed.

If I reconnect the BMS in the seconds from 0 - 60, it's not recognized again. Only when the driver is restarted.

If the BMS gets disconnected in Venus OS after 60 seconds the charging/discharging is not blocked anymore. This can be good or bad depending on the cause.

Would it make more sense to keep the driver "connected", but blocking charge/discharge and eventually let the user select/change the behaviour by config file? In this case the driver/system has to be restarted, if a BMS is removed from the device.

What are your thoughts?

@transistorgit
Copy link
Contributor

yes, I had quite some error handling to do until I had fixed all Type errors. just very probable that I misses one that is now triggered with your BMS. Can you send me the log line with the line no where is crashed?

If all exceptions are handled, the behaviour should be like I wrote. And I had also no problem with redetecting the device then. This is in my view the cleanest solution.

@mr-manuel
Copy link
Collaborator

Ok, in my case somehow ttyUSB0 gets blocked from the driver. When I disconnect from the USB port ttyUSB0 and connect it again, before the driver exited, then the devices is recognized as ttyUSB1. If I reconnect it again after the driver exited, then it's recognized as ttyUSB0 again.

I made some more changes in the jkbms_ble branch in my fork, that are merged later today with Louisvdw jkbms_ble branch:

  • Added: Alert is triggered, when BMS communication is lost
  • Added: Block charge/discharge when BMS communication is lost. Can be enabled trough the config file

@mr-manuel
Copy link
Collaborator

mr-manuel commented May 4, 2023

We wrote nearly at the same time :P I already fixed the other errors, so none left. If the adapter does not change port, all is working as expected. In my eyes we can close this now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants