dotnet / MQTTnet

MQTTnet is a high performance .NET library for MQTT based communication. It provides a MQTT client and a MQTT server (broker). The implementation is based on the documentation from http://mqtt.org/.
MIT License
4.41k stars 1.06k forks source link

Removal of UWP support #708

Closed chkr1011 closed 4 years ago

chkr1011 commented 5 years ago

Hi, to make things easier, I want to remove the direct support of the UWP platform. That does not mean that I want to remove support in general. I just want to remove dedicated builds and UWP-only code which requires compilation constants etc.

To me it looks like that every Windows 10 Version (including IoT Core) supports .netstandard 1.3 at least. So removing the dedicated code might not have any impact?

It would be amazing if some UWP specialists would share some thoughts with me.

Links:

JanEggers commented 5 years ago

+1 for me its always a pain to build because we include the uwp SDK's

SeppPenner commented 5 years ago

I can only agree on that as well. Nevertheless, I'm not an UWP expert at all 😄

JanEggers commented 5 years ago

why did you have to create the uwp specific adapters / channels in the first place?

I guess the regular ones dont work on that platform?

chkr1011 commented 5 years ago

I must say I don't now/remember 😄

But one thing I remember is that .net standard 2.0 was not supported in .net core. And because I have some experience in UWP I just started that special build. That's it.

I will remove the UWP stuff in 3.1.x and will see if anyone will complain 😄

chkr1011 commented 5 years ago

I saw that there are also some constants for NET 452, 461 etc.

I will try to reduce them to:

  1. netstandard1.3
  2. netstandard2.0
  3. net452

only

JanEggers commented 5 years ago

If it works with netstanard 1.3 remove 2.0 also

chkr1011 commented 5 years ago

There are already useful changes in 2.0 which I want to keep. Nothing big but at least worth having 2.0.

pashmak73 commented 5 years ago

Hi, I want to use this project in a UWP app, removing uwp will force me to choose a higher minimum target platform, which is impossible in my case [this will force uwp apps to choose 16299 SDK platform as minimum and this SDK is not supported in mobile devices). so Please consider this.

Also I'm new in MQTT, Can you please help me in private? With a subscription plan or something else?

If yes, please leave your email or skype or other messenger here.

Thanks.

chkr1011 commented 5 years ago

@pashmak73 You can reach me in gitter for example or from the nuget repo.

But isn't .netstandard 1.3 also supprted in UWP? I assume at lease this should work!? Or am I wrong?

I am also thinking about removing netfx 4.5.2 build and adding netstandard1.1 instead. Then all possible platforms are covered with netstandard builds only. But I have to check first if all required APIs we need are already available in netstandard1.1.

pashmak73 commented 5 years ago

I didn't read .netstandard 1.3 correctly, yes it should working fine in uwp.

Also sent you an message in gitter.

chkr1011 commented 5 years ago

I tried to remove UWP but now I remember why I initially added it. The problem is that System.Net.Security (which contains SslStream etc.) is not available in UWP. No matter if .netstandard2.0 or whatever. The required assembly is simply not supported. So it is missing and the code will fail while loading the assembly. Even if the SSL stream is not used. By just having the class in code is enough to fail.

Does anyone have an idea how to get around this?

SeppPenner commented 5 years ago

Well, you can exclude the System.Net.Security reference on project level and create a new project file that references the same .cs files as the regular (Non-UWP) project (Check e.g. https://docs.microsoft.com/en-US/visualstudio/msbuild/how-to-exclude-files-from-the-build?view=vs-2019). The problem is that this won't help if the System.Net.Security library is used in any classes...

I don't know whether it's possible to do this using pre-processor directives like #if UWP following https://stackoverflow.com/questions/18441299/if-debug-if-myownconfig/18441421#18441421.

Something like:

#if UWP
#else 
using System.Net.Security.Whatever
#endif 

Well, this is still ugly as hell and you would need to build everything twice (for everything else and for UWP extra).

pashmak73 commented 5 years ago

@SeppPenner as far as I know, It's WINDOWS_UWP

if !WINDOWS_UWP

using....

endif

@chkr1011 I think you should put this class in the above code. This way you wouldn't have problem with the uwp compilation.

SeppPenner commented 5 years ago

@SeppPenner as far as I know, It's WINDOWS_UWP

I didn't know that there is a pre-processor directive already :)

This way you wouldn't have problem with the uwp compilation.

If it's not already a problem to reference System.Net.Security. If it is a problem, then the following applies:

Well, you can exclude the System.Net.Security reference on project level and create a new project file that references the same .cs files as the regular (Non-UWP) project (Check e.g. https://docs.microsoft.com/en-US/visualstudio/msbuild/how-to-exclude-files-from-the-build?view=vs-2019).

queequac commented 5 years ago

AFAIK, namespace System.Net.Security is available in UWP, but prior to Windows 10 build 16299 there won't be any SslStream inside. (No need to explicitly reference System.Net.Security via nuget!)

So I don't get the point. I can write UWP code that makes use of SslStream (if min. target platform >= 16299). And usually guarding usage of SslStream via ApiInformation.IsTypePresent should be good enough for older platforms, maybe throwing an NotImplementedException in this case.

Can someone enlight me what exactly the problem is?

chkr1011 commented 5 years ago

This thing is that this will require 16299 for UWP which is quite young. I still want to support older UWP platforms. So to me there is no chance yet to remove it. The only thing I can to is to use .netstandard when the OS version is 16299 or higher and keep UWP classes only for older systems. But then there is still code which must be maintained.

So in the end I assume it must stay as it is at the moment.