Skip to content

Upgrade mqtt #1311

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

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Upgrade mqtt #1311

wants to merge 3 commits into from

Conversation

helto4real
Copy link
Collaborator

@helto4real helto4real commented Jun 28, 2025

Breaking change

Proposed change

Trying to upgrade the mqtt manager to the latest breaking version of MQTTNet.

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New feature (which adds functionality to an existing integration)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

This PR is far from ready. Needs manual testing and also add some integration tests imo. Like reconnect logic.

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue:
  • Link to documentation pull request:

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • The code compiles without warnings (code quality check)
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration are added/changed:

@helto4real helto4real marked this pull request as draft June 28, 2025 14:04
@helto4real helto4real requested a review from Copilot June 28, 2025 14:04
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR upgrades the MQTT.NET dependency to v5 and refactors the MqttEntityManager extension to use the new raw IMqttClient APIs.

  • Bumped MQTTnet (and related) package versions across projects.
  • Replaced all IManagedMqttClient usages with IMqttClient, updating factories, subscriber/sender logic, and connection handling.
  • Adjusted integration and unit tests to align with the new client API and payload handling.

Reviewed Changes

Copilot reviewed 33 out of 33 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/Extensions/NetDaemon.Extensions.MqttEntityManager/*.csproj Upgraded MQTTnet package to 5.x and removed managed-client refs.
src/Extensions/NetDaemon.Extensions.MqttEntityManager/MqttFactoryFactory.cs Switched factory to produce IMqttClient.
src/Extensions/NetDaemon.Extensions.MqttEntityManager/MqttClientOptionsFactory.cs Updated options builder to use MqttClientOptionsBuilder.
src/Extensions/NetDaemon.Extensions.MqttEntityManager/MessageSubscriber.cs Swapped to SubscribeAsync with new options; streamlined payload parsing.
src/Extensions/NetDaemon.Extensions.MqttEntityManager/MessageSender.cs Replaced EnqueueAsync with PublishAsync on IMqttClient.
src/Extensions/NetDaemon.Extensions.MqttEntityManager/AssuredMqttConnection.cs Reworked connection loop for raw client with retry/cancellation.
src/Client/NetDaemon.HassClient.Tests/**/*Tests.cs Updated tests to use ReadOnlySequence<byte> payload helpers.
Comments suppressed due to low confidence (2)

src/Extensions/NetDaemon.Extensions.MqttEntityManager/MqttFactoryFactory.cs:17

  • The class MqttClientFactory does not exist in MQTTnet v5; you should use new MqttFactory().CreateMqttClient() instead.
        return new MqttClientFactory().CreateMqttClient();

src/Extensions/NetDaemon.Extensions.MqttEntityManager/AssuredMqttConnection.cs:138

  • Extraneous quotes after the closing brace are causing a syntax error; remove the stray ''.
}''


mqttSetup.LastPublishedMessage.Topic.Should().Be("homeassistant/domain/sensor/availability");
payload.Should().Be("up");
}


private static byte[] ConvertPayloadToArray(ReadOnlySequence<byte> payload)
Copy link
Preview

Copilot AI Jun 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The ConvertPayloadToArray helper is duplicated across multiple test classes; consider moving it into a shared test utility to reduce duplication.

Copilot uses AI. Check for mistakes.

helto4real and others added 2 commits June 28, 2025 16:07
…ubscriber.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@helto4real
Copy link
Collaborator Author

@skotl would you mind check this out. As far as I can tell it works when I run it locally. It would be great if you could manually confirm it works. At some point I will add an integration tests for this too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants