Link device instance to unique_identifier instead of port

[mr-manuel]

Don't link the device instance to the port name, but to the unique_identifier.

This prevents that the batteries change sometimes device instance and MQTT battery ID causing missing history in the VRM portal and unfunctioning nodes in Node-RED. The root cause is that sometimes on reboot or unplug/plug of the USB to serial adapter the ttyUSB* number changes.

• It has to be verified that the unique_identifier is not already used (some BMS does not have one, then it's concatenated from hardware version and battery capacity:
  

  dbus-serialbattery/etc/dbus-serialbattery/battery.py

  Lines 138 to 151 in 4af10dd

  	def unique_identifier(self) -> str:
  	"""
  	Used to identify a BMS when multiple BMS are connected
  	If not provided by the BMS/driver then the hardware version and capacity is used,
  	since it can be changed by small amounts to make a battery unique.
  	On +/- 5 Ah you can identify 11 batteries
  	"""
  	string = (
  	"".join(filter(str.isalnum, str(self.hardware_version))) + "_"
  	if self.hardware_version is not None and self.hardware_version != ""
  	else ""
  	)
  	string += str(self.capacity) + "Ah"
  	return string

• If the unique_identifier exists, prevent the driver from starting and mention in the logfile that the capacity of multiple connected batteries has do differ.

Anything else?

[mr-manuel]

@ogurevich maybe you have time to look into this?

[ogurevich]

I'll try to understand that tomorrow and have a couple of thoughts

Do all BMS have a serialNumber ?

[mr-manuel]

The value serial is fed by the code linked above, so yes all BMS display a serial, which is in the most cases not the real serial but a concatenated value that tries to be unique.

[ogurevich]

I'm still thinking about what can be a reasonable solution here.

At the moment three tasks can be solved, that I believe the desired behavior can be realized. We can consider together how this can be implemented. We need at least:

• a function that saves the last seen (used) batteries and the corresponding assigned IDs somewhere in VRM.
• a function that builds the UniqueID for recognized devices when the driver is started and tries to find it by comparing stored information, then we also have the ID, which can then be assigned again.
• the third function would be responsible for notifying the user in case of a conflict. Writing to the log as already suggested in the topic and perhaps also as an info alarm. In this way, the user is informed in any case.

Would an SQLite database, for example, be conceivable for this purpose?

[mr-manuel]

1. The data is already stored in the dbus under com.victronenergy.settings
2. There is already a function for the unique identifier, but not for the rest

[ogurevich]

yes, thank you, i see. There are: Enabled, Name (unique Name), Service
The idea is what if we could store a little more information about a battery, no matter where, also the info: "last seen" and "last seen SOC", so we can match the battery to the name when the driver starts make smarter. We can compare different factors and thus assign the right battery. In case the formed UniqueName result in the same values. This is still a rough idea, it should mature.

[ogurevich]

ok, I entered the value: PrivDatagur01 in the User Private Data field. This is how the values in the screenshots result

[mr-manuel]

Yes, thats correct. Since on serial connection I found no way to read the serial number.

[ogurevich]

In my setup, the BMSs are also connected via USB 👍

[arrow1800]

any updates on this? would love to have a fix for this. very hard to read bms data with changing device instances

[mr-manuel]

It‘s a work in progress since it takes a lot of time. You a very welcome to open a PR for this.

[arrow1800]

It‘s a work in progress since it takes a lot of time. You a very welcome to open a PR for this.

fair enough : - ).

I'm still thinking about what can be a reasonable solution here.

At the moment three tasks can be solved, that I believe the desired behavior can be realized. We can consider together how this can be implemented. We need at least:

• a function that saves the last seen (used) batteries and the corresponding assigned IDs somewhere in VRM.
• a function that builds the UniqueID for recognized devices when the driver is started and tries to find it by comparing stored information, then we also have the ID, which can then be assigned again.
• the third function would be responsible for notifying the user in case of a conflict. Writing to the log as already suggested in the topic and perhaps also as an info alarm. In this way, the user is informed in any case.

Would an SQLite database, for example, be conceivable for this purpose?

can't we construct or calculate a device id based on any of the already existing properties instead of trying to save data and rematching that to the existing BMS on reboot? maybe even try to generate a hash of 2 numeric characters in total based on a few properties

[arrow1800]

something like this:

import hashlib

def generate_limited_hash(input_data, min_value, max_value):
    sha256_hash = hashlib.sha256()

    # Convert the input to bytes
    input_bytes = str(input_data).encode('utf-8')

    # Update the hash 
    sha256_hash.update(input_bytes)

    # Get the resulting hash value as a hexadecimal string
    hash_result = sha256_hash.hexdigest()

    # Convert the hexadecimal hash to a decimal number
    decimal_hash = int(hash_result, 16)

    # Limit the hash to the specified range
    limited_hash = min_value + (decimal_hash % (max_value - min_value + 1))

    return limited_hash


input_data = "a combination of the unique i3dent4ifier and some other fields"

min_value = 10
max_value = 50

hash_value = generate_limited_hash(input_data, min_value, max_value)
print(f"Input: {input_data}, Limited Hash: {hash_value}")

[mr-manuel]

I see no difference in this. Since you cannot save the hash to the BMS it makes no sense for me. You have to recalculate the hash based on the unique data we try to generate.

[arrow1800]

ok, so the problem is there is no unique data? what about the first post, a combination of hardware id and capacity

[mr-manuel]

Yes the problem is, that some BMS do not have unique data. This is already solved since a few months:

dbus-serialbattery/etc/dbus-serialbattery/battery.py

Lines 144 to 157 in 9b476a2

	def unique_identifier(self) -> str:
	"""
	Used to identify a BMS when multiple BMS are connected
	If not provided by the BMS/driver then the hardware version and capacity is used,
	since it can be changed by small amounts to make a battery unique.
	On +/- 5 Ah you can identify 11 batteries
	"""
	string = (
	"".join(filter(str.isalnum, str(self.hardware_version))) + "_"
	if self.hardware_version is not None and self.hardware_version != ""
	else ""
	)
	string += str(self.capacity) + "Ah"
	return string

[arrow1800]

i am a bit confused now; so we already have a unique identifier method that is able to unique identify a BMS. that is good, what are we missing then ?

could this strategy work?

1. generate a unique identifier for each bms using the already implemented method above unique_identifier()
2. translate the return value of this method to a unique and fixed VRM ID using the hash method above.
3. set hash as VRM ID, this way we dont have to store the VRM ID as it can just be generated everytime the drivers boots up

[mr-manuel]

• a function that builds the UniqueID for recognized devices when the driver is started and tries to find it by comparing stored information, then we also have the ID, which can then be assigned again.

• the third function would be responsible for notifying the user in case of a conflict. Writing to the log as already suggested in the topic and perhaps also as an info alarm. In this way, the user is informed in any case.

This is missing.

1. When a BMS is attached the driver has to check, if the BMS has already assigned a VRM ID, if not then it has to assign the next free VRM ID and store this connection somewhere with an expiration date.
2. If the BMS is not connected for x days, then the BMS with the VRM ID should be removed from the list.
3. If a BMS with the same ID is already connected the driver has to throw an error.

[arrow1800]

yea but that is my whole point. if we calculate the vrm id we dont have to store anything.

[mr-manuel]

VRM ID is the wrong name, is the device instance and has to be a value between 1 and 255. Calculating a hash for that low possibilities causes more problems than storing the data somewhere.

Did you try your algorithm with some random data and see how often values are the same?

[arrow1800]

i can write a simulation. can you give me some possible values for the field 'hardware version' ? i only have one type of BMS

[mr-manuel]

You have to check some issues here on GitHub and the logs.

[ramack]

I can give you some from my 4 Heltec BMSes

[ramack]

In case there is a hash-generated instance number which is already used, we could still fall back to
a) increasing numbers
b) using the USB port device name
c) integrating the capacity

[mr-manuel]

We want to to this to prevent using the USB port device name

[arrow1800]

I can give you some from my 4 Heltec BMSes ![Screenshot_20231124-161107](https://private-user-

thnx!

and ah! the Heltec BMS.. that one actually has a serial number that can be read :)

serial1 = mbdev.read_registers(2, number_of_registers=4)

self.unique_identifier = "-".join("{:04x}".format(x) for x in serial1)

[ramack]

We want to to this to prevent using the USB port device name

Clearly, but my impression is, that it will work for most systems perfectly with the hasing and only where this clashes we'd need a backup.

[arrow1800]

looked into it a bit more.

one more question @mr-manuel; the method you mentioned in the first post "unique_identifier()" doesnt seem to be used anywhere in code, is that right?

instead every specific BMS class inherits from the battery class and has its own implementation in which the unique_identifier property/variable is set. however not every BMS class seems to implement this fully (only 4 classes are setting the property for now). #link

so whats missing here is that in the base class (battery) the unique_identifier property should be set to a default value with the unique_identifier method, which can then be overwritten later on by a specific BMS class if implemented.

so this line: link

should change to:

self.unique_identifier = unique_identifier()

so at least all BMS classes have some form of a unique_identifier set. and in the case that a serial number or a custom field can actually be read from the BMS the default value can be overwritten with a more accurate value from the bms itself.

at the end; the unique_identifier property is written to the dbus path /serial.

so now we have a baseline for which i can build a simulation. if i'm still correct so far, so please confirm :)

[mr-manuel]

@arrow1800 can you join our Discord server?

[arrow1800]

@arrow1800 can you join our Discord server?

accepted

[mr-manuel]

This was solved and will be merged with the dev branch in the next days.