Skip to content

Conversation

san4esmc
Copy link
Contributor

@san4esmc san4esmc commented Oct 1, 2025

[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

Signed-off-by: Oleksandr Mishchuk <olexandr.mishchuk@gmail.com>
@san4esmc san4esmc force-pushed the solarman_write_registers branch from 3ec51cb to 6a4d5c4 Compare October 1, 2025 17:49
@lsiepel lsiepel changed the title Solarman write registers [solarman] Enable write registers Oct 1, 2025
@lsiepel lsiepel requested a review from Copilot October 1, 2025 18:50
@lsiepel lsiepel added the enhancement An enhancement or new feature for an existing add-on label Oct 1, 2025
Copy link

@Copilot Copilot AI left a 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.

@san4esmc san4esmc force-pushed the solarman_write_registers branch from 6a4d5c4 to 51727d7 Compare October 1, 2025 19:02
@stefan-hoehn
Copy link
Contributor

I highly appreciate this PR.
Please don't forget to update the Readme.md (including describing new channels).

@san4esmc san4esmc force-pushed the solarman_write_registers branch from 51727d7 to 9f8d78d Compare October 5, 2025 10:55
@san4esmc
Copy link
Contributor Author

san4esmc commented Oct 5, 2025

I highly appreciate this PR. Please don't forget to update the Readme.md (including describing new channels).

I have updated readme.md now.
Also added ignoring refresh command (as we use polling to get data), so we don't spam logs with warnings about writable channel not found

Copy link
Contributor

@stefan-hoehn stefan-hoehn left a 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.

@san4esmc
Copy link
Contributor Author

san4esmc commented Oct 5, 2025

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 - deye_sg01hp3 and in readme.md we have registers example for deye_sg04lp3. I can add new registers to that one also, but we'll need someone to test that everything is working there also

@san4esmc san4esmc force-pushed the solarman_write_registers branch from 9f8d78d to d4cf7c0 Compare October 5, 2025 13:20
@stefan-hoehn
Copy link
Contributor

I was referring to what you wrote in the description:

"New channels introduced there are connected to Time Of Use functionality, including time periods, battery SOC, charge enabled."

I might have overlooked them in the Readme? 🤔

@san4esmc san4esmc force-pushed the solarman_write_registers branch from d4cf7c0 to a9bb5c4 Compare October 6, 2025 06:28
@san4esmc
Copy link
Contributor Author

san4esmc commented Oct 6, 2025

I have added Time Of Use registers to deye_sg04lp3 also now (they should work similar to deye_sg01hp3, but someone still needs to check it 😄). Also added those new channels to readme.md

@san4esmc san4esmc force-pushed the solarman_write_registers branch from a9bb5c4 to b3731f3 Compare October 6, 2025 07:32
@san4esmc san4esmc requested a review from stefan-hoehn October 11, 2025 19:31
Signed-off-by: Oleksandr Mishchuk <olexandr.mishchuk@gmail.com>
@san4esmc san4esmc force-pushed the solarman_write_registers branch from b3731f3 to 4234b0a Compare October 17, 2025 07:54
Copy link
Contributor

@lsiepel lsiepel left a 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?

@stefan-hoehn
Copy link
Contributor

No, sorry, I won't be able to do this before it will be rolled out. Thanks for asking.

@lsiepel
Copy link
Contributor

lsiepel commented Oct 21, 2025

@san4esmc ready for a merge?

@san4esmc
Copy link
Contributor Author

Yep

@lsiepel lsiepel merged commit d998a26 into openhab:main Oct 21, 2025
2 checks passed
@lsiepel lsiepel added this to the 5.1 milestone Oct 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement An enhancement or new feature for an existing add-on

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants