Skip to content

feat(Modbus): add WriteMultipleRegistersAsync function#26

Merged
ArgoZhang merged 2 commits into
mainfrom
refactor-register
Sep 4, 2025
Merged

feat(Modbus): add WriteMultipleRegistersAsync function#26
ArgoZhang merged 2 commits into
mainfrom
refactor-register

Conversation

@ArgoZhang

Copy link
Copy Markdown
Member

Link issues

fixes #25

Summary By Copilot

Regression?

  • Yes
  • No

Risk

  • High
  • Medium
  • Low

Verification

  • Manual (required)
  • Automated

Packaging changes reviewed?

  • Yes
  • No
  • N/A

☑️ Self Check before Merge

⚠️ Please check all items below before review. ⚠️

  • Doc is updated/provided or not needed
  • Demo is updated/provided or not needed
  • Merge the latest code from the main branch

Copilot AI review requested due to automatic review settings September 4, 2025 09:42
@ArgoZhang ArgoZhang merged commit 6c57031 into main Sep 4, 2025
1 check passed
@ArgoZhang ArgoZhang deleted the refactor-register branch September 4, 2025 09:42

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +102 to +104
result = values.Length == 1
? data.Span.SequenceEqual(response.Span[8..])
: response.Span[10..11].SequenceEqual(data.Span[2..3]);

Copilot AI Sep 4, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
Comment on lines +161 to +164
for (var i = 0; i < values.Length; i++)
{
data[i * 2 + 5] = (byte)(values[i] >> 8);
data[i * 2 + 6] = (byte)(values[i] & 0xFF);

Copilot AI Sep 4, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
// 连接 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]);

Copilot AI Sep 4, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The octal literal 01 should be written as 1 for consistency with other decimal values in the array, unless octal representation is specifically intended.

Suggested change
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]);

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(Modbus): add WriteMultipleRegistersAsync function

2 participants