[seaspotter]

#4226 (reply in thread)

Problem

write_register(..., data_type=ModbusDataType.INT_16) scheitert bei negativen Werten - betrifft weiterhin den Lambda-Verbraucher (#4269) und unabhängig davon ego/ego/consumer.py:25, das einen literalen -1 über denselben Pfad schreibt, bei jedem Aufruf.

Ursache

write_register() behandelt Datentypen mit bits <= 16 (außer Float) als "einfach" und reicht den rohen Wert ungekodiert an pymodbus durch - das scheitert bei negativen Werten, da die Zweierkomplement-Kodierung fehlt, die _build_binary_payload() für alle anderen Typen bereits korrekt übernimmt.

Fix

Kodiert jetzt immer über _build_binary_payload(), sobald data_type angegeben ist.

Summary by Sourcery

Ensure typed Modbus register writes correctly encode negative signed values.

Bug Fixes:

• Fix writing negative values for small signed Modbus data types such as INT_16 and INT_8 by applying the required binary encoding before transmission.

Tests:

• Add coverage ensuring negative INT_16 values are encoded and sent as register data.

[sourcery-ai]

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

The PR fixes negative INT_16/INT_8 writes by routing every explicitly typed value through _build_binary_payload(), while retaining raw writes without a data type and adding regression coverage for two’s-complement encoding.

Sequence diagram for typed Modbus register writes

sequenceDiagram
    participant Caller
    participant Modbus as ModbusClient
    participant Builder as BinaryPayloadBuilder
    participant Delegate as pymodbusDelegate

    Caller->>Modbus: write_register(address, value, data_type)
    Modbus->>Builder: _build_binary_payload(value, data_type, byteorder, wordorder)
    Builder-->>Modbus: two's-complement registers
    Modbus->>Delegate: write_registers(address, registers)

Loading

Flow diagram for typed and untyped register writes

flowchart TD
    A[write_register] --> B{data_type is provided?}
    B -->|Yes| C[_build_binary_payload]
    C --> D[write_registers with encoded registers]
    B -->|No| E[write_registers with raw value]

Loading

File-Level Changes

Change	Details	Files
Always encode explicitly typed register values through the binary payload builder before writing them.	Removed the direct-write shortcut for 16-bit and smaller data types. Ensured negative INT_16/INT_8 values receive two’s-complement encoding expected by pymodbus. Preserved the raw-value fallback when no data type is provided.	packages/modules/common/modbus.py
Added regression coverage for negative small integer writes.	Verified that a negative INT_16 value is converted to its unsigned register representation before delegation. Confirmed the encoded register write is issued through write_registers with the original address and options.	packages/modules/common/modbus_test.py

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey - I've found 2 issues

Prompt for AI Agents

Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="packages/modules/common/modbus.py" line_range="226" />
<code_context>
-                if data_type.bits > 16 or data_type in [ModbusDataType.FLOAT_16,
-                                                        ModbusDataType.FLOAT_32,
-                                                        ModbusDataType.FLOAT_64]:
-                    registers = self._build_binary_payload(value, data_type, byteorder, wordorder)
-                    self._delegate.write_registers(address, registers, **kwargs)
-                else:
</code_context>
<issue_to_address>
**Negative INT_8 values read back incorrectly**

When a negative INT_8 value other than -1 is written and later decoded as INT_8 with the default big-endian settings, `write_register()` encodes `INT_8` with `add_16bit_int` as a sign-extended 16-bit register, while `__read_registers()` decodes only the first byte with `decode_8bit_int`, so values such as -5 read back as -1.

Encode `INT_8` values in a register representation that `decode_8bit_int` reads back correctly.
</issue_to_address>

### Comment 2
<location path="packages/modules/common/modbus_test.py" line_range="81" />
<code_context>
+    delegate.is_socket_open.return_value = True
+
+    client = ModbusClient(delegate, "test", 502)
+    client.write_register(1000, -500, data_type=ModbusDataType.INT_16, unit=1)
+
+    delegate.write_registers.assert_called_once_with(1000, [65036], unit=1)
</code_context>
<issue_to_address>
**Modbus test assertion fails**

When the added test runs with the repository's normal `packages/modules/conftest.py` fixture, `BinaryPayloadBuilder.to_registers()` returns an unconstrained `Mock`, so `_build_binary_payload()` passes that mock to the delegate instead of `[65036]` and the assertion fails, preventing the modules test suite from passing.

Configure the test fixture or test builder so `to_registers()` returns the expected register list.

Also at `packages/modules/common/modbus_test.py:83`.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook