Mqttnet 4 #721
Merged
Mqttnet 4 #721
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Bumps [MQTTnet](https://github.com/dotnet/MQTTnet) from 3.1.2 to 4.0.1.184. - [Release notes](https://github.com/dotnet/MQTTnet/releases) - [Commits](dotnet/MQTTnet@v3.1.2...v4.0.1.184) --- updated-dependencies: - dependency-name: MQTTnet dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
1. IMqttFactory removed so substituted an internal version that is only called in our `UseNetDaemonMqttEntityManagement` DI setup method 2. Multiple `MQTTnet.*` namespaces collapsed to fewer 3. PublishAsync() is now EnqueueAsync() 4. EnqueueAsync() does not support a CancellationToken and does not return a success / fail status 5. UseApplicationMessageReceivedHander() has been replaced with event subscriptions `OnMessageReceivedAsync` 6. Managed Mqtt Client connection/disconnection have changed from delegates to events
Codecov Report
@@ Coverage Diff @@
## dev #721 +/- ##
==========================================
+ Coverage 79.44% 79.60% +0.16%
==========================================
Files 137 138 +1
Lines 3503 3516 +13
Branches 356 356
==========================================
+ Hits 2783 2799 +16
+ Misses 576 573 -3
Partials 144 144
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Collaborator
|
Man they really messed this one up. Removng the IMqttFactory was a weird one! I will try to review it but even better try to test it too. |
helto4real
reviewed
Jun 27, 2022
Collaborator
helto4real
left a comment
helto4real
left a comment
There was a problem hiding this comment.
I have no suggestions for changes. Good job with this But I would want someone to test it manually before I merge this. I will try to find time this weekend.
| /// MqttNet removed the IMqttFactory interface at v4 which breaks our DI | ||
| /// So this is a factory for the MqttFactory to satisfy our use case | ||
| /// </summary> | ||
| internal class MqttFactoryFactory : IMqttFactory |
Collaborator
There was a problem hiding this comment.
Sad that they removed the IMqttFactory interface. This is the way we can deal with it and still use DI.
helto4real
approved these changes
Jul 4, 2022
Collaborator
helto4real
left a comment
helto4real
left a comment
There was a problem hiding this comment.
I did not find time to test manually but code looks good to me.
Ikcelaks
pushed a commit
to Ikcelaks/netdaemon
that referenced
this pull request
Dec 23, 2022
* Bump MQTTnet from 3.1.2 to 4.0.1.184 Bumps [MQTTnet](https://github.com/dotnet/MQTTnet) from 3.1.2 to 4.0.1.184. - [Release notes](https://github.com/dotnet/MQTTnet/releases) - [Commits](dotnet/MQTTnet@v3.1.2...v4.0.1.184) --- updated-dependencies: - dependency-name: MQTTnet dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> * Support MqttNet v4: 1. IMqttFactory removed so substituted an internal version that is only called in our `UseNetDaemonMqttEntityManagement` DI setup method 2. Multiple `MQTTnet.*` namespaces collapsed to fewer 3. PublishAsync() is now EnqueueAsync() 4. EnqueueAsync() does not support a CancellationToken and does not return a success / fail status 5. UseApplicationMessageReceivedHander() has been replaced with event subscriptions `OnMessageReceivedAsync` 6. Managed Mqtt Client connection/disconnection have changed from delegates to events Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Tomas Hellström <tomas.hellstrom@yahoo.se>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Support MqttNet v4
Non-breaking change
Proposed change
Based on
dependabot/nuget/MQTTnet-4.0.1.184branchUseNetDaemonMqttEntityManagementDI setup methodMQTTnet.*namespaces collapsed to fewerOnMessageReceivedAsyncThe public interfaces to the
IMQTTEntityManagerare unchanged so there should be no updates to client code required.[ugliness warning] The upstream library retains
MqttFactorybut deletedIMqttFactory. To make things even more challenging they sealedMqttFactory...So I have created our own
IMqttFactoryto retain current workflow, however there is now a new class calledMqttFactoryFactory, which calls into the originalMqttFactoryto create the client. It's an ugly name but is descriptive(!) and is internal to the extension so users shouldn't see it.Open to alternative suggestions though!
Type of change
Additional information
Checklist
addedupdated to verify that the new code works.If user exposed functionality or configuration variables are added/changed: