diff --git a/esphome/components/modbus/modbus_helpers.cpp b/esphome/components/modbus/modbus_helpers.cpp index 92bd06cdf5..d80e6c86ad 100644 --- a/esphome/components/modbus/modbus_helpers.cpp +++ b/esphome/components/modbus/modbus_helpers.cpp @@ -292,25 +292,52 @@ std::optional payload_to_number(const uint8_t *data, size_t size, Senso } std::optional registers_to_number(const uint16_t *registers, size_t count, SensorValueType sensor_value_type) { - const size_t required_size = required_payload_size(sensor_value_type); - if (required_size == 0) { - return 0; // RAW/unsupported: nothing to read + // RAW and BIT carry no fixed-width number, so there is nothing to decode whatever the span holds. + // register_width_for() reports 1 for them, so this must be checked before the width test below. + if (sensor_value_type == SensorValueType::RAW || sensor_value_type == SensorValueType::BIT) { + return 0; } - const size_t required_words = required_size / 2; + const uint16_t required_words = register_width_for(sensor_value_type); if (required_words > count) { - ESP_LOGE(TAG, "not enough registers for value type=%u count=%zu required=%zu", - static_cast(sensor_value_type), count, required_words); + ESP_LOGE(TAG, "not enough registers for value type=%u count=%zu required=%u", + static_cast(sensor_value_type), count, static_cast(required_words)); return std::nullopt; } - // Serialize the needed words back to big-endian bytes and reuse the audited byte decoder so the - // sign-extension behaviour stays identical to the wire path. - uint8_t bytes[8]; // at most 4 registers (QWORD) - for (size_t i = 0; i < required_words; i++) { - uint16_t reg = registers[i]; - bytes[i * 2] = static_cast(reg >> 8); - bytes[i * 2 + 1] = static_cast(reg & 0xFF); + // Registers are the wire's own unit, so decode them directly rather than serializing back to bytes. + // Each case defers to registers_to_value() so the word order and sign rules have one definition, with + // two deliberate exceptions matching what the byte decoder returned: the float types yield their bit + // pattern rather than a float, and U_QWORD shares the signed branch because the return type is int64_t. + switch (sensor_value_type) { + case SensorValueType::U_WORD: + return registers_to_value(registers); + case SensorValueType::U_WORD_S: + return registers_to_value(registers); + case SensorValueType::S_WORD: + return registers_to_value(registers); + case SensorValueType::S_WORD_S: + return registers_to_value(registers); + case SensorValueType::U_DWORD: + return registers_to_value(registers); + case SensorValueType::U_DWORD_R: + return registers_to_value(registers); + case SensorValueType::S_DWORD: + return registers_to_value(registers); + case SensorValueType::S_DWORD_R: + return registers_to_value(registers); + case SensorValueType::FP32: + return registers_to_uint32(registers[0], registers[1]); + case SensorValueType::FP32_R: + return registers_to_uint32(registers[1], registers[0]); + // Signed for both: an unsigned QWORD above INT64_MAX has to come back as a negative int64_t. + case SensorValueType::U_QWORD: + case SensorValueType::S_QWORD: + return registers_to_value(registers); + case SensorValueType::U_QWORD_R: + case SensorValueType::S_QWORD_R: + return registers_to_value(registers); + default: + return 0; } - return payload_to_number(bytes, required_size, sensor_value_type, 0, 0xFFFFFFFF); } // Append a 16-bit value to a PDU in big-endian (wire) byte order. diff --git a/esphome/components/modbus/modbus_helpers.h b/esphome/components/modbus/modbus_helpers.h index 486064da01..9488a88088 100644 --- a/esphome/components/modbus/modbus_helpers.h +++ b/esphome/components/modbus/modbus_helpers.h @@ -229,7 +229,7 @@ inline bool value_type_is_float(SensorValueType v) { } /// Number of 16-bit registers a value of this type occupies (RAW counts as one register). -inline uint16_t register_width_for(SensorValueType v) { +constexpr uint16_t register_width_for(SensorValueType v) { switch (v) { case SensorValueType::U_DWORD: case SensorValueType::S_DWORD: @@ -478,6 +478,11 @@ constexpr uint32_t registers_to_uint32(uint16_t high_word, uint16_t low_word) { return (static_cast(high_word) << 16) | low_word; } +/// Combine four register words into a 64-bit value, most significant word first. +constexpr uint64_t registers_to_uint64(uint16_t word0, uint16_t word1, uint16_t word2, uint16_t word3) { + return (static_cast(registers_to_uint32(word0, word1)) << 32) | registers_to_uint32(word2, word3); +} + // Always false, whatever the type: it exists only to make the static_assert below depend on the // template argument. Not a queryable trait. template inline constexpr bool VALUE_TYPE_SUPPORTED = false; @@ -486,8 +491,8 @@ template inline constexpr bool VALUE_TYPE_SUPPORTED = false; * Unlike registers_to_number(), the type is a template argument, so only the one decode is compiled * and the caller gets the value's natural type back rather than an int64_t. The "_R" types take the * low word first; the rest take the high word first. - * Supports the WORD, DWORD and FP32 types, including their _S and _R forms; the QWORD types are - * out of scope and fail to compile, so use registers_to_number() for those. + * Supports every fixed-width type: the WORD, DWORD, QWORD and FP32 families, including their _S and + * _R forms. RAW and BIT have no fixed width and fail to compile. * Use register_width_for() for the number of registers the caller must supply. * Note that the FP32 branches are only usable in a constant expression where std::bit_cast is * available; elsewhere bit_cast falls back to a non-constexpr memcpy (see core/helpers.h). @@ -513,11 +518,42 @@ template constexpr auto registers_to_value(const uin return bit_cast(registers_to_uint32(registers[0], registers[1])); } else if constexpr (VALUE_TYPE == SensorValueType::FP32_R) { return bit_cast(registers_to_uint32(registers[1], registers[0])); + } else if constexpr (VALUE_TYPE == SensorValueType::U_QWORD) { + return registers_to_uint64(registers[0], registers[1], registers[2], registers[3]); + } else if constexpr (VALUE_TYPE == SensorValueType::U_QWORD_R) { + return registers_to_uint64(registers[3], registers[2], registers[1], registers[0]); + } else if constexpr (VALUE_TYPE == SensorValueType::S_QWORD) { + return static_cast(registers_to_uint64(registers[0], registers[1], registers[2], registers[3])); + } else if constexpr (VALUE_TYPE == SensorValueType::S_QWORD_R) { + return static_cast(registers_to_uint64(registers[3], registers[2], registers[1], registers[0])); } else { static_assert(VALUE_TYPE_SUPPORTED, "registers_to_value() does not support this value type"); } } +/// The type registers_to_value() yields for a given value type. Distinct from modbus::RegisterValues, +/// which is a container of raw words. +template +using RegisterValueType = decltype(registers_to_value(static_cast(nullptr))); + +/** The value stored at an absolute register address, or nullopt when it is not wholly inside this + * response. Lets a device decode by address rather than by offset, so a poll split across several + * requests needs no extra bookkeeping: a value outside the response simply yields nullopt. + * @param registers the response registers, in host byte order + * @param start_address the address the response begins at + * @param address the address of the wanted value + */ +template +constexpr std::optional> value_at(std::span registers, + uint16_t start_address, uint16_t address) { + if (address < start_address) + return std::nullopt; + const size_t offset = static_cast(address) - start_address; + if (offset + register_width_for(VALUE_TYPE) > registers.size()) + return std::nullopt; + return registers_to_value(registers.data() + offset); +} + /// The widest standard numeric value (a QWORD) spans 4 registers, so one entity value never writes more. static constexpr uint16_t MAX_FEW_REGISTERS = 4; diff --git a/tests/components/modbus/modbus_helpers_test.cpp b/tests/components/modbus/modbus_helpers_test.cpp index 21c264ea69..a42625760d 100644 --- a/tests/components/modbus/modbus_helpers_test.cpp +++ b/tests/components/modbus/modbus_helpers_test.cpp @@ -427,14 +427,37 @@ TEST(ModbusHelpersTest, RegistersToNumberMatchesPayloadToNumber) { } } +TEST(ModbusHelpersTest, RegistersToNumberMatchesPayloadToNumberForQwords) { + // The word shuffle the QWORD_R decode replaces is the least obvious code in the byte path, so pin + // it against that path rather than against registers_to_value(). The top bit is set, which is where + // U_QWORD's unsigned value and this function's int64_t return deliberately diverge. + const uint16_t registers[] = {0xF123, 0x4567, 0x89AB, 0xCDEF}; + const std::vector bytes{0xF1, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, 0xEF}; + for (auto value_type : + {SensorValueType::U_QWORD, SensorValueType::S_QWORD, SensorValueType::U_QWORD_R, SensorValueType::S_QWORD_R}) { + EXPECT_EQ(registers_to_number(registers, 4, value_type), + payload_to_number(std::span(bytes), value_type, 0, 0xFFFFFFFF)) + << "value_type=" << static_cast(value_type); + } +} + +TEST(ModbusHelpersTest, RegistersToNumberTreatsRawAndBitAsNothingToDecode) { + // Both have no fixed-width number, so they decode to 0 whatever the span holds - including none. + const uint16_t registers[] = {0x1234}; + EXPECT_EQ(registers_to_number(registers, 1, SensorValueType::RAW), std::optional(0)); + EXPECT_EQ(registers_to_number(registers, 0, SensorValueType::RAW), std::optional(0)); + EXPECT_EQ(registers_to_number(registers, 0, SensorValueType::BIT), std::optional(0)); +} + TEST(ModbusHelpersTest, RegistersToNumberRejectsTruncatedMultiRegisterValue) { const uint16_t registers[] = {0x1234}; EXPECT_FALSE(registers_to_number(registers, 1, SensorValueType::U_DWORD).has_value()); } // --- registers_to_value ---------------------------------------------------- -// The compile-time decoder must agree with the runtime one for every type it supports, -// so the two implementations cannot drift apart. +// registers_to_number() dispatches to registers_to_value(), so this checks the dispatch table picks +// the right specialisation for each type, not that two implementations agree. The independent check +// against the byte decoder is RegistersToNumberMatchesPayloadToNumber below. template void expect_matches_registers_to_number(const uint16_t *registers) { const auto expected = registers_to_number(registers, register_width_for(VALUE_TYPE), VALUE_TYPE); @@ -472,6 +495,64 @@ TEST(ModbusHelpersTest, RegistersToUint32CombinesWordsHighFirst) { EXPECT_EQ(registers_to_uint32(0x1234, 0x5678), 0x12345678u); } +// --- value_at --------------------------------------------------------------- +// Addresses are absolute; anything not wholly inside the response yields nullopt. + +TEST(ModbusHelpersTest, ValueAtDecodesByAbsoluteAddress) { + const uint16_t registers[] = {0x1111, 0x2222, 0x3333}; + const std::span span(registers, 3); + EXPECT_EQ(value_at(span, 100, 100), std::optional(0x1111)); + EXPECT_EQ(value_at(span, 100, 102), std::optional(0x3333)); + EXPECT_EQ(value_at(span, 100, 101), std::optional(0x22223333u)); + // Types whose RegisterValueType<> is not an unsigned integer, and the widest bounds check. + const uint16_t floats[] = {0x4048, 0xF5C3, 0xF5C3, 0x4048}; + const std::span float_span(floats, 4); + EXPECT_FLOAT_EQ(value_at(float_span, 10, 10).value_or(0.0f), 3.14f); + EXPECT_FLOAT_EQ(value_at(float_span, 10, 12).value_or(0.0f), 3.14f); + EXPECT_EQ(value_at(float_span, 10, 10), std::optional(0x4048F5C3F5C34048ULL)); + EXPECT_FALSE(value_at(float_span, 10, 11).has_value()); +} + +TEST(ModbusHelpersTest, ValueAtIsUsableInAConstantExpression) { + static constexpr uint16_t REGISTERS[] = {0x1234, 0x5678}; + static_assert(value_at(REGISTERS, 7, 7).value_or(0) == 0x12345678u); + static_assert(!value_at(REGISTERS, 7, 6).has_value()); +} + +TEST(ModbusHelpersTest, ValueAtRejectsAddressesOutsideTheResponse) { + const uint16_t registers[] = {0x1111, 0x2222, 0x3333}; + const std::span span(registers, 3); + // Below the response: must not wrap when the subtraction would go negative. + EXPECT_FALSE(value_at(span, 100, 99).has_value()); + EXPECT_FALSE(value_at(span, 100, 0).has_value()); + // Past the end, and a multi-register value truncated by the end of the response. + EXPECT_FALSE(value_at(span, 100, 103).has_value()); + EXPECT_FALSE(value_at(span, 100, 102).has_value()); + EXPECT_TRUE(value_at(span, 100, 101).has_value()); +} + +TEST(ModbusHelpersTest, ValueAtHandlesAnEmptyResponse) { + EXPECT_FALSE(value_at(std::span(), 0, 0).has_value()); +} + +// --- QWORD decoding --------------------------------------------------------- + +TEST(ModbusHelpersTest, RegistersToValueDecodesQwordBothWordOrders) { + const uint16_t registers[] = {0x0123, 0x4567, 0x89AB, 0xCDEF}; + EXPECT_EQ(registers_to_value(registers), 0x0123456789ABCDEFULL); + const uint16_t reversed[] = {0xCDEF, 0x89AB, 0x4567, 0x0123}; + EXPECT_EQ(registers_to_value(reversed), 0x0123456789ABCDEFULL); + // Signed reading of the same bits, and the sign-extreme case. + EXPECT_EQ(registers_to_value(registers), 0x0123456789ABCDEFLL); + const uint16_t negative[] = {0xFFFF, 0xFFFF, 0xFFFF, 0xFFFE}; + EXPECT_EQ(registers_to_value(negative), -2); + EXPECT_EQ(registers_to_value(negative), 0xFFFFFFFFFFFFFFFEULL); +} + +TEST(ModbusHelpersTest, RegistersToUint64CombinesWordsHighFirst) { + EXPECT_EQ(registers_to_uint64(0x0123, 0x4567, 0x89AB, 0xCDEF), 0x0123456789ABCDEFULL); +} + // --- packed bit helpers ------------------------------------------------------ TEST(ModbusHelpersTest, PackBitsAppendsToContainer) {