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

Added AllowPacketFragmentationSelector option. #2124

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

xljiulang
Copy link
Contributor

When I was developing the WebSocket capability of #2103, I found that the AllowPacketFragmentation of MqttServerTcpEndpointBaseOptions could not be perfectly adapted. We need to dynamically choose whether to use PacketFragmentation based on whether the IMqttChannelAdapter is a WebSocket connection, rather than simply true or false.

I also considered adding the AllowPacketFragmentationSelector option to MQTTnet.AspNetCore, but considering that MQTTnet.Server actually needs this option capability, I think it is most appropriate to add it to MqttServerTcpEndpointBaseOptions.

AllowPacketFragmentationSelector is a nullable property with a higher priority than the AllowPacketFragmentation property. It exposes the IMqttChannelAdapter to the implementer of the selector. We need to add an IsWebSocketConnection property to the IMqttChannelAdapter.

/// <summary>
///     Usually the MQTT packets can be send partially. This is done by using multiple TCP packets
///     or WebSocket frames etc. Unfortunately not all clients do support this feature and
///     will close the connection when receiving such packets. If such clients are connecting to this
///     server the flag must be set to _false_.
/// </summary>
public bool AllowPacketFragmentation { get; set; } = true;

/// <summary>
/// Select whether to AllowPacketFragmentation for an <see cref="IMqttChannelAdapter"/>.
/// Its priority is higher than the <see cref="AllowPacketFragmentation"/>.
/// </summary>
public Func<IMqttChannelAdapter, bool> AllowPacketFragmentationSelector { get; set; }

Currently I have only adapted MAllowPacketFragmentationSelector for MQTTnet.Server, because it doesn't make much sense to adapt it for the current branch of MQTTnet.AspNetCore, and it needs to be adapted after #2103.

@chkr1011
Copy link
Collaborator

chkr1011 commented Jan 2, 2025

I don't understand why we need the selector. From what you wrote the reason is that depending on the channel the result might be true or false. The options are affecting all adapters right now. What makes me wonder is that the user only should enable the fragmentation (or disable) if it makes sense for the adapter which is set by the user later on. The packet fragmentation is also possible for both TCP and WebSockets. To me your comment sounds like that it is only a feature for WebSockets at all.

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.

2 participants