Proposal: Use of pysolarmanv5 library instead of current embedded communications implementation

[charles-sd]

Hey folks

First of all, I would like to express my gratitude and appreciation to @StephanJoubert and contributors for creating a great Home Assistant component that has been very useful and valuable to many of us! Thank you also for your hard work and dedication to keep this project moving forward.

I hope you will consider this proposal as my constructive suggestion on how to improve the project.

I think that the project could benefit from a greater separation of concerns - in particular by separating the implementation of the inverter communications protocol from the functional HA component with HA Integration, Sensors and UX components. There is much potential for expanding on the latter and this approach would allow for more focused and efficient development.

After getting into the code today to squash a bug, I noticed that the implementation of the communications protocol to the inverter is still feature limited and challenging to easily grow new features on top of. (Such as updating inverter settings for example.) Enhancing this would require significant time and effort.
New and improved HA features are what I would expect the main focus of this project to be, not in refining the current inverter protocol implementation. It has already been developed to a high level of maturity in the pysolarmanv5 library led by @jmccrohan. Protocol efforts could be better consolidated under that project, instead of duplicating them here.

For this reason I'd like to propose that the code be refactored to replace the current embedded inverter protocol implementation with use of the pysolarmanv5 library. This library is well documented, feature rich and technically very well implemented.

I would love to hear your thoughts on this proposal. (And I am especially interested in hearing from the project maintainers…)

Cheers

[frsantos]

Maybe this is related? jmccrohan/pysolarmanv5#1

[charles-sd]

I would expect some alignment effort required between the projects. No such thing as a free lunch 😃

Some features could be added to the library project (without breaking the existing interfaces) that would serve the need for efficient bulk operations.
Data type issues could be dealt with through refactoring into additional facades. I have worked with many legacy PDU implementations in my professional career and a core abstraction at byte level is typically more flexible (no loss of information), with type conversions only performed in a specified presentation layer, as required by different implementations.
Would also need to consider how to use the inverter definition configuration to drive such type conversion in the library.

[frsantos]

Good news, it seems @StephanJoubert had some time to integrate with the library. The changes are already in the repository, waiting a few days before a realease. See #338

[rimelis]

Hello,

Unfortunately it's a way a less stable than previous internal implementation found in <v1.5.0

 Logger: root
 Source: /usr/src/homeassistant/homeassistant/bootstrap.py:400
 First occurred: 2:29:49 PM **(4 occurrences)**
 Last logged: 5:07:19 PM
 
 Uncaught thread exception
 Traceback (most recent call last):
   File "/usr/local/lib/python3.11/threading.py", line 1038, in _bootstrap_inner
     self.run()
   File "/usr/local/lib/python3.11/threading.py", line 975, in run
     self._target(*self._args, **self._kwargs)
   File "/usr/local/lib/python3.11/site-packages/pysolarmanv5/pysolarmanv5.py", line 298, in _data_receiver
     data = self.sock.recv(1024)
            ^^^^^^^^^^^^^^^^^^^^
 ConnectionResetError: [Errno 104] Connection reset by peer

and...

Logger: custom_components.solarman.solarman
Source: custom_components/solarman/solarman.py:61
Integration: Solarman ([documentation](https://github.com/StephanJoubert/home_assistant_solarman/blob/main/README.md), [issues](https://github.com/StephanJoubert/home_assistant_solarman/issues))
First occurred: 1:46:34 PM **(14 occurrences)**
Last logged: 5:07:34 PM

Querying [1028 - 1056] failed with exception [Empty: ]
Querying [1028 - 1056] failed, [1] retry attempts left
Querying [1156 - 1199] failed with exception [V5FrameError: V5 frame does not contain a valid Modbus RTU frame]
Querying [1156 - 1199] failed, [1] retry attempts left

I don't have a luxury to find a time fighting with these and I am going to stick with older version.

[Dummy0815]

This is an issue with https://github.com/jmccrohan/pysolarmanv5. This lib is assuming that each an every received frame start at offset 0 in the receive buffer. But unfortunately this is not the case in real world.
E.g. I'm using a Deye inverter with a connected AC-Relay. In this case the relay is issuing frequently "ATx" commands to the inverter. The answer to this commands is inherently also seen on the socket for Solarman. So pysolarmanv5 is seeing a mixture of requested v5 frames and answers to AT-commands, which the current implementation can not deal with.
I've spend some time and I think I found a solution. For those like to test this use the URL "https://github.com/Dummy0815/home_assistant_solarman.git" instead of the original in HACS. This git-repo contains a patched copy of pysolarmanv5.py which works fine for me.
I will also issue a pull request to the original pysolarmanv5 git-repo.