Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve compatibility with .NET Core and later (including .NET 5 and above) #141

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

robinputzar
Copy link

.NET Core and later (includes .NET Platform 5 and above) no longer supports delegate BeginInvoke and EndInvoke. See e.g. this blog post from Mike Rousos for details. I changed the corresponding code in HidDevice.cs and HidFastReadDevice.cs to use Task instead.

Note I only tested this with .NET 6.0.

The changes should work on .NET Core and .NET Platform 5 and above. They should also work on .NET framework 4.5 and above, since the corresponding features (Task.Run(), Task.ContinueWith(), Task.Factory and TaskFactory.StartNew()) are available there also according to the docs.

I also modified some code provided earlier in two regards:

  1. call corresponding functions directly in the lambda expression supplied to TaskFactory.StartNew() instead of using delegate.Invoke() (needs less code and is easier to read),
  2. changed the preprocessor symbols used in the #if switches from NET5_0_OR_GREATER to NETCOREAPP1_0_OR_GREATER, since that should be the appropriate symbol according to the docs.

Note: On .NET Platform 6 at least, the dependency on the Theraot.Core nuget can be removed, because the corresponding backport is no longer needed there.

Any review is appreciated. Hope this helps others using this library.

@robinputzar
Copy link
Author

I just noticed that @Mutex666 also added a preprocessor symbol in src/HidLibrary/HidDeviceEventMonitor.cs in his pull request, and I failed to change this one from NET5_0_OR_GREATER to NETCOREAPP1_0_OR_GREATER in the first place. I just updated this in my fork, and it seems github just included these changes.

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.

1 participant