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

Compatibility with US5000 battery #41

Open
Benoit3 opened this issue Mar 3, 2024 · 4 comments
Open

Compatibility with US5000 battery #41

Benoit3 opened this issue Mar 3, 2024 · 4 comments

Comments

@Benoit3
Copy link

Benoit3 commented Mar 3, 2024

I belong a bank with a unique U5000C Battery. I use default settings for DIP switch.

get_protocol_version, get_manufacturer_info, get_values are inoperative and return exception.

For example with get_protocol_version:
`

import pylontech
p = pylontech.Pylontech()
print(p.get_protocol_version())
Traceback (most recent call last):
File "", line 1, in
File "/home/domotic/python-pylontech/pylontech/pylontech.py", line 242, in get_protocol_version
return self.read_frame()
File "/home/domotic/python-pylontech/pylontech/pylontech.py", line 215, in read_frame
f = self._decode_hw_frame(raw_frame=raw_frame)
File "/home/domotic/python-pylontech/pylontech/pylontech.py", line 196, in _decode_hw_frame
assert got_frame_checksum == int(frame_chksum, 16)
ValueError: invalid literal for int() with base 16: b''

`

I think issue is coming from the fact that adr=0 is not authorized and should be set to the right value (eg 0x02 in simple/default case case). With this correction, the result is:
'

import pylontech
p = pylontech.Pylontech()
print(p.get_protocol_version())
Container:
ver = b' ' (total 1)
adr = b'\x02' (total 1)
cid1 = b'F' (total 1)
cid2 = b'\x00' (total 1)
infolength = b'\x00\x00' (total 2)
info = b'' (total 0)
'
The protocol value seems to be 'space' in Ascii, so 0x20 or V2.0

I need to do same correction for get_manufacturer_info.

For get_values, it's seems to be an other problem, it's seems battery doesn't manage the "system information case"

@Benoit3
Copy link
Author

Benoit3 commented Mar 3, 2024

One question: are usually system information command answered with one bank US2000 battery ?

@Frankkkkk
Copy link
Owner

Hi,
Have you checked #35 and #23 (comment) ?
Cheers

@Benoit3
Copy link
Author

Benoit3 commented Mar 5, 2024

Hello,

Thanks. Yes it is a duplicate of #23.

But with an additionnal suggestion :-) :

  • it's seems that the usage of adr=0 is not in the spec, check §2.5.3 page 11 (adr values are always >=2 , even for system information requests (page 11 & page 12)
    I can propose a PR with corrections but I can not check if modification stays compatible with legacy US2000 battery, as I don't own anyone.
    Regards

@Frankkkkk
Copy link
Owner

Hi, yeah that would be great :-) Maybe it would be better to edit the README and add a "US2000 vs US5000" section with some code examples ?
Thanks and cheers !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants