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

ModbusRecordChannel: fix read error for write-only channels #2256

Conversation

tsicking
Copy link
Contributor

@tsicking tsicking commented Jul 7, 2023

An Edge2EdgeEss tried to read out the write-only channels, causing a read error. This is fixed by setting the modbus register of a write-only channel to "undefined" (like it is done for the registers in between natures).

@tsicking
Copy link
Contributor Author

May I ask for a review on this and the related PR #2257 ? We use the Edge2Edge-Ess in read-write-mode in two productive systems, and without those PRs it wouldn't work. Thanks.

# Conflicts:
#	io.openems.edge.common/src/io/openems/edge/common/modbusslave/ModbusRecordChannel.java
@sfeilmeier
Copy link
Contributor

May I ask for a review on this and the related PR #2257 ? We use the Edge2Edge-Ess in read-write-mode in two productive systems, and without those PRs it wouldn't work. Thanks.

Sury. In fact the problem had been solved in f0b0527#diff-bb02b130c9bfd04e1cdb847b3f138cd028aef03a1770b6bfc9a6bde6894f26ceR120-R124 before - but your solution is better.

I merged the changes here in this PR, added a JUnit test and migrated the entire file to switch expressions.

@sfeilmeier sfeilmeier self-requested a review October 1, 2023 19:40
@github-actions
Copy link

github-actions bot commented Oct 1, 2023

Code Coverage

@sfeilmeier sfeilmeier changed the title Read error on write-only channels removed. ModbusRecordChannel: fix read error for write-only channels Oct 1, 2023
@sfeilmeier sfeilmeier merged commit 55602f7 into OpenEMS:develop Oct 1, 2023
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

Successfully merging this pull request may close these issues.

2 participants