feat(Modbus): add WriteMultipleRegistersAsync function#26
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements the WriteMultipleRegistersAsync function for Modbus TCP client by adding support for multiple register writes. The implementation distinguishes between single and multiple register writes, using different packet formats according to the Modbus protocol.
- Added logic to handle multiple register writes with proper packet formatting
- Updated response validation to accommodate both single and multiple register operations
- Enhanced test cases with more comprehensive data arrays
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/Longbow.Modbus/DefaultModbusTcpClient.cs | Implements multiple register write logic with conditional packet formatting and response validation |
| test/UnitTestModbus/TcpClientTest.cs | Updates test data arrays to include more values for testing multiple register scenarios |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| result = values.Length == 1 | ||
| ? data.Span.SequenceEqual(response.Span[8..]) | ||
| : response.Span[10..11].SequenceEqual(data.Span[2..3]); |
There was a problem hiding this comment.
The response validation logic is incorrect. For multiple register writes, response.Span[10..11] should be response.Span[10..12] to properly compare the 2-byte quantity field, and data.Span[2..3] should be data.Span[2..4] to match the corresponding 2-byte quantity in the request data.
| for (var i = 0; i < values.Length; i++) | ||
| { | ||
| data[i * 2 + 5] = (byte)(values[i] >> 8); | ||
| data[i * 2 + 6] = (byte)(values[i] & 0xFF); |
There was a problem hiding this comment.
Array index calculation is incorrect. The second line should use data[i * 2 + 6] which will cause an IndexOutOfRangeException. It should be data[i * 2 + 5] for the high byte and data[i * 2 + 6] for the low byte, but the array size calculation needs to account for this properly.
| // 连接 Master | ||
| await client.ConnectAsync("127.0.0.1", 502); | ||
| var response = await client.WriteMultipleRegistersAsync(0x01, 0, [12, 0, 23, 0, 45]); | ||
| var response = await client.WriteMultipleRegistersAsync(0x01, 0, [12, 0, 23, 0, 46, 0, 01, 02, 04, 05]); |
There was a problem hiding this comment.
The octal literal 01 should be written as 1 for consistency with other decimal values in the array, unless octal representation is specifically intended.
| var response = await client.WriteMultipleRegistersAsync(0x01, 0, [12, 0, 23, 0, 46, 0, 01, 02, 04, 05]); | |
| var response = await client.WriteMultipleRegistersAsync(0x01, 0, [12, 0, 23, 0, 46, 0, 1, 02, 04, 05]); |
Link issues
fixes #25
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge