[modbus_server] Fix register range issues and allow partial reads (#17205)

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: pre-commit-ci-lite[bot] <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com>
Co-authored-by: J. Nick Koston <nick@koston.org>

esphome/components/modbus_server/__init__.py

@@ -8,8 +8,10 @@ from esphome.components.modbus.helpers import (
 )
 import esphome.config_validation as cv
 from esphome.const import CONF_ADDRESS, CONF_ID
+from esphome.types import ConfigType
 
 from .const import (
+    CONF_ALLOW_PARTIAL_READ,
     CONF_COURTESY_RESPONSE,
     CONF_READ_LAMBDA,
     CONF_REGISTER_LAST_ADDRESS,
@@ -41,17 +43,62 @@ SERVER_COURTESY_RESPONSE_SCHEMA = cv.Schema(
     }
 )
 
+# RAW has no numeric encoding, so it is not a valid server register type: a server value is produced by a
+# lambda and encoded into registers, and on the server a RAW register would just be a single 16-bit word --
+# use U_WORD for that. Restrict the choices to the encodable types.
+SERVER_SENSOR_VALUE_TYPE = {
+    key: value for key, value in SENSOR_VALUE_TYPE.items() if key != "RAW"
+}
+
 ModbusServerRegisterSchema = cv.Schema(
     {
         cv.GenerateID(): cv.declare_id(ServerRegister),
         cv.Required(CONF_ADDRESS): cv.hex_uint16_t,
-        cv.Optional(CONF_VALUE_TYPE, default="U_WORD"): cv.enum(SENSOR_VALUE_TYPE),
+        cv.Optional(CONF_VALUE_TYPE, default="U_WORD"): cv.enum(
+            SERVER_SENSOR_VALUE_TYPE
+        ),
         cv.Required(CONF_READ_LAMBDA): cv.returning_lambda,
         cv.Optional(CONF_WRITE_LAMBDA): cv.returning_lambda,
+        cv.Optional(CONF_ALLOW_PARTIAL_READ, default=False): cv.boolean,
     }
 )
 
 
+def _validate_register_ranges(config: ConfigType) -> ConfigType:
+    # Each register occupies [address, address + register_count); the whole span must fit inside the 16-bit
+    # Modbus address space (0x0000-0xFFFF).
+    for register in config.get(CONF_REGISTERS, []):
+        address = register[CONF_ADDRESS]
+        register_count = TYPE_REGISTER_MAP[register[CONF_VALUE_TYPE]]
+        if address + register_count > 0x10000:
+            raise cv.Invalid(
+                f"Register at 0x{address:04X} spans {register_count} register(s) and runs past "
+                "the end of the 16-bit address space (0xFFFF)",
+                path=[CONF_REGISTERS],
+            )
+    return config
+
+
+def _validate_no_overlapping_registers(config: ConfigType) -> ConfigType:
+    # Each register occupies [address, address + register_count). Reject configs where any two ranges
+    # overlap -- the same address twice, or a multi-register value straddling a neighbour -- since the
+    # server resolves a request by the value containing an address and overlaps are ambiguous.
+    spans = sorted(
+        (register[CONF_ADDRESS], TYPE_REGISTER_MAP[register[CONF_VALUE_TYPE]])
+        for register in config.get(CONF_REGISTERS, [])
+    )
+    for (address, register_count), (next_address, _) in zip(
+        spans, spans[1:], strict=False
+    ):
+        if next_address < address + register_count:
+            raise cv.Invalid(
+                f"Register address 0x{next_address:04X} overlaps the register at 0x{address:04X}, "
+                f"which spans {register_count} register(s); each register's address range must be unique",
+                path=[CONF_REGISTERS],
+            )
+    return config
+
+
 CONFIG_SCHEMA = cv.All(
     cv.Schema(
         {
@@ -62,10 +109,12 @@ CONFIG_SCHEMA = cv.All(
             ): cv.ensure_list(ModbusServerRegisterSchema),
         }
     ).extend(modbus.modbus_device_schema(0x01, role="server")),
+    _validate_register_ranges,
+    _validate_no_overlapping_registers,
 )
 
 
-def _final_validate(config):
+def _final_validate(config: ConfigType) -> ConfigType:
     return modbus.final_validate_modbus_device("modbus_server", role="server")(config)
 
 
@@ -118,6 +167,8 @@ async def to_code(config):
                         ),
                     )
                 )
+            if server_register[CONF_ALLOW_PARTIAL_READ]:
+                cg.add(server_register_var.set_allow_partial_read(True))
             cg.add(var.add_server_register(server_register_var))
     await cg.register_component(var, config)
     return await modbus.register_modbus_server_device(var, config)

esphome/components/modbus_server/const.py

@@ -5,3 +5,4 @@ CONF_COURTESY_RESPONSE = "courtesy_response"
 CONF_READ_LAMBDA = "read_lambda"
 CONF_WRITE_LAMBDA = "write_lambda"
 CONF_REGISTERS = "registers"
+CONF_ALLOW_PARTIAL_READ = "allow_partial_read"

esphome/components/modbus_server/modbus_server.cpp

@@ -8,6 +8,25 @@ using modbus::helpers::registers_to_number;
 
 static const char *const TAG = "modbus_server";
 
+// The widest Modbus value type (QWORD) spans four registers.
+static constexpr uint8_t MAX_REGISTERS_PER_VALUE = 4;
+// number_to_payload() encodes the 64-bit value returned by read_lambda() into 16-bit registers, so the
+// widest possible value spans exactly sizeof(int64_t) / sizeof(uint16_t) registers. Tie the bound to that
+// source so a future wider value type -- which would require widening the encoded value itself -- can't
+// silently overflow the value_words buffer below (StaticVector::push_back drops words past capacity).
+static_assert(MAX_REGISTERS_PER_VALUE == sizeof(int64_t) / sizeof(uint16_t),
+              "MAX_REGISTERS_PER_VALUE must match the register span of the widest encodable value");
+
+ServerRegister *ModbusServer::find_containing_register_(uint32_t address) const {
+  for (auto *server_register : this->server_registers_) {
+    if (address >= server_register->address &&
+        address < static_cast<uint32_t>(server_register->address) + server_register->register_count) {
+      return server_register;
+    }
+  }
+  return nullptr;
+}
+
 modbus::ServerResponseStatus ModbusServer::on_modbus_read_registers(uint16_t start_address,
                                                                     uint16_t number_of_registers,
                                                                     modbus::RegisterValues &registers) {
@@ -15,42 +34,68 @@ modbus::ServerResponseStatus ModbusServer::on_modbus_read_registers(uint16_t sta
            "Received read holding/input registers for device 0x%X. Start address: 0x%X. Number of registers: 0x%X.",
            this->address_, start_address, number_of_registers);
 
-  for (uint16_t current_address = start_address; current_address < start_address + number_of_registers;) {
-    bool found = false;
-    for (auto *server_register : this->server_registers_) {
-      if (server_register->address == current_address) {
-        if (!server_register->read_lambda) {
-          break;
-        }
-        int64_t value = server_register->read_lambda();
-        char value_buf[ServerRegister::FORMAT_VALUE_BUF_SIZE];
-        ESP_LOGV(TAG, "Matched register. Address: 0x%02X. Value type: %zu. Register count: %u. Value: %s.",
-                 server_register->address, static_cast<size_t>(server_register->value_type),
-                 server_register->register_count, server_register->format_value(value, value_buf, sizeof(value_buf)));
+  const uint32_t end_address = static_cast<uint32_t>(start_address) + number_of_registers;
+  uint32_t current_address = start_address;
+  while (current_address < end_address) {
+    ServerRegister *server_register = this->find_containing_register_(current_address);
 
-        modbus::helpers::number_to_payload(registers, value, server_register->value_type);
-        current_address += server_register->register_count;
-        found = true;
-        break;
-      }
-    }
-
-    if (!found) {
+    if (server_register == nullptr) {
+      // Unregistered address: optionally answer with the courtesy default, otherwise reject.
       if (this->server_courtesy_response_.enabled &&
-          (current_address <= this->server_courtesy_response_.register_last_address)) {
-        ESP_LOGV(TAG,
-                 "Could not match any register to address 0x%02X, but default allowed. "
-                 "Returning default value: %" PRIu16 ".",
-                 current_address, this->server_courtesy_response_.register_value);
+          current_address <= this->server_courtesy_response_.register_last_address) {
+        ESP_LOGV(TAG, "No register at 0x%04X; returning courtesy default %" PRIu16 ".",
+                 static_cast<uint16_t>(current_address), this->server_courtesy_response_.register_value);
         registers.push_back(this->server_courtesy_response_.register_value);
-        current_address += 1;  // Just increment by 1, as the default response is a single register
-      } else {
-        ESP_LOGW(TAG,
-                 "Could not match any register to address 0x%02X and default not allowed. Sending exception response.",
-                 current_address);
-        return ModbusExceptionCode::ILLEGAL_DATA_ADDRESS;
+        current_address += 1;  // the courtesy default is always a single register
+        continue;
       }
+      ESP_LOGW(TAG, "No register at 0x%04X and courtesy default not allowed. Sending exception response.",
+               static_cast<uint16_t>(current_address));
+      return ModbusExceptionCode::ILLEGAL_DATA_ADDRESS;
     }
+
+    if (!server_register->read_lambda) {
+      // Registered but not readable (write-only); don't mask it with the courtesy default.
+      ESP_LOGW(TAG, "Register at 0x%04X is not readable. Sending exception response.", server_register->address);
+      return ModbusExceptionCode::ILLEGAL_DATA_ADDRESS;
+    }
+
+    // A multi-register value is normally atomic: the request must start at its first register and cover all of
+    // it. A value may opt in to partial reads, in which case the request may start inside it or stop short of
+    // its end and we return only the covered words.
+    const uint16_t value_offset = static_cast<uint16_t>(current_address - server_register->address);
+    const uint16_t words_available = static_cast<uint16_t>(server_register->register_count - value_offset);
+    const uint16_t words_wanted = static_cast<uint16_t>(end_address - current_address);
+    const uint16_t take = words_available < words_wanted ? words_available : words_wanted;
+    const bool clipped = value_offset != 0 || take != server_register->register_count;
+    if (clipped && !server_register->allow_partial_read) {
+      ESP_LOGW(TAG,
+               "Read clips the multi-register value at 0x%04X, which does not allow partial reads. "
+               "Sending exception response.",
+               server_register->address);
+      return ModbusExceptionCode::ILLEGAL_DATA_ADDRESS;
+    }
+
+    int64_t value = server_register->read_lambda();
+    char value_buf[ServerRegister::FORMAT_VALUE_BUF_SIZE];
+    ESP_LOGV(TAG, "Matched register. Address: 0x%02X. Value type: %zu. Register count: %u. Value: %s.",
+             server_register->address, static_cast<size_t>(server_register->value_type),
+             server_register->register_count, server_register->format_value(value, value_buf, sizeof(value_buf)));
+
+    // Encode the whole value once (wire word order) and emit only the covered words. Slicing the encoded words
+    // handles the reversed value types for free, since number_to_payload already emits in wire order.
+    StaticVector<uint16_t, MAX_REGISTERS_PER_VALUE> value_words;
+    modbus::helpers::number_to_payload(value_words, value, server_register->value_type);
+    if (value_offset + take > value_words.size()) {
+      // The value encoded to fewer words than its register span (e.g. a RAW register); treat as a device fault.
+      ESP_LOGE(TAG, "Register at 0x%04X did not encode to %u registers", server_register->address,
+               server_register->register_count);
+      return ModbusExceptionCode::SERVICE_DEVICE_FAILURE;
+    }
+    for (uint16_t i = 0; i < take; i++) {
+      registers.push_back(value_words[value_offset + i]);
+    }
+    current_address += take;
   }
 
   return {};

esphome/components/modbus_server/modbus_server.h

@@ -84,9 +84,13 @@ class ServerRegister {
     }
   }
 
+  void set_allow_partial_read(bool allow_partial_read) { this->allow_partial_read = allow_partial_read; }
+
   uint16_t address{0};
   SensorValueType value_type{SensorValueType::RAW};
   uint8_t register_count{0};
+  // When true, a read may cover only part of this multi-register value; otherwise it must read the whole value.
+  bool allow_partial_read{false};
   ReadLambda read_lambda;
   WriteLambda write_lambda;
 };
@@ -111,6 +115,8 @@ class ModbusServer : public Component, public modbus::ModbusServerDevice {
   ServerCourtesyResponse get_server_courtesy_response() const { return this->server_courtesy_response_; }
 
  protected:
+  /// Find the registered value whose register span contains address, or nullptr if none does.
+  ServerRegister *find_containing_register_(uint32_t address) const;
   /// Collection of all server registers for this component
   std::vector<ServerRegister *> server_registers_{};
   /// Server courtesy response

tests/component_tests/modbus_server/test_modbus_server.py

@@ -0,0 +1,84 @@
+"""Tests for modbus_server configuration validation."""
+
+import pytest
+
+from esphome import config_validation as cv
+from esphome.components.modbus_server import (
+    SERVER_SENSOR_VALUE_TYPE,
+    _validate_no_overlapping_registers,
+    _validate_register_ranges,
+)
+from esphome.components.modbus_server.const import CONF_REGISTERS, CONF_VALUE_TYPE
+from esphome.const import CONF_ADDRESS
+
+
+def _config(registers: list[tuple[int, str]]) -> dict:
+    return {
+        CONF_REGISTERS: [
+            {CONF_ADDRESS: address, CONF_VALUE_TYPE: value_type}
+            for address, value_type in registers
+        ]
+    }
+
+
+def test_non_overlapping_registers_pass() -> None:
+    # Values that tile the address space without gaps or overlaps are accepted.
+    config = _config([(0x00, "U_WORD"), (0x01, "U_DWORD"), (0x03, "U_WORD")])
+    assert _validate_no_overlapping_registers(config) is config
+
+
+def test_registers_with_gaps_pass() -> None:
+    config = _config([(0x00, "U_WORD"), (0x05, "U_QWORD"), (0x20, "U_WORD")])
+    assert _validate_no_overlapping_registers(config) is config
+
+
+def test_no_registers_pass() -> None:
+    assert _validate_no_overlapping_registers({}) == {}
+
+
+def test_duplicate_address_rejected() -> None:
+    config = _config([(0x10, "U_WORD"), (0x10, "U_WORD")])
+    with pytest.raises(cv.Invalid, match="overlaps"):
+        _validate_no_overlapping_registers(config)
+
+
+def test_multi_register_value_overlapping_neighbour_rejected() -> None:
+    # U_DWORD at 0x10 occupies 0x10 and 0x11; a U_WORD at 0x11 collides with its low word.
+    config = _config([(0x10, "U_DWORD"), (0x11, "U_WORD")])
+    with pytest.raises(cv.Invalid, match="overlaps"):
+        _validate_no_overlapping_registers(config)
+
+
+def test_overlap_detected_regardless_of_order() -> None:
+    # The U_DWORD at 0x10 covers 0x10-0x11 and overlaps the U_WORD at 0x11 even when declared after it.
+    config = _config([(0x11, "U_WORD"), (0x10, "U_DWORD")])
+    with pytest.raises(cv.Invalid, match="overlaps"):
+        _validate_no_overlapping_registers(config)
+
+
+def test_register_span_within_address_space_pass() -> None:
+    # A value whose span ends exactly at 0xFFFF is fine (U_QWORD at 0xFFFC covers 0xFFFC-0xFFFF).
+    config = _config([(0xFFFF, "U_WORD"), (0xFFFC, "U_QWORD")])
+    assert _validate_register_ranges(config) is config
+
+
+def test_register_span_past_end_rejected() -> None:
+    # U_QWORD at 0xFFFE would need 0xFFFE-0x10001, running off the 16-bit address space.
+    config = _config([(0xFFFE, "U_QWORD")])
+    with pytest.raises(cv.Invalid, match="past the end"):
+        _validate_register_ranges(config)
+
+
+def test_multi_register_value_at_last_address_rejected() -> None:
+    # A U_DWORD at 0xFFFF needs a second register at 0x10000, which does not exist.
+    config = _config([(0xFFFF, "U_DWORD")])
+    with pytest.raises(cv.Invalid, match="past the end"):
+        _validate_register_ranges(config)
+
+
+def test_raw_value_type_rejected() -> None:
+    # RAW has no numeric encoding, so it is not offered as a server register type.
+    validator = cv.enum(SERVER_SENSOR_VALUE_TYPE)
+    with pytest.raises(cv.Invalid):
+        validator("RAW")
+    assert validator("U_WORD") == "U_WORD"

tests/components/modbus_server/common.yaml

@@ -18,6 +18,7 @@ modbus_server:
     registers:
       - address: 0x9
         value_type: S_DWORD
+        allow_partial_read: true
         read_lambda: |-
           return 31;
         write_lambda: |-

tests/components/modbus_server/modbus_server_test.cpp

@@ -121,4 +121,165 @@ TEST(ModbusServerWrite, CallbackFailureIsServiceDeviceFailure) {
   EXPECT_TRUE(first_written);  // pre-validation passed, so the first write applied before the failure
 }
 
+// --- on_modbus_read_registers --------------------------------------------------
+
+TEST(ModbusServerRead, SingleWordSucceeds) {
+  ModbusServer server;
+  ServerRegister reg(0x0000, SensorValueType::U_WORD, 1);
+  reg.read_lambda = []() -> int64_t { return 0x1234; };
+  server.add_server_register(&reg);
+
+  RegisterValues out;
+  auto status = server.on_modbus_read_registers(0x0000, 1, out);
+  EXPECT_FALSE(status.has_value());
+  ASSERT_EQ(out.size(), 1u);
+  EXPECT_EQ(out[0], 0x1234);
+}
+
+TEST(ModbusServerRead, DwordReturnsTwoWordsHighFirst) {
+  ModbusServer server;
+  ServerRegister reg(0x0000, SensorValueType::U_DWORD, 2);
+  reg.read_lambda = []() -> int64_t { return 0x12345678; };
+  server.add_server_register(&reg);
+
+  RegisterValues out;
+  auto status = server.on_modbus_read_registers(0x0000, 2, out);
+  EXPECT_FALSE(status.has_value());
+  ASSERT_EQ(out.size(), 2u);
+  EXPECT_EQ(out[0], 0x1234);
+  EXPECT_EQ(out[1], 0x5678);
+}
+
+// Starting inside a multi-register value is rejected with ILLEGAL_DATA_ADDRESS -- not masked by the courtesy
+// default -- and the read_lambda is never invoked.
+TEST(ModbusServerRead, StartInsideValueRejected) {
+  ModbusServer server;
+  bool read_called = false;
+  ServerRegister reg(0x0010, SensorValueType::U_DWORD, 2);  // occupies 0x0010 and 0x0011
+  reg.read_lambda = [&read_called]() -> int64_t {
+    read_called = true;
+    return 0;
+  };
+  server.set_server_courtesy_response(
+      ServerCourtesyResponse{.enabled = true, .register_last_address = 0xFFFF, .register_value = 0xABCD});
+  server.add_server_register(&reg);
+
+  RegisterValues out;
+  auto status = server.on_modbus_read_registers(0x0011, 1, out);  // the second cell of the DWORD
+  ASSERT_TRUE(status.has_value());
+  if (status.has_value())
+    EXPECT_EQ(status.value(), ModbusExceptionCode::ILLEGAL_DATA_ADDRESS);
+  EXPECT_FALSE(read_called);
+}
+
+// A read that stops short of a value's end clips it -> ILLEGAL_DATA_ADDRESS, and the read_lambda is not invoked.
+TEST(ModbusServerRead, ClippedTailRejected) {
+  ModbusServer server;
+  bool read_called = false;
+  ServerRegister reg(0x0000, SensorValueType::U_DWORD, 2);
+  reg.read_lambda = [&read_called]() -> int64_t {
+    read_called = true;
+    return 0;
+  };
+  server.add_server_register(&reg);
+
+  RegisterValues out;
+  auto status = server.on_modbus_read_registers(0x0000, 1, out);  // only 1 of the DWORD's 2 registers
+  ASSERT_TRUE(status.has_value());
+  if (status.has_value())
+    EXPECT_EQ(status.value(), ModbusExceptionCode::ILLEGAL_DATA_ADDRESS);
+  EXPECT_FALSE(read_called);
+}
+
+// A write-only register (no read_lambda) is not readable -> ILLEGAL_DATA_ADDRESS, not a courtesy default.
+TEST(ModbusServerRead, WriteOnlyRegisterRejected) {
+  ModbusServer server;
+  ServerRegister reg(0x0000, SensorValueType::U_WORD, 1);  // no read_lambda set
+  server.set_server_courtesy_response(
+      ServerCourtesyResponse{.enabled = true, .register_last_address = 0xFFFF, .register_value = 0xABCD});
+  server.add_server_register(&reg);
+
+  RegisterValues out;
+  auto status = server.on_modbus_read_registers(0x0000, 1, out);
+  ASSERT_TRUE(status.has_value());
+  if (status.has_value())
+    EXPECT_EQ(status.value(), ModbusExceptionCode::ILLEGAL_DATA_ADDRESS);
+}
+
+// An unregistered address with courtesy enabled returns the default value for each cell.
+TEST(ModbusServerRead, CourtesyDefaultForUnregistered) {
+  ModbusServer server;
+  server.set_server_courtesy_response(
+      ServerCourtesyResponse{.enabled = true, .register_last_address = 0xFFFF, .register_value = 0xABCD});
+
+  RegisterValues out;
+  auto status = server.on_modbus_read_registers(0x0005, 2, out);
+  EXPECT_FALSE(status.has_value());
+  ASSERT_EQ(out.size(), 2u);
+  EXPECT_EQ(out[0], 0xABCD);
+  EXPECT_EQ(out[1], 0xABCD);
+}
+
+// An unregistered address with courtesy disabled is rejected.
+TEST(ModbusServerRead, UnregisteredRejectedWithoutCourtesy) {
+  ModbusServer server;
+  RegisterValues out;
+  auto status = server.on_modbus_read_registers(0x0005, 1, out);
+  ASSERT_TRUE(status.has_value());
+  if (status.has_value())
+    EXPECT_EQ(status.value(), ModbusExceptionCode::ILLEGAL_DATA_ADDRESS);
+}
+
+// --- partial reads (opt-in) ----------------------------------------------------
+
+// With allow_partial_read, reading only the first register of a DWORD returns its high word.
+TEST(ModbusServerRead, PartialReadHighWord) {
+  ModbusServer server;
+  ServerRegister reg(0x0010, SensorValueType::U_DWORD, 2);
+  reg.allow_partial_read = true;
+  reg.read_lambda = []() -> int64_t { return 0x12345678; };
+  server.add_server_register(&reg);
+
+  RegisterValues out;
+  auto status = server.on_modbus_read_registers(0x0010, 1, out);
+  EXPECT_FALSE(status.has_value());
+  ASSERT_EQ(out.size(), 1u);
+  EXPECT_EQ(out[0], 0x1234);
+}
+
+// With allow_partial_read, starting at the interior cell returns the low word.
+TEST(ModbusServerRead, PartialReadLowWordFromInterior) {
+  ModbusServer server;
+  ServerRegister reg(0x0010, SensorValueType::U_DWORD, 2);
+  reg.allow_partial_read = true;
+  reg.read_lambda = []() -> int64_t { return 0x12345678; };
+  server.add_server_register(&reg);
+
+  RegisterValues out;
+  auto status = server.on_modbus_read_registers(0x0011, 1, out);
+  EXPECT_FALSE(status.has_value());
+  ASSERT_EQ(out.size(), 1u);
+  EXPECT_EQ(out[0], 0x5678);
+}
+
+// Slicing is in wire order, so a reversed value type partials correctly: U_DWORD_R emits the low word
+// first, so 0x0010 holds 0x5678 and 0x0011 holds 0x1234.
+TEST(ModbusServerRead, PartialReadReversedType) {
+  ModbusServer server;
+  ServerRegister reg(0x0010, SensorValueType::U_DWORD_R, 2);
+  reg.allow_partial_read = true;
+  reg.read_lambda = []() -> int64_t { return 0x12345678; };
+  server.add_server_register(&reg);
+
+  RegisterValues first;
+  ASSERT_FALSE(server.on_modbus_read_registers(0x0010, 1, first).has_value());
+  ASSERT_EQ(first.size(), 1u);
+  EXPECT_EQ(first[0], 0x5678);
+
+  RegisterValues second;
+  ASSERT_FALSE(server.on_modbus_read_registers(0x0011, 1, second).has_value());
+  ASSERT_EQ(second.size(), 1u);
+  EXPECT_EQ(second[0], 0x1234);
+}
+
 }  // namespace esphome::modbus_server