mr-manuel / venus-os_dbus-serialbattery

Battery Monitor driver for serial battery in VenusOS GX systems
https://mr-manuel.github.io/venus-os_dbus-serialbattery_docs/
MIT License
47 stars 17 forks source link

Bug in code to generate connection name #55

Closed calledit closed 4 months ago

calledit commented 4 months ago

Describe the bug

Some type of bug ( introduced in this commit https://github.com/mr-manuel/venus-os_dbus-serialbattery/commit/af773cc2084c7bf9d95ce4202bb6ec666c9292ab?diff=split&w=0 )

@40000000669521d304449914 INFO:SerialBattery:
@40000000669521d3044ade8c INFO:SerialBattery:Starting dbus-serialbattery
@40000000669521d3047c1fec INFO:SerialBattery:Venus OS v3.33
@40000000669521d30482bb54 INFO:SerialBattery:dbus-serialbattery v1.4.20240714dev
@40000000669521e305746684 INFO:SerialBattery:test_connection
@40000000669521e3057acb3c INFO:SerialBattery:Connection established to LiTime_Ble
@40000000669521e305eb3494 INFO:SerialBattery:unique_identifier called before get_settings so serial_number is not populated
@40000000669521e3281e9b64 INFO:SerialBattery:DeviceInstance = 1
@40000000669521e3282fd974 INFO:SerialBattery:PID file created successfully: /var/tmp/dbus-serialbattery_1.pid
@40000000669521e328368864 INFO:SerialBattery:Used DeviceInstances = []
@40000000669521e3283d27b4 INFO:SerialBattery:com.victronenergy.battery.
@40000000669521e328451ec4 INFO:SerialBattery:get_settings
@40000000669521e328fed1d4 Traceback (most recent call last):
@40000000669521e328feed2c   File "/opt/victronenergy/dbus-serialbattery/dbus-serialbattery.py", line 287, in <module>
@40000000669521e328ff00b4     main()
@40000000669521e328ff0884   File "/opt/victronenergy/dbus-serialbattery/dbus-serialbattery.py", line 236, in main
@40000000669521e328ff1c0c     if not helper.setup_vedbus():
@40000000669521e328ff23dc   File "/opt/victronenergy/dbus-serialbattery/dbushelper.py", line 496, in setup_vedbus
@40000000669521e328ff3764     self._dbusservice.add_path("/Mgmt/Connection", self.battery.connection_name())
@40000000669521e329005874   File "/opt/victronenergy/dbus-serialbattery/battery.py", line 297, in connection_name
@40000000669521e329006fe4     "__" + utils.bytearray_to_string(self.address).replace("\\", "0")
@40000000669521e329007f84   File "/opt/victronenergy/dbus-serialbattery/utils.py", line 580, in bytearray_to_string
@40000000669521e32900930c     return "".join("\\x" + format(byte, "02x") for byte in data)
@40000000669521e32901006c   File "/opt/victronenergy/dbus-serialbattery/utils.py", line 580, in <genexpr>
@40000000669521e3290113f4     return "".join("\\x" + format(byte, "02x") for byte in data)
@40000000669521e329012394 ValueError: Unknown format code 'x' for object of type 'str'
@40000000669521e33393839c
@40000000669521e333939724
@40000000669521e333939b0c INFO:Preparing Bluetooth for connection to BMS

How to reproduce

Add a battery with a address to the config.ini

Expected behavior

Not crash

Driver version of the currently installed driver

1

Driver version of the last known working driver

1

Venus OS device type

Raspberry Pi 2

Venus OS version

1

BMS type

JKBMS Inverter

Cell count

8

Battery count

1

Connection type

Bluetooth

Config file

[DEFAULT]

; If you want to add custom values/settings, then check the values/settings you want to change in "config.default.ini"
; and insert them below to persist future driver updates.

; Example (remove the semicolon ";" to uncomment and activate the value/setting):
; MAX_BATTERY_CHARGE_CURRENT = 50.0
; MAX_BATTERY_DISCHARGE_CURRENT = 60.0

BLUETOOTH_BMS = LiTime_Ble C8:47:8C:00:00:00

Relevant log output

se above

Any other information that may be helpful

No response

mr-manuel commented 4 months ago

Since you are not using the code of my repository, it's a bit hard to help you. Have you pushed your code at least to your repository, so I can check it?

I don't think, that this commit introduced this bug. If you are using a Bluetooth connection you have to overwrite the connection_name():

https://github.com/mr-manuel/venus-os_dbus-serialbattery/blob/ed2f95dcfba59585e460f5f8ebe9d3775b9abb03/etc/dbus-serialbattery/bms/jkbms_ble.py#L31-L32

https://github.com/mr-manuel/venus-os_dbus-serialbattery/blob/ed2f95dcfba59585e460f5f8ebe9d3775b9abb03/etc/dbus-serialbattery/bms/lltjbd_ble.py#L64-L65

calledit commented 4 months ago

Nah it is in such a early stage that I jave not even made a fork yet.

Okay did not know that. Good to know, that fixed that issue.

When that is fixed this error occurs instead:

@400000006695784403b7e1a4 Traceback (most recent call last):
@400000006695784403b7f914   File "/opt/victronenergy/dbus-serialbattery/dbus-serialbattery.py", line 287, in <module>
@400000006695784403b80c9c     main()
@400000006695784403b8146c   File "/opt/victronenergy/dbus-serialbattery/dbus-serialbattery.py", line 236, in main
@400000006695784403b827f4     if not helper.setup_vedbus():
@400000006695784403b82fc4   File "/opt/victronenergy/dbus-serialbattery/dbushelper.py", line 794, in setup_vedbus
@400000006695784403b8434c     self._dbusservice.register()
@400000006695784403b94cec   File "/opt/victronenergy/dbus-serialbattery/ext/velib_python/vedbus.py", line 91, in register
@400000006695784403b96074     self._dbusname = dbus.service.BusName(self.name, self._dbusconn, do_not_queue=True)
@400000006695784403b973fc   File "/usr/lib/python3.8/site-packages/dbus/service.py", line 112, in __new__
@400000006695784403b98784     validate_bus_name(name, allow_well_known=True, allow_unique=False)
@400000006695784403b99724 ValueError: Invalid bus name 'com.victronenergy.battery.': must not end with '.'

I solved it temporarily with a hack by adding the address argument on this line: dbus-serialbattery.py line 236 So it becomes: helper = DbusHelper(battery, "address")

Is there a similar thing (overloading some function) I should do to fix that problem the proper way?

calledit commented 4 months ago

Aha I see the error happens cause the port is not filled in here https://github.com/mr-manuel/venus-os_dbus-serialbattery/blob/ed2f95dcfba59585e460f5f8ebe9d3775b9abb03/etc/dbus-serialbattery/dbus-serialbattery.py#L187.

And the two other bluetooth BMS implementations has workarounds to get around that by ignoring the supplied port name and making up another one on this line https://github.com/mr-manuel/venus-os_dbus-serialbattery/blob/ed2f95dcfba59585e460f5f8ebe9d3775b9abb03/etc/dbus-serialbattery/bms/jkbms_ble.py#L22

Maybe not the cleanest solution. Maybe a nicer fix should be implemented in the future.

mr-manuel commented 4 months ago

I'm always open for recommendations!