Fix IllegalArgumentException by da-Kai · Pull Request #17 · OpenEMS/j2mod

da-Kai · 2026-07-29T16:05:26Z

Fix serial timing and refactor transmission delay handling

Changes:

Fix nanosecond timeout value out of range exceptions in Thread.sleep()
Fix serial throughput calculation by including start bit and fix end-bit retreval
Extract sleep fudge factors into named constants (LONG_DELAY_FUDGE_FACTOR, SHORT_DELAY_FUDGE_FACTOR)
Add getBitsPerCharacter() and getStopBits() to AbstractSerialConnection
Add baud rate validation (>= 1) to prevent division-by-zero
Refactor SerialParameters constructors to use proper delegation chain
Rename DEFAULT_RS485_TX_DURING_RX → DEFAULT_RS485_RX_DURING_TX for clarity

make shure milli and nano values are in range

* move `getBitsPerCharacter` to `AbstractSerialConnection` * fix stopBits value

- Fix getCharIntervalMicro() to correctly return microseconds (was returning nanoseconds due to using NS_IN_A_MS instead of MICROS_IN_A_SEC) - Refactor SerialParameters constructors to use proper delegation (no-arg → full-arg chain) - Add baud rate validation (must be >= 1) to prevent division-by-zero in timing calculations - Rename DEFAULT_RS485_TX_DURING_RX to DEFAULT_RS485_RX_DURING_TX for clarity - Fix spurious Thread.currentThread().interrupt() calls on non-InterruptedException paths - Minor log message improvements for waitForTransmission

michaelgrill

🚀 (only some minor comments)

- Replace thread priority manipulation with a hybrid wait strategy (sleep -> yield -> busy spin). - Anchor target end time dynamically using System.nanoTime() inside waitForTransmission. - Prevent thread starvation and reduce syscall overhead during sub-millisecond delays. - Improve IllegalArgumentException message in SerialParameters for invalid baud rate.

Align serial timing with MODBUS over Serial Line v1.02 by using a 750µs inter-character timeout for baud rates above 19200 and keeping 1750µs for inter-frame delay. Renames `getMaxCharDelay()` to `getMaxCharTimeout()` and updates RTU receive logic to use the correct timeout value. Also simplifies the nanos wait loop to sleep until close to target, then busy-wait for precise timing.

Reduce CPU-heavy waiting in `ModbusSerialTransport` by yielding the thread before the final busy-spin for longer transmission delays. This keeps the precise timing behavior while giving the scheduler a chance to run other work.

Howaner · 2026-08-17T13:23:58Z

