[modbus] Build single-value register writes on a right-sized stack buffer (#18844)

Co-authored-by: J. Nick Koston <nick@koston.org>
This commit is contained in:
Bonne Eggleston
2026-08-28 18:36:24 +00:00
committed by GitHub
co-authored by J. Nick Koston
parent 768ab5b672
commit 59397b4e28
6 changed files with 61 additions and 3 deletions
+3
View File
@@ -534,6 +534,9 @@ class ModbusClientDevice {
return this->queue_pdu(helpers::create_write_single_coil_pdu(address, value)); return this->queue_pdu(helpers::create_write_single_coil_pdu(address, value));
} }
bool write_multiple_registers(uint16_t start_address, std::span<const uint16_t> values) { bool write_multiple_registers(uint16_t start_address, std::span<const uint16_t> values) {
// Empty goes to the full-size builder so the rejection log names this method's limit, not the small one's.
if (!values.empty() && values.size() <= helpers::MAX_FEW_REGISTERS)
return this->queue_pdu(helpers::create_write_few_registers_pdu(start_address, values));
return this->queue_pdu(helpers::create_write_registers_pdu(start_address, values)); return this->queue_pdu(helpers::create_write_registers_pdu(start_address, values));
} }
/// Note: std::vector<bool> cannot bind to std::span<const bool>; use a contiguous bool container or the packed /// Note: std::vector<bool> cannot bind to std::span<const bool>; use a contiguous bool container or the packed
@@ -116,6 +116,9 @@ static constexpr uint16_t MAX_RAW_SIZE = 254; // Max RAW size is 256 - CRC(2) =
static constexpr uint16_t READ_PDU_SIZE = 5; static constexpr uint16_t READ_PDU_SIZE = 5;
// A single-write PDU is always function code(1) + address(2) + value(2) // A single-write PDU is always function code(1) + address(2) + value(2)
static constexpr uint16_t WRITE_SINGLE_PDU_SIZE = 5; static constexpr uint16_t WRITE_SINGLE_PDU_SIZE = 5;
// A multiple-write PDU starts with function code(1) + start address(2) + quantity(2) + byte count(1),
// followed by two bytes per register.
static constexpr uint16_t WRITE_MULTIPLE_HEADER_SIZE = 6;
static constexpr uint16_t MAX_FRAME_SIZE = 256; static constexpr uint16_t MAX_FRAME_SIZE = 256;
// 4.1 Address 0 is the broadcast address: the request is processed by every device and never answered. // 4.1 Address 0 is the broadcast address: the request is processed by every device and never answered.
+19 -3
View File
@@ -485,9 +485,12 @@ static bool register_block_in_range(const LogString *role, uint16_t start_addres
return true; return true;
} }
PduBuffer create_write_registers_pdu(uint16_t start_address, std::span<const uint16_t> values) { // The ceiling comes from the buffer itself: push_back() drops silently, so a bound wider than the buffer
PduBuffer pdu; // declared before every return so NRVO fires (all paths return the same object) // would put a truncated frame on the wire.
if (!register_block_in_range(LOG_STR("Write"), start_address, values.size(), MAX_NUM_OF_REGISTERS_TO_WRITE)) { template<typename Pdu> static Pdu build_write_registers_pdu(uint16_t start_address, std::span<const uint16_t> values) {
constexpr auto max_registers = static_cast<uint16_t>((Pdu::capacity() - WRITE_MULTIPLE_HEADER_SIZE) / 2);
Pdu pdu; // declared before every return so NRVO fires (all paths return the same object)
if (!register_block_in_range(LOG_STR("Write"), start_address, values.size(), max_registers)) {
return pdu; return pdu;
} }
append_pdu_header(pdu, FunctionCode::WRITE_MULTIPLE_REGISTERS, start_address, values.size()); append_pdu_header(pdu, FunctionCode::WRITE_MULTIPLE_REGISTERS, start_address, values.size());
@@ -498,6 +501,19 @@ PduBuffer create_write_registers_pdu(uint16_t start_address, std::span<const uin
return pdu; return pdu;
} }
static_assert((PduBuffer::capacity() - WRITE_MULTIPLE_HEADER_SIZE) / 2 == MAX_NUM_OF_REGISTERS_TO_WRITE,
"a full-frame PDU must hold exactly MAX_NUM_OF_REGISTERS_TO_WRITE registers");
static_assert((WriteFewRegistersPdu::capacity() - WRITE_MULTIPLE_HEADER_SIZE) / 2 == MAX_FEW_REGISTERS,
"the small write buffer must hold exactly MAX_FEW_REGISTERS registers");
PduBuffer create_write_registers_pdu(uint16_t start_address, std::span<const uint16_t> values) {
return build_write_registers_pdu<PduBuffer>(start_address, values);
}
WriteFewRegistersPdu create_write_few_registers_pdu(uint16_t start_address, std::span<const uint16_t> values) {
return build_write_registers_pdu<WriteFewRegistersPdu>(start_address, values);
}
PduBuffer create_read_write_multiple_registers_pdu(uint16_t read_start_address, uint16_t read_count, PduBuffer create_read_write_multiple_registers_pdu(uint16_t read_start_address, uint16_t read_count,
uint16_t write_start_address, uint16_t write_start_address,
std::span<const uint16_t> write_values) { std::span<const uint16_t> write_values) {
@@ -473,11 +473,15 @@ inline int64_t payload_to_number(const std::vector<uint8_t> &data, SensorValueTy
*/ */
std::optional<int64_t> registers_to_number(const uint16_t *registers, size_t count, SensorValueType sensor_value_type); std::optional<int64_t> registers_to_number(const uint16_t *registers, size_t count, SensorValueType sensor_value_type);
/// 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;
// Named PDU buffer types: the builders' storage strategy (currently stack-allocated StaticVector, // Named PDU buffer types: the builders' storage strategy (currently stack-allocated StaticVector,
// right-sized per shape) can be swapped in one place without touching every signature. // right-sized per shape) can be swapped in one place without touching every signature.
using PduBuffer = StaticVector<uint8_t, MAX_PDU_SIZE>; using PduBuffer = StaticVector<uint8_t, MAX_PDU_SIZE>;
using ReadPdu = StaticVector<uint8_t, READ_PDU_SIZE>; using ReadPdu = StaticVector<uint8_t, READ_PDU_SIZE>;
using WriteSinglePdu = StaticVector<uint8_t, WRITE_SINGLE_PDU_SIZE>; using WriteSinglePdu = StaticVector<uint8_t, WRITE_SINGLE_PDU_SIZE>;
using WriteFewRegistersPdu = StaticVector<uint8_t, WRITE_MULTIPLE_HEADER_SIZE + 2 * MAX_FEW_REGISTERS>;
/// Scratch space for packing coils into wire layout: one bit per coil, sized for the spec maximum. /// Scratch space for packing coils into wire layout: one bit per coil, sized for the spec maximum.
using CoilPackBuffer = StaticVector<uint8_t, packed_bit_bytes(MAX_NUM_OF_COILS_TO_WRITE)>; using CoilPackBuffer = StaticVector<uint8_t, packed_bit_bytes(MAX_NUM_OF_COILS_TO_WRITE)>;
@@ -521,6 +525,15 @@ PduBuffer create_client_pdu(FunctionCode function_code, uint16_t start_address,
*/ */
PduBuffer create_write_registers_pdu(uint16_t start_address, std::span<const uint16_t> values); PduBuffer create_write_registers_pdu(uint16_t start_address, std::span<const uint16_t> values);
/** Create modbus write multiple registers command (function 0x10) on a right-sized stack buffer.
* Identical wire bytes to create_write_registers_pdu() for any accepted input.
* @param start_address modbus address of the first register to write
* @param values register values to write, at most MAX_FEW_REGISTERS (an over-long or empty set is
* rejected and an empty PDU is returned)
* @return PDU (function code + data, no address, no CRC)
*/
WriteFewRegistersPdu create_write_few_registers_pdu(uint16_t start_address, std::span<const uint16_t> values);
/** Create modbus read/write multiple registers command /** Create modbus read/write multiple registers command
* Function 0x17 Read/Write Multiple Registers * Function 0x17 Read/Write Multiple Registers
* Writes write_values then reads read_count registers in one transaction (write first, per Modbus 6.17); * Writes write_values then reads read_count registers in one transaction (write first, per Modbus 6.17);
+1
View File
@@ -290,6 +290,7 @@ template<typename T, size_t N> class StaticVector {
} }
size_t size() const { return count_; } size_t size() const { return count_; }
static constexpr size_t capacity() { return N; }
bool empty() const { return count_ == 0; } bool empty() const { return count_ == 0; }
// Direct access to underlying data // Direct access to underlying data
@@ -489,6 +489,28 @@ TEST(ModbusTypedBuilders, WriteRegistersPduRejectsOverLimit) {
EXPECT_FALSE(create_write_registers_pdu(0x0000, values).empty()); EXPECT_FALSE(create_write_registers_pdu(0x0000, values).empty());
} }
TEST(ModbusTypedBuilders, WriteFewRegistersPduMatchesFullSizeBuilder) {
static_assert(sizeof(WriteFewRegistersPdu) < sizeof(PduBuffer) / 4,
"WriteFewRegistersPdu must be meaningfully smaller");
const uint16_t values[] = {0x000B, 0x0016, 0xABCD, 0xFF00};
for (size_t count = 1; count <= MAX_FEW_REGISTERS; count++) {
auto small = create_write_few_registers_pdu(0x0102, std::span<const uint16_t>(values, count));
auto full = create_write_registers_pdu(0x0102, std::span<const uint16_t>(values, count));
EXPECT_EQ(std::vector<uint8_t>(small.begin(), small.end()), std::vector<uint8_t>(full.begin(), full.end()))
<< count << " registers";
EXPECT_EQ(small.size(), 6u + 2 * count);
EXPECT_TRUE(is_client_pdu_standard(small.data(), small.size()));
}
}
TEST(ModbusTypedBuilders, WriteFewRegistersPduRejectsInvalidInput) {
const uint16_t values[MAX_FEW_REGISTERS + 1] = {0xAAAA, 0xAAAA, 0xAAAA, 0xAAAA, 0xAAAA};
EXPECT_TRUE(create_write_few_registers_pdu(0x0000, values).empty());
EXPECT_FALSE(create_write_few_registers_pdu(0x0000, std::span<const uint16_t>(values, MAX_FEW_REGISTERS)).empty());
EXPECT_TRUE(create_write_few_registers_pdu(0x0000, std::span<const uint16_t>()).empty());
EXPECT_TRUE(create_write_few_registers_pdu(0xFFFF, std::span<const uint16_t>(values, 2)).empty());
}
TEST(ModbusTypedBuilders, ReadWriteMultipleRegistersPduWireBytes) { TEST(ModbusTypedBuilders, ReadWriteMultipleRegistersPduWireBytes) {
const uint16_t write_values[] = {0x000B, 0x0016}; const uint16_t write_values[] = {0x000B, 0x0016};
// Read 2 registers at 0x0010, write 2 registers at 0x0020. // Read 2 registers at 0x0010, write 2 registers at 0x0020.