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.31k stars 1.04k forks source link

Release 2.5 #82

Closed chkr1011 closed 6 years ago

chkr1011 commented 6 years ago

I want to release a new version soon because we have lots of new features. Before doing this I want to summarize the open topics:

  1. Units Tests are not running for server tests. I am not able to fix them because I don't understand how to inject the test adapter since we have DI. @JanEggers Can you please have a look?
  2. @haeberle can you confirm that the bug with UWP is now fixed since latest changes from @JanEggers? If not I can also keep searching for a solution.
  3. @JTrotta does the ManagedClient have everything you need so far? We can add more features but I want to release it soon with the features it already has.
haeberle commented 6 years ago

Not still the same issue in UWP, cureently I'm testing from version 2.1.3. to 2.3.1, so fare all fine, the issues must be in a newer version. I will test next versions asap.

JTrotta commented 6 years ago

Let me do last test this morning. (1000 utc). But it should be almost ready and working.

JanEggers commented 6 years ago

@chkr1011 i created a pr that fixes the unittests, addresses your todos according configuring the server and added nuspec for the new package

JTrotta commented 6 years ago

@chkr1011 Got some problem. Cannot instantiate ManagedClient without IOC. Now you added Microsoft IOC logging extensions. But I'm already using TinyIoc, and I do not like to add more and more libraries. Also it seems no more .NET451 compatible, isn't? If not, it cannot be used on MONO anymore. Is it possible to avoid this construction mode?

chkr1011 commented 6 years ago

@JTrotta Sorry but the introduction of the MS logging and also DI was already some time on the list. Already since two versions. In general this library is only supported by 4.5.2 and above because older .net frameworks are no longer supported by MS. Mono is surely a different story. Maybe @JanEggers can shed some light here how to restore support for Mono etc. But you can already stay at your logging library and just add it to the logging infrastructure like the still existing static event. You can also still use the static trace event and push the messages into your logging component.

JTrotta commented 6 years ago

Ok, gonna try 4.5.2 on MONO. Please can you wait till 1600 UTC? Otherwise I have to fork it ti 451. Thank you

chkr1011 commented 6 years ago

Don't worry. I believe we will find a solution.

JanEggers commented 6 years ago

regarding ioc: have a look at Frameworks/MQTTnet.NetStandard/ServiceCollectionExtensions.cs just register the classes in a similar fashon in your di solution or use MQTTFactory created with default constructor to create the IManagedMqttClient https://github.com/chkr1011/MQTTnet/blob/f85721fbf50c1de320a277041088a2513d8106de/Frameworks/MQTTnet.NetStandard/MqttFactory.cs#L103

regarding mono: i never used mono so far. but according to https://docs.microsoft.com/en-us/dotnet/standard/net-standard you should be able to use dotnetstandard version for mono or am i getting things wrong here?

JTrotta commented 6 years ago

Yes, I see. The problem is that MQTTnetStandard requires some libraries not compatible with .NET451. I tried to install .452, recompile and test it on MONO, with no success. microsoft.extensions.dependencyinjection is not compatible with .452

JTrotta commented 6 years ago

Dears, .NET 4.6.2 should work with new libraries microsoft.extensions.XXXXX. But it takes long time to test 4.6.2 with MONO, even though it should work, I need to add and test many new libraries. So I really prefer the previous version without IOC and microsoft.extensions.XXXX, even because I do not think it's a good to add an IOC inside a library, it should be the final user to choose the right one (i.e. I use Autofac or TinyIOC depending on the complexity of the project). Anyway, I will try next days to get it working with MONO. @chkr1011 If you want publish, go ahead. Thank you JJ

JanEggers commented 6 years ago

im confused if you need net451 you can use it!? also M.E.DI is not the limiting factor for netstandard we could use 1.1 with it but 1.1 is missing security features required for tls.

chkr1011 commented 6 years ago

@JTrotta Is it only DI which is causing the issues or also the logging component which is also not compatible?

chkr1011 commented 6 years ago

@JTrotta I also don't agree fully with the doubts of DI. You are right that you choose the DI for your project on your own but DI also makes sense for this project. I also allows you to inject your own handling etc. It gives you more accessibility. This is also a good thing for a library to make it not sealed with a fence and you must use it the developer wants to. You can also combine DI containers and put the high level client or server into your container of choice. The user must not use the same DI container as we do. He may don't know it because this library is just added to the project and that's it. The DI is just an internal thing which we exposed instead of hiding it. That's be opinion about this.