+     *
+     * @return Stop-bit length in bit times.
+     */
+    public float getStopBits() {


Das Naming passt nicht. Es werden nicht die StopBits zurückgegeben, sondern die Bit Times.

Ich hätte es so gelassen. In der Modbus Serial Specification wird auch nicht zwischen Stop Bits und Stop Bit Times unterschieden.
Ich habe den Zusatz "in bit times." ergänzt, um den 1,5 wert zu erklären. Aber es ist heute sowieso eher unüblich den zu verwenden.

Howaner · 2026-08-17T13:31:07Z

+                    Thread.sleep(0);
+                }
+            }
+            while (System.nanoTime() < targetEndNanos) {


Das verursacht 100% CPU Auslastung auf einem Kern, solange es läuft. Sollte man, wie früher, davor und danach die Thread Priorität auf LOW stellen?

Wenn überhaupt, würde ich eher Thread.yield() verwenden. Aber damit nimmt man sich die Genauigkeit.

Auf der anderen Seite hat die aktuelle implementierung die factoren x1,3 bzw. x1,7 drin. Also ist das Timing sowieso nicht exakt.

Leider ist im Code nirgends dokumentiert, wofür diese Fudge-Faktoren gedacht sind oder wie sie ermittelt wurden.

Aber wo ruft man Thread.yield() auf? Thread.yield() in jedem while Durchlauf auszuführen ist auch nicht gut. Macht es vllt. Sinn j2mod auf Java 21 upzudaten und die neue Thread.sleep() Implementierung mit dem nanoseconds Übergabeparameter zu nutzen?

Thread.sleep(millis, nanos) gibt es auch in Java 8 schon. Ein sleep() schickt den Thread aber immer an den OS-Scheduler und ist deshalb oft um mehrere Millisekunden ungenau, egal was man an Nanos übergibt. Java nutzt die Nanos eher zum Runden.

Für echte nano- oder mikrosekunden-Genauigkeit kommen wir um ein busy-wait nicht herum.

Zum entlasten im busy-wait sehe ich zwei möglichkeiten:

Thread.yield(): Da gibt es aber keine Garantie, was das OS daraus macht. Es könnte Compute freigeben, oder sich wie ein reiner Busy-Wait verhalten. Ist wieder völlig OS abhängig.

Thread.onSpinWait(): Dafür müssten wir auf Java 9 Upgraden. Damit kann man innerhalb des busy-wait der CPU signalisieren, dass sie compute freigeben kann.

Du kannst hier gerne was ergänzen, falls du mehr weißt, aber das ist, was ich mir bisher schon zusammen gereimt habe.

Howaner · 2026-08-17T13:34:29Z

+     *
+     * @return Total bit-times per character.
+     */
+    public double getBitsPerCharacter() {


Das Naming ist nicht gut. Bei getBitsPerCharacter wird wahrscheinlich an die Data Bits gedacht. Besser wäre getBitsToTransferPerCharacter()

Es sind ja die Bits, die ein Character für die Übertragung braucht. Da es ja um Datenübertragung geht, würde ich Bit Times und Bits in diesem Zusammenhang gleichsetzen.

Es geht nicht um den Kommentar, sondern um den Funktionsnamen. Weil es eben um die Übertragung geht, würde ich die Funktion getBitsToTransferPerCharacter() nennen. Kommentare ließt niemand.

Ich verstehe, das Clean-Code möglichst präzise namen bevorzugt. Ich würde aber Argumentieren, dass es im Kontext von AbstractSerialConnection keinen Unterschied zwischen getBitsToTransferPerCharacter und getBitsPerCharacter gibt und ich dann eher zum einfachern Namen greifen würde.

Gleichzeitig wäre es mir aber auch nicht besonders wichtig. @michaelgrill als unparteiischer Dritter?

fix

8f1141b

make shure milli and nano values are in range

da-Kai requested review from Copilot and removed request for Copilot July 31, 2026 06:54

Copilot started reviewing on behalf of da-Kai July 31, 2026 06:54 View session

da-Kai added 2 commits July 31, 2026 09:00

quickfix

c352e3c

improve

b802934

da-Kai requested a lite review from Copilot August 11, 2026 10:27

Copilot started reviewing on behalf of da-Kai August 11, 2026 10:27 View session

This comment was marked as outdated.

Sign in to view

da-Kai added 4 commits August 11, 2026 12:34

use NS_IN_A_MS

4fefda0

Update ModbusSerialTransport.java

bd12151

further improve

d1f89d5

improve

e295920

* move `getBitsPerCharacter` to `AbstractSerialConnection` * fix stopBits value

da-Kai requested a lite review from Copilot August 12, 2026 14:48

Copilot started reviewing on behalf of da-Kai August 12, 2026 14:52 View session

This comment was marked as outdated.

Sign in to view

da-Kai added 2 commits August 12, 2026 17:28

quickgix

e93dcd8

quickfix

994c3ce

da-Kai requested a lite review from Copilot August 12, 2026 17:09

Copilot started reviewing on behalf of da-Kai August 12, 2026 17:15 View session

This comment was marked as outdated.

Sign in to view

da-Kai added 2 commits August 12, 2026 20:46

catch RuntimeExceptions

7d6a41b

da-Kai requested a review from michaelgrill August 13, 2026 11:40

michaelgrill approved these changes Aug 13, 2026

View reviewed changes

Comment thread src/main/java/com/ghgande/j2mod/modbus/io/ModbusSerialTransport.java Outdated

Comment thread src/main/java/com/ghgande/j2mod/modbus/util/SerialParameters.java Outdated

da-Kai added 4 commits August 13, 2026 20:24

add tests

2c5ab6f

Improve serial transport delay yielding

d1a7baa

Reduce CPU-heavy waiting in `ModbusSerialTransport` by yielding the thread before the final busy-spin for longer transmission delays. This keeps the precise timing behavior while giving the scheduler a chance to run other work.

Howaner requested changes Aug 17, 2026

View reviewed changes

da-Kai and others added 2 commits August 17, 2026 22:29

replace magic numbers with sleep constants

d6c02f9

cleanup

a55ab83

Uh oh!

Conversation

da-Kai commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

michaelgrill left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

da-Kai commented Jul 29, 2026 •

edited

Loading