search

S7NetPlus / s7netplus

S7.NET+ -- A .NET library to connect to Siemens Step7 devices
MIT License
1.35k stars 596 forks source link

OpenAsync with cancellation? #422

Closed gfoidl closed 2 years ago

gfoidl commented 2 years ago

Starting with .NET 5 TcpClient got an overload that takes CancellationToken.

Should there be added a .NET 5 target to the project, so that the cancellation token can be flowed to https://github.com/S7NetPlus/s7netplus/blob/b475aee2e768f709e6b8f01084b176e53d43cd8c/S7.Net/PlcAsynchronous.cs#L28 ?

The issue https://github.com/S7NetPlus/s7netplus/issues/346 -- besided being closed -- could be fixed that way too, as a CancellationTokenSource can have a timeout.

I could submit a PR for this.

scamille commented 2 years ago

Sure that might be helpful. While having code duplication and specialization based on the net version targeted isn't very nice and is generally avoided, I think in this case it is worth the trouble.

Just make sure to stick to the minimum required changes, without polluting the PR with unrelated things :-)

gfoidl commented 2 years ago

Just make sure to stick to the minimum required changes, without polluting the PR with unrelated things :-)

I'm not sure what you are talking about 🤣 Of course I'll do here, otherwise this could be a mega-PR which isn't manageable...

dannyhaak commented 2 years ago

Any reason why this is still not merged? We would love to have this in production.

mycroes commented 2 years ago

Any reason why this is still not merged? We would love to have this in production.

I guess just lack of time. Updating the PR as I'm writing this, if it builds I'll merge it.

dannyhaak commented 2 years ago

Understandable and much appreciated. Thanks!

mycroes commented 2 years ago

Should be available as 0.15.0 anytime now (NuGet is validating).