But I am still wondering why it leads to problems because the target frameworks didn't change at all. Where is this incompatibility coming from? This library should still support 4.5.2 at least. This is what I am expecting and which is the goal of this library. Do you have some output or errors or something else so I can understand it?

JTrotta commented 6 years ago

I can be more precise tomorrow, but for now the error is related to creation of the ManagedMqttClient. It needs some libraries I do not use. Adding the libraries I get a new error like "this library is not compatible with 4.5.1". This happens on VS12 and VS17. Try to create a new project in 4.5 1, add the two Mqtt libraries, and try to nuget the others. You should get the same errors. Using 4.5.2 on VS17, raise the same errors. Than I tried 4.6.2, and it works on Windows, but not on MONO, that's because I need to add many dll, and test them. That's all. But if you think that DI is a good choice, please go ahead, do not worry about old frameworks. Tomorrow afternoon I can send you much more details Thank you guys.

JanEggers commented 6 years ago

sry i still dont get it. the testapp is net451 and works fine.

chkr1011 commented 6 years ago

@JanEggers I just added a server to the UWP test app and again I had to fix several UWP relevant issues by copy and pasting code from netstandard projects. So my conclusion is that this is not a good approach because this was not the first time and I cannot check those files like a trigger all the time :smile:

Do you have an idea how to move them together? Maybe by reorganizing some factories or adding some interfaces for DI and then move the factory to the core library? Or even add UWP to the "multi platform" library? Is this even possible? I was not able to add UWP as a target.

JanEggers commented 6 years ago

this was not the first time and I cannot check those files like a trigger all the time.

ill do another attempt on getting ci and unittest working so i dont accidently break stuff.

Or even add UWP to the "multi platform" library?

i would prefere that.

JanEggers commented 6 years ago

i got travis building something. but it does not support uwp builds. see https://github.com/YoshinoriN/Kinugasa/issues/90

also travis is not able to run mstest.

id like to try jenkins as to their docs they support uwp builds and mstest

thoughts?

i also was able to merge the uwp and netstandard projects :).

does it make sense for you to merge mqttnet and mqttnet.core and remove .core from name? We could deliver all platforms from one project?

JanEggers commented 6 years ago

narf there was no cloud option of jenkins so i now settled with appveyor. prs exist let me know if you are ok with the changes. hopefully we finally defeated the copy and paste madness 👍

JTrotta commented 6 years ago

FYI, look at two screenshoot. The first is telling me that I need a the library: Microsoft.Extensions.DependencyInjection image Trying installing the library I got the second error. image

Moreover. Installing the right library on VS17( 1.1.10), I had to install some other libraries, some of witch are not MONO compatible. I'm doing some more test. Will let you know.

JanEggers commented 6 years ago

@JTrotta im guessing this is just an intermidiate problem becuase you reference the project directly. can you please try to build mqttnet with build/build.ps1 and add a reference to that package. then all required dependencies should be added to your project. alternativly you can try recreate your project with sdk based csproj (new dotnetcore app then replace targetframework with net451) then the project reference should work as intended.

according to https://docs.microsoft.com/en-us/dotnet/standard/net-standard

the ms.extensions.di package (1.1.1) should be campatible with mono 4.6

JTrotta commented 6 years ago

@JanEggers OK! will try tomorrow morning. Will let you know. Thank you

chkr1011 commented 6 years ago

@JanEggers Good job! This looks much better to me. Hopefully this will decrease issues with different platforms a lot.

@JTrotta I think it is strange that v. 2.0.0 of DI is being installed. This lib uses only v1.1. Very strange. Lets wait for your test results.

JanEggers commented 6 years ago

do you need help with appveyor? there should be a option to add batch for readme.md but i didnt set that up because it should be on your account. if its setup id like to create a broken pr to verify appveyor blocks merging it.

chkr1011 commented 6 years ago

@JanEggers I added this project to appveyor but I get the following output:

Build started
git clone -q --branch=master https://github.com/chkr1011/MQTTnet.git C:\projects\mqttnet
git checkout -qf 49b1706495a523f7a5f014ac8ebe77e80f5244c0
msbuild "C:\projects\mqttnet\MQTTnet.sln" /verbosity:minimal /logger:"C:\Program Files\AppVeyor\BuildAgent\Appveyor.MSBuildLogger.dll"
Microsoft (R) Build Engine version 14.0.25420.1
Copyright (C) Microsoft Corporation. All rights reserved.
C:\projects\mqttnet\Tests\MQTTnet.Core.Tests\MQTTnet.Core.Tests.csproj.metaproj : warning MSB4078: The project file "Tests\MQTTnet.Core.Tests\MQTTnet.Core.Tests.csproj" is not supported by MSBuild and cannot be built.
C:\projects\mqttnet\MQTTnet.Core\MQTTnet.Core.csproj.metaproj : warning MSB4078: The project file "MQTTnet.Core\MQTTnet.Core.csproj" is not supported by MSBuild and cannot be built.
C:\projects\mqttnet\Frameworks\MQTTnet.Netstandard\MQTTnet.NetStandard.csproj.metaproj : warning MSB4078: The project file "Frameworks\MQTTnet.Netstandard\MQTTnet.NetStandard.csproj" is not supported by MSBuild and cannot be built.
C:\projects\mqttnet\Tests\MQTTnet.TestApp.NetCore\MQTTnet.TestApp.NetCore.csproj.metaproj : warning MSB4078: The project file "Tests\MQTTnet.TestApp.NetCore\MQTTnet.TestApp.NetCore.csproj" is not supported by MSBuild and cannot be built.
C:\projects\mqttnet\MQTTnet.Core\MQTTnet.Core.csproj(1,1): error MSB4041: The default XML namespace of the project must be the MSBuild XML namespace. If the project is authored in the MSBuild 2003 format, please add xmlns="http://schemas.microsoft.com/developer/msbuild/2003" to the <Project> element. If the project has been authored in the old 1.0 or 1.2 format, please convert it to MSBuild 2003 format.
C:\projects\mqttnet\MQTTnet.Core\MQTTnet.Core.csproj(1,1): error MSB4041: The default XML namespace of the project must be the MSBuild XML namespace. If the project is authored in the MSBuild 2003 format, please add xmlns="http://schemas.microsoft.com/developer/msbuild/2003" to the <Project> element. If the project has been authored in the old 1.0 or 1.2 format, please convert it to MSBuild 2003 format.
C:\projects\mqttnet\MQTTnet.Core\MQTTnet.Core.csproj(1,1): error MSB4041: The default XML namespace of the project must be the MSBuild XML namespace. If the project is authored in the MSBuild 2003 format, please add xmlns="http://schemas.microsoft.com/developer/msbuild/2003" to the <Project> element. If the project has been authored in the old 1.0 or 1.2 format, please convert it to MSBuild 2003 format.
C:\Program Files (x86)\MSBuild\Microsoft\WindowsXaml\v14.0\8.21\Microsoft.Windows.UI.Xaml.Common.targets(29,3): error MSB4019: The imported project "C:\Program Files (x86)\Windows Kits\10\bin\10.0.15063.0\XamlCompiler\Microsoft.Windows.UI.Xaml.Common.targets" was not found. Confirm that the path in the <Import> declaration is correct, and that the file exists on disk. [C:\projects\mqttnet\Tests\MQTTnet.TestApp.UniversalWindows\MQTTnet.TestApp.UniversalWindows.csproj]
Command exited with code 1

I will have a look for a batch.

chkr1011 commented 6 years ago

Sorry it was the wrong branch but develop look like this:

Build started
git clone -q --branch=develop https://github.com/chkr1011/MQTTnet.git C:\projects\mqttnet
git checkout -qf 5427b488c3eba77300fbf01b8422023e649cf711
dotnet restore
C:\projects\mqttnet\Tests\MQTTnet.TestApp.UniversalWindows\MQTTnet.TestApp.UniversalWindows.csproj(175,3): error MSB4019: The imported project "C:\Program Files\dotnet\sdk\2.0.2\Microsoft\WindowsXaml\v14.0\Microsoft.Windows.UI.Xaml.CSharp.targets" was not found. Confirm that the path in the <Import> declaration is correct, and that the file exists on disk.
C:\projects\mqttnet\Tests\MQTTnet.TestApp.UniversalWindows\MQTTnet.TestApp.UniversalWindows.csproj : warning NU1503: Skipping restore for project 'C:\projects\mqttnet\Tests\MQTTnet.TestApp.UniversalWindows\MQTTnet.TestApp.UniversalWindows.csproj'. The project file may be invalid or missing targets required for restore. [C:\projects\mqttnet\MQTTnet.sln]
C:\Program Files\dotnet\sdk\2.0.2\Sdks\Microsoft.NET.Sdk\Sdk\Sdk.targets(41,3): error MSB4019: The imported project "C:\Program Files\dotnet\sdk\2.0.2\Microsoft\WindowsXaml\v\Microsoft.Windows.UI.Xaml.CSharp.targets" was not found. Confirm that the path in the <Import> declaration is correct, and that the file exists on disk. [C:\projects\mqttnet\Frameworks\MQTTnet.Netstandard\MQTTnet.NetStandard.csproj]
C:\Program Files\dotnet\sdk\2.0.2\Sdks\Microsoft.NET.Sdk\Sdk\Sdk.targets(41,3): error MSB4019: The imported project "C:\Program Files\dotnet\sdk\2.0.2\Microsoft\WindowsXaml\v\Microsoft.Windows.UI.Xaml.CSharp.targets" was not found. Confirm that the path in the <Import> declaration is correct, and that the file exists on disk. [C:\projects\mqttnet\Frameworks\MQTTnet.Netstandard\MQTTnet.NetStandard.csproj]
C:\Program Files\dotnet\sdk\2.0.2\Sdks\Microsoft.NET.Sdk\Sdk\Sdk.targets(41,3): error MSB4019: The imported project "C:\Program Files\dotnet\sdk\2.0.2\Microsoft\WindowsXaml\v\Microsoft.Windows.UI.Xaml.CSharp.targets" was not found. Confirm that the path in the <Import> declaration is correct, and that the file exists on disk. [C:\projects\mqttnet\Frameworks\MQTTnet.Netstandard\MQTTnet.NetStandard.csproj]
C:\Program Files\dotnet\sdk\2.0.2\Sdks\Microsoft.NET.Sdk\Sdk\Sdk.targets(41,3): error MSB4019: The imported project "C:\Program Files\dotnet\sdk\2.0.2\Microsoft\WindowsXaml\v\Microsoft.Windows.UI.Xaml.CSharp.targets" was not found. Confirm that the path in the <Import> declaration is correct, and that the file exists on disk. [C:\projects\mqttnet\Frameworks\MQTTnet.Netstandard\MQTTnet.NetStandard.csproj]
C:\Program Files\dotnet\sdk\2.0.2\Sdks\Microsoft.NET.Sdk\Sdk\Sdk.targets(41,3): error MSB4019: The imported project "C:\Program Files\dotnet\sdk\2.0.2\Microsoft\WindowsXaml\v\Microsoft.Windows.UI.Xaml.CSharp.targets" was not found. Confirm that the path in the <Import> declaration is correct, and that the file exists on disk. [C:\projects\mqttnet\Frameworks\MQTTnet.Netstandard\MQTTnet.NetStandard.csproj]
Command exited with code 1
JanEggers commented 6 years ago

i had to use msbuild in build.ps1 because of this. dotnet.exe does not support uwp build.

i updated appveyor.yml on my build it now fails because pfx of the uwp app is missing. i dunno how to fix that though.

chkr1011 commented 6 years ago

The PFX is just a self signed certificate without a password. I assume you can generate a new one. But it is probably ignored in gitignore and thus not committed.

JTrotta commented 6 years ago

@chkr1011 @JanEggers , dears, I succeed in running the test application MQTTnet.TestApp.NetCore .exe on MONO 5.2.0.224 Today I will do the integration with my app. Will let you know. JJ

JanEggers commented 6 years ago

@chkr1011 its a long road.... i created a cert and now build runs but.... tests are not discovered.

dotnet test does not work with uwp vstest.console does not find any tests dotnet vstest finally worked but the appveyor testlogger is not recorgniced

pr incoming

@JTrotta nice

chkr1011 commented 6 years ago

@JanEggers I would like to stay with 2 DLLs. One for the protocol related stuff and the platform related one. Even if all platforms are in one project now I like the separation of everything+platform_specific.

JTrotta commented 6 years ago

@chkr1011 @JanEggers
I did many test on MONO 5.2.0.224 and also 4.x.x.x (do not remember). Everything works ad expected. Even though I had to add 8 libraries!!! image To me you can upgrade.

chkr1011 commented 6 years ago

@JTrotta Thank you for your feedback. As @JanEggers already explained in #80, adding much libraries is the way how it works with .net core 2.0. I also don't see any issue in adding lots of libraries instead of a fat single one.

So the last open point is the stable UWP broker issues. @haeberle & @Zuendelmeister: Do you have some news about that?

chkr1011 commented 6 years ago

@JanEggers I just tried to test the nuget package but I got several errors from the build script. I tried to solve them but I was not able to find the root cause. To me the error seems to be related to the UAP framework in the cross platform library. Also there is no "Release" folder after building the AspNetCore project. I tested with different "Configruation" parameters but no luck. The "Release" folder is not created.

There is a screenshot of the error.

image

Also I am not able to add the nuget to LINQPad. It was added but then I cannot run the code because it did not find the DI libraries. But they are shown in the project references and I assume that they will be downloaded automatically. But it doesn't seem so!?

Could you please check it? I ran out of ideas 😄

chkr1011 commented 6 years ago

I just realized that the dependency to DI in not part of the nuspec. This will explain that error at least. I will add all of those which are new since 2.5.

haeberle commented 6 years ago

Hi Christian I will follow up this week on that.

JanEggers commented 6 years ago

narf dotnet build does not work with uwp involved even if i specified --no-dependency when building the aspnet project :(. anyway should work with that pr...

chkr1011 commented 6 years ago

@haeberle Thank you for your feedback. I will also test again with some ESP-12F devices and a UWP server.

haeberle commented 6 years ago

@chkr1011 : I did some tests with a MQTTnet based UWP client, a ESPEasy client and the test UWP Server from the developer branch. Unfort. I'm not sure if the UWP client is sometime not responding or another issue is the root cause, but as you see still an exception in the trace.

image

chkr1011 commented 6 years ago

Thanks for sharing. Do you also have problems in communication? I checked the code and the second exception (ObjectDisposedException) is a result of an issue in cleaning up the session. The first exception is a regular timeout. Does the client disconnected itself? I mean is this OK or is the timeout wrong?

haeberle commented 6 years ago

No, the client doesn't trigger a disconnection, the Client.Disconnected handler is executed.

chkr1011 commented 6 years ago

OK but it is guaranteed that the connection is stable? Sometimes my ESP devices also disconnect if the Wifi is not in range or has bad signal quality.

I just pushed a change which will fix the second exception. I am also keep testing with my ESP devices (which seems to work so far).

haeberle commented 6 years ago

good point ;-) I will investigate, the client (Raspi) is connected with wire, the UWP Server with Wifi and running on my Surface...

chkr1011 commented 6 years ago

The only thing I noted is that my application messages are not processed that often. The old version processes them all. The new one suddenly stops for a while (but keep alive packets are still sent) and then it continues showing several publish packets in the trace. The ESP is sending every 5 seconds but sometimes nothing happens for about 30 seconds. The old version had no problems and shows an application message every 5 seconds.

@JanEggers I found the following code which is related to Raw stream and buffered stream (in ReceivePacketAsync.

if (timeout > TimeSpan.Zero)
{
       receivedMqttPacket = await ReceiveAsync(_channel.RawReceiveStream, cancellationToken).TimeoutAfter(timeout).ConfigureAwait(false);
}
else
{
       receivedMqttPacket = await ReceiveAsync(_channel.ReceiveStream, cancellationToken).ConfigureAwait(false);
}

I don't know if this is correct or not. Does it make sense to distinguish by the timeout value which stream must be used? As far as I understand your comments in the channel class only the connect packet causes problems when using the buffered stream. Is it maybe better to add an enum/overload etc. to ensure that the RAW stream is only used for the connect packet? Do you also have some details why the buffering causes issues?

chkr1011 commented 6 years ago

@haeberle This time I really fixed the ObjectDisposedException 😄 Also my ESP is sending data without problems. I had an issue in the firmware code 😄

JanEggers commented 6 years ago

regarding timeouts: from my findings connect is the only place where receive is called with a timeout, so it should work. but an enumwould make it more obvious....

regarding bufffering: we could add an option so buffering is opt in/out that way we could test if that changes behavior.

Zuendelmeister commented 6 years ago

Hi @chkr1011, sorry for answering this late. I don't have an issue with UWP though but with net core. Best regards.

chkr1011 commented 6 years ago

@JanEggers OK. Do you want to add it? I am trying to fix the connection problems.

@Zuendelmeister Ah OK. But a net core server?

Zuendelmeister commented 6 years ago

@chkr1011 Server and Client. If both are on the same host it works. Between an arm linux and my Laptop it doesn't.

chkr1011 commented 6 years ago

@Zuendelmeister Just a next try: Can you please remove the BufferedStreams from the Channel class and test it again? So using the RAW stream in both cases of:

            SendStream = new BufferedStream(RawReceiveStream, BufferSize);
            ReceiveStream = new BufferedStream(RawReceiveStream, BufferSize);