From 8bd9f213e5d0d37ea045ed2e1996f7c36ce36e92 Mon Sep 17 00:00:00 2001 From: Marek Pilch <47844572+marpi82@users.noreply.github.com> Date: Tue, 4 Aug 2026 18:17:27 +0200 Subject: [PATCH] [modbus] Add unit tests for U_WORD_S and S_WORD_S (#17831) --- esphome/components/modbus/helpers.py | 6 +++ esphome/components/modbus/modbus_helpers.cpp | 12 +++++ esphome/components/modbus/modbus_helpers.h | 8 ++- .../components/modbus/modbus_helpers_test.cpp | 51 +++++++++++++++++++ .../components/modbus_controller/common.yaml | 7 +++ 5 files changed, 83 insertions(+), 1 deletion(-) diff --git a/esphome/components/modbus/helpers.py b/esphome/components/modbus/helpers.py index e3029b2648..e7eaacee0c 100644 --- a/esphome/components/modbus/helpers.py +++ b/esphome/components/modbus/helpers.py @@ -38,7 +38,9 @@ SensorValueType = SensorValueType_ns.enum("SensorValueType") SENSOR_VALUE_TYPE = { "RAW": SensorValueType.RAW, "U_WORD": SensorValueType.U_WORD, + "U_WORD_S": SensorValueType.U_WORD_S, "S_WORD": SensorValueType.S_WORD, + "S_WORD_S": SensorValueType.S_WORD_S, "U_DWORD": SensorValueType.U_DWORD, "U_DWORD_R": SensorValueType.U_DWORD_R, "S_DWORD": SensorValueType.S_DWORD, @@ -54,7 +56,9 @@ SENSOR_VALUE_TYPE = { TYPE_REGISTER_MAP = { "RAW": 1, "U_WORD": 1, + "U_WORD_S": 1, "S_WORD": 1, + "S_WORD_S": 1, "U_DWORD": 2, "U_DWORD_R": 2, "S_DWORD": 2, @@ -70,7 +74,9 @@ TYPE_REGISTER_MAP = { CPP_TYPE_REGISTER_MAP = { "RAW": cg.uint16, "U_WORD": cg.uint16, + "U_WORD_S": cg.uint16, "S_WORD": cg.int16, + "S_WORD_S": cg.int16, "U_DWORD": cg.uint32, "U_DWORD_R": cg.uint32, "S_DWORD": cg.int32, diff --git a/esphome/components/modbus/modbus_helpers.cpp b/esphome/components/modbus/modbus_helpers.cpp index 8428ea27ea..2c87928e9f 100644 --- a/esphome/components/modbus/modbus_helpers.cpp +++ b/esphome/components/modbus/modbus_helpers.cpp @@ -177,7 +177,9 @@ bool is_client_pdu_standard(const uint8_t *pdu, size_t size) { static size_t required_payload_size(SensorValueType sensor_value_type) { switch (sensor_value_type) { case SensorValueType::U_WORD: + case SensorValueType::U_WORD_S: case SensorValueType::S_WORD: + case SensorValueType::S_WORD_S: return 2; case SensorValueType::U_DWORD: case SensorValueType::FP32: @@ -228,6 +230,11 @@ std::optional payload_to_number(const uint8_t *data, size_t size, Senso case SensorValueType::U_WORD: value = mask_and_shift_by_rightbit(get_data(data, offset), bitmask); // default is 0xFFFF ; break; + case SensorValueType::U_WORD_S: { + uint16_t word = byteswap(get_data(data, offset)); + value = mask_and_shift_by_rightbit(word, bitmask); + break; + } case SensorValueType::U_DWORD: case SensorValueType::FP32: value = get_data(data, offset); @@ -242,6 +249,11 @@ std::optional payload_to_number(const uint8_t *data, size_t size, Senso case SensorValueType::S_WORD: value = mask_and_shift_by_rightbit(get_data(data, offset), bitmask); // default is 0xFFFF ; break; + case SensorValueType::S_WORD_S: { + uint16_t word = byteswap(get_data(data, offset)); + value = mask_and_shift_by_rightbit(static_cast(word), bitmask); + break; + } case SensorValueType::S_DWORD: value = mask_and_shift_by_rightbit(get_data(data, offset), bitmask); break; diff --git a/esphome/components/modbus/modbus_helpers.h b/esphome/components/modbus/modbus_helpers.h index 3dd933c4d7..89e9a2b8ea 100644 --- a/esphome/components/modbus/modbus_helpers.h +++ b/esphome/components/modbus/modbus_helpers.h @@ -119,7 +119,9 @@ enum class SensorValueType : uint8_t { U_QWORD_R = 0xA, S_QWORD_R = 0xB, FP32 = 0xC, - FP32_R = 0xD + FP32_R = 0xD, + U_WORD_S = 0xE, // 1 Register unsigned, bytes swapped + S_WORD_S = 0xF, // 1 Register signed, bytes swapped }; inline bool value_type_is_float(SensorValueType v) { @@ -284,6 +286,10 @@ template void number_to_payload(Container &data, int64_t val case SensorValueType::S_WORD: data.push_back(value & 0xFFFF); break; + case SensorValueType::U_WORD_S: + case SensorValueType::S_WORD_S: + data.push_back(byteswap(static_cast(value & 0xFFFF))); + break; case SensorValueType::U_DWORD: case SensorValueType::S_DWORD: case SensorValueType::FP32: diff --git a/tests/components/modbus/modbus_helpers_test.cpp b/tests/components/modbus/modbus_helpers_test.cpp index 49de4f9d14..553ec163b2 100644 --- a/tests/components/modbus/modbus_helpers_test.cpp +++ b/tests/components/modbus/modbus_helpers_test.cpp @@ -331,6 +331,7 @@ TEST(ModbusCreateClientPdu, WriteCoilsUseTheCoilLimitNotTheRegisterLimit) { EXPECT_TRUE(create_client_pdu(FC::WRITE_MULTIPLE_COILS, 0x0000, 1969, big.data(), big.size()).empty()); } +// --- payload_to_number ----------------------------------------------------- TEST(ModbusHelpersTest, PayloadToNumberRejectsOffsetAtEndOfBuffer) { const std::vector data{0x12, 0x34}; EXPECT_FALSE(payload_to_number(std::span(data), SensorValueType::U_WORD, 2, 0xFFFFFFFF).has_value()); @@ -346,6 +347,28 @@ TEST(ModbusHelpersTest, PayloadToNumberDecodesValidWord) { EXPECT_EQ(payload_to_number(std::span(data), SensorValueType::U_WORD, 0, 0xFFFFFFFF), 0x1234); } +TEST(ModbusHelpersTest, PayloadToNumberDecodesSwappedUnsignedWord) { + const std::vector data{0x34, 0x12}; + EXPECT_EQ(payload_to_number(std::span(data), SensorValueType::U_WORD_S, 0, 0xFFFFFFFF), 0x1234); +} + +TEST(ModbusHelpersTest, PayloadToNumberDecodesSwappedSignedWord) { + const std::vector data{0xFE, 0xFF}; + EXPECT_EQ(payload_to_number(std::span(data), SensorValueType::S_WORD_S, 0, 0xFFFFFFFF), -2); +} + +TEST(ModbusHelpersTest, PayloadToNumberAppliesBitmaskAfterSwap) { + // Bytes {0x34,0x12} decode as U_WORD_S to 0x1234; mask 0xFF00 then right-shift by bit 8 -> 0x12 + const std::vector data{0x34, 0x12}; + EXPECT_EQ(payload_to_number(std::span(data), SensorValueType::U_WORD_S, 0, 0xFF00), 0x12); +} + +TEST(ModbusHelpersTest, PayloadToNumberAppliesBitmaskAfterSwapSigned) { + // Bytes {0x34,0xFE} decode as S_WORD_S to 0xFE34 (negative); mask 0x00F0 then right-shift by bit 4 -> 0x3 + const std::vector data{0x34, 0xFE}; + EXPECT_EQ(payload_to_number(std::span(data), SensorValueType::S_WORD_S, 0, 0x00F0), 0x3); +} + // --- registers_to_number --------------------------------------------------- // Register words are host byte order; results must match the byte-based payload_to_number. @@ -354,6 +377,16 @@ TEST(ModbusHelpersTest, RegistersToNumberDecodesWord) { EXPECT_EQ(registers_to_number(registers, 1, SensorValueType::U_WORD), 0x1234); } +TEST(ModbusHelpersTest, RegistersToNumberDecodesSwappedUnsignedWord) { + const uint16_t registers[] = {0x3412}; + EXPECT_EQ(registers_to_number(registers, 1, SensorValueType::U_WORD_S), 0x1234); +} + +TEST(ModbusHelpersTest, RegistersToNumberDecodesSwappedSignedWord) { + const uint16_t registers[] = {0xFEFF}; + EXPECT_EQ(registers_to_number(registers, 1, SensorValueType::S_WORD_S), -2); +} + TEST(ModbusHelpersTest, RegistersToNumberDecodesDwordHighWordFirst) { const uint16_t registers[] = {0x1234, 0x5678}; EXPECT_EQ(registers_to_number(registers, 2, SensorValueType::U_DWORD), 0x12345678); @@ -434,6 +467,24 @@ TEST(ModbusTypedBuilders, FloatToPayloadAppendsToExistingContent) { EXPECT_EQ(data[1], 0x0001); } +// --- number_to_payload ----------------------------------------------------- + +TEST(ModbusHelpersTest, NumberToPayloadRoundTripsSwappedUnsignedWord) { + std::vector regs; + number_to_payload(regs, 0x1234, SensorValueType::U_WORD_S); + ASSERT_EQ(regs.size(), 1u); + EXPECT_EQ(regs[0], 0x3412); + EXPECT_EQ(registers_to_number(regs.data(), regs.size(), SensorValueType::U_WORD_S), 0x1234); +} + +TEST(ModbusHelpersTest, NumberToPayloadRoundTripsSwappedSignedWord) { + std::vector regs; + number_to_payload(regs, -2, SensorValueType::S_WORD_S); + ASSERT_EQ(regs.size(), 1u); + EXPECT_EQ(regs[0], 0xFEFF); + EXPECT_EQ(registers_to_number(regs.data(), regs.size(), SensorValueType::S_WORD_S), -2); +} + TEST(ModbusCreateClientPdu, ExceptionFlaggedWriteCodesRejected) { // is_function_code_write() masks the exception bit; the builder must not. const uint8_t values[] = {0x00, 0x0B, 0x00, 0x16}; diff --git a/tests/components/modbus_controller/common.yaml b/tests/components/modbus_controller/common.yaml index a0db1e7888..986d807dfb 100644 --- a/tests/components/modbus_controller/common.yaml +++ b/tests/components/modbus_controller/common.yaml @@ -41,6 +41,13 @@ number: return x * 2.0; write_lambda: |- return x / 2.0; + # Covers Python value-type maps + read/write path for byte-swapped words + - platform: modbus_controller + modbus_controller_id: modbus_controller1 + id: modbus_number3 + name: Test Number Swapped Word + address: 0x9003 + value_type: U_WORD_S output: - platform: modbus_controller