Skip to content
This repository was archived by the owner on Nov 13, 2025. It is now read-only.

Conversation

@ArgoZhang
Copy link
Member

Link issues

fixes #59

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 24, 2025 04:19
@ArgoZhang ArgoZhang merged commit 25554b4 into master Sep 24, 2025
1 check failed
@ArgoZhang ArgoZhang deleted the refactor branch September 24, 2025 04:20
Copy link

Copilot AI left a comment

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 bumps the library version to 9.1.0 and refactors connection handling across different Modbus client implementations to improve consistency and remove error handling that was moved elsewhere.

  • Removes synchronous operations from async methods in UDP and RTU clients
  • Removes connection state validation from TCP client send operations
  • Deletes unused extension method for TCP connection validation

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/Longbow.Modbus/Longbow.Modbus.csproj Updates version number from 9.0.10 to 9.1.0
src/Longbow.Modbus/Udp/DefaultUdpClient.cs Makes ConnectAsync synchronous by removing await and using ValueTask.FromResult
src/Longbow.Modbus/Tcp/DefaultTcpClient.cs Removes connection validation call from SendAsync method
src/Longbow.Modbus/Rtu/DefaultRtuClient.cs Simplifies ConnectAsync to delegate directly to client.OpenAsync, adds connection check in CloseAsync
src/Longbow.Modbus/Extensions/ITcpSocketClientExtensions.cs Deletes entire file containing ThrowIfNotConnected extension method

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +14 to +19
public ValueTask<bool> ConnectAsync(IPEndPoint endPoint, CancellationToken token = default)
{
await CloseAsync();
_client = new UdpClient(options.LocalEndPoint);
_client.Connect(endPoint);
return true;

return ValueTask.FromResult(true);
Copy link

Copilot AI Sep 24, 2025

Choose a reason for hiding this comment

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

The removal of await CloseAsync() could cause resource leaks if the method is called multiple times. The previous implementation properly cleaned up existing connections before creating new ones.

Copilot uses AI. Check for mistakes.

return ret;
}
public ValueTask<bool> ConnectAsync(CancellationToken token = default) => client.OpenAsync(token);
Copy link

Copilot AI Sep 24, 2025

Choose a reason for hiding this comment

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

The simplified implementation removes exception handling that was previously setting the Exception property. This could break error handling expectations if other parts of the codebase rely on accessing connection errors through this property.

Suggested change
public ValueTask<bool> ConnectAsync(CancellationToken token = default) => client.OpenAsync(token);
public async ValueTask<bool> ConnectAsync(CancellationToken token = default)
{
try
{
var result = await client.OpenAsync(token);
Exception = null;
return result;
}
catch (Exception ex)
{
Exception = ex;
return false;
}
}

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

chore: bump version 9.1.0

2 participants