Skip to content

Changed visibility modifiers and moved code into seperate functions#15

Merged
da-Kai merged 7 commits into
OpenEMS:developmentfrom
Howaner:feature/visibility-modifier-changes
Apr 16, 2026
Merged

Changed visibility modifiers and moved code into seperate functions#15
da-Kai merged 7 commits into
OpenEMS:developmentfrom
Howaner:feature/visibility-modifier-changes

Conversation

@Howaner
Copy link
Copy Markdown

@Howaner Howaner commented Mar 5, 2026

This makes it easier to unit test and it allows the injection of additional request/response types

@da-Kai da-Kai self-requested a review March 5, 2026 15:15
Copy link
Copy Markdown
Collaborator

@da-Kai da-Kai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Howaner
If you just want to test the changes first, I can have a snapshot built for you. However, Maven snapshots are only stored temporarily, so we would need to coordinate that.

If you’re not already familiar with it, I can also show you how to build the package locally and install it into your local Maven repo for testing.

Comment thread src/main/java/com/ghgande/j2mod/modbus/util/SerialParameters.java Outdated
@Howaner
Copy link
Copy Markdown
Author

Howaner commented Mar 6, 2026

@Howaner If you just want to test the changes first, I can have a snapshot built for you. However, Maven snapshots are only stored temporarily, so we would need to coordinate that.

If you’re not already familiar with it, I can also show you how to build the package locally and install it into your local Maven repo for testing.

I'm currently testing with a local OSGI repo (cnf/release), but it's a bit of pain to edit the index.xml file manually.
It's not regenerated automaticly because I'm not using eclipse.

I would be very grateful if you could show me how to do it with a local Maven repository.
I tried it with my local maven cache, but then had OSGI problems with it.

This makes it easier to unit test and it allows the injection of additional request/response types
@Howaner Howaner force-pushed the feature/visibility-modifier-changes branch from 7bcee9a to dffc181 Compare March 6, 2026 10:00
@Howaner Howaner requested a review from da-Kai March 6, 2026 10:39
Copy link
Copy Markdown
Collaborator

@da-Kai da-Kai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Howaner
Grundsätzlich solide. Ich würde jedoch die bestehenden Funktionen und Konstruktoren so weit wie möglich beibehalten, um einen späteren Backport in das Upstream-Repository zu erleichtern.

Comment thread src/main/java/com/ghgande/j2mod/modbus/io/ModbusRTUTransport.java
Comment thread src/main/java/com/ghgande/j2mod/modbus/slave/ModbusSlave.java
Comment thread src/main/java/com/ghgande/j2mod/modbus/slave/ModbusSlave.java
@da-Kai da-Kai requested a review from michaelgrill April 9, 2026 13:41
Comment thread src/main/java/com/ghgande/j2mod/modbus/slave/ModbusSlave.java
Comment thread src/main/java/com/ghgande/j2mod/modbus/io/ModbusTransaction.java Outdated
Copy link
Copy Markdown
Collaborator

@da-Kai da-Kai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Howaner
Noch eine Kleinigkeit, ansonsten würde ich mergen und einen Release erstellen, sobald @michaelgrill auch approved hat.
Ich geh davon aus, bei deinem Test hat alles soweit funktioniert.

Comment thread src/main/java/com/ghgande/j2mod/modbus/io/ModbusSerialTransport.java Outdated
@Howaner Howaner requested a review from michaelgrill April 15, 2026 09:36
Copy link
Copy Markdown

@michaelgrill michaelgrill left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

Copy link
Copy Markdown
Collaborator

@da-Kai da-Kai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hab doch noch was gefunden.

Comment thread src/main/java/com/ghgande/j2mod/modbus/slave/ModbusSlaveFactory.java Outdated
@Howaner Howaner requested a review from da-Kai April 16, 2026 11:17
@da-Kai da-Kai merged commit 7aee431 into OpenEMS:development Apr 16, 2026
1 check passed
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.

3 participants