-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[solarman] Enable write registers #19420
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
Conversation
Signed-off-by: Oleksandr Mishchuk <olexandr.mishchuk@gmail.com>
3ec51cb
to
6a4d5c4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds write register functionality to the Solarman binding, allowing users to modify inverter settings in addition to reading data. By default, all registers remain read-only unless explicitly marked as writable with isReadOnly: false
in channel configuration.
Key changes include:
- Implementation of write register functionality in both SolarmanV5 and SolarmanRaw protocols
- Addition of SolarmanRegisterUpdater for handling write commands across different data types
- New inverter definition
deye_sg01hp3
with Time Of Use (TOU) writable channels
Reviewed Changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 6 comments.
Show a summary per file
File | Description |
---|---|
SolarmanV5Protocol.java | Added write register support and refactored frame building methods |
SolarmanRawProtocol.java | Added write register support for raw protocol |
SolarmanRegisterUpdater.java | New class handling register writes for different data types |
SolarmanLoggerHandler.java | Integrated register updater and command handling |
ParameterItem.java | Added isReadOnly field to control write permissions |
deye_sg01hp3.yaml | New inverter definition with writable TOU channels |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
...arman/src/test/java/org/openhab/binding/solarman/internal/modbus/SolarmanV5ProtocolTest.java
Outdated
Show resolved
Hide resolved
bundles/org.openhab.binding.solarman/src/main/resources/definitions/deye_sg01hp3.yaml
Show resolved
Hide resolved
bundles/org.openhab.binding.solarman/src/main/resources/definitions/deye_sg01hp3.yaml
Outdated
Show resolved
Hide resolved
bundles/org.openhab.binding.solarman/src/main/resources/definitions/deye_sg01hp3.yaml
Outdated
Show resolved
Hide resolved
...man/src/main/java/org/openhab/binding/solarman/internal/updater/SolarmanRegisterUpdater.java
Show resolved
Hide resolved
...ding.solarman/src/main/java/org/openhab/binding/solarman/internal/SolarmanLoggerHandler.java
Show resolved
Hide resolved
6a4d5c4
to
51727d7
Compare
I highly appreciate this PR. |
51727d7
to
9f8d78d
Compare
I have updated readme.md now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see any channel changes in the readme? So no new channels have been added?
Btw, I have SUN-8K-SG04LP3-EU which works well with the binding.
I have added new invertor type where this was tested - |
9f8d78d
to
d4cf7c0
Compare
I was referring to what you wrote in the description:
I might have overlooked them in the Readme? 🤔 |
d4cf7c0
to
a9bb5c4
Compare
I have added |
a9bb5c4
to
b3731f3
Compare
Signed-off-by: Oleksandr Mishchuk <olexandr.mishchuk@gmail.com>
b3731f3
to
4234b0a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thansk for providing this PR.
LGTM
@stefan-hoehn do you need to finsih some tests?
No, sorry, I won't be able to do this before it will be rolled out. Thanks for asking. |
@san4esmc ready for a merge? |
Yep |
[solarman] Add ability to write to Solarman registers
Description
Current binding implementation doesn't allow to change data in invertor - only reads data from it. At the same time there are sets of registers that can safely be manipulated to automate eletricity usage, etc.
By default all registers are treated as read-only unless other is directly stated in channel configuration, which should avoid any accidental writes.
To make channel writable you should add readOnly=false to channel configuration. I have also added one new invertor definition
deye_sg01hp3
for Deye 30KWh High Volatage inverter, where this was tested. New channels introduced there are connected to Time Of Use functionality, including time periods, battery SOC, charge enabled.Testing
Existing unit tests were adapted to refactored code.
To test updated binding you can download built jar here: https://mega.nz/file/jjAlAT4D#mFLoSH1T8NP36iZzJKroslgVaMFkOTtW1ve7hfJ7KnY