-
-
Notifications
You must be signed in to change notification settings - Fork 0
chore: bump version 9.1.0 #60
Conversation
There was a problem hiding this 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.
| 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); |
Copilot
AI
Sep 24, 2025
There was a problem hiding this comment.
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.
|
|
||
| return ret; | ||
| } | ||
| public ValueTask<bool> ConnectAsync(CancellationToken token = default) => client.OpenAsync(token); |
Copilot
AI
Sep 24, 2025
There was a problem hiding this comment.
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.
| 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; | |
| } | |
| } |
Link issues
fixes #59
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge