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

When unsubscribing, client disconnects with an "OnDisconnected callback is unsolicited or none" message #451

Closed
CordMemescape opened this issue Mar 11, 2023 · 6 comments

Comments

@CordMemescape
Copy link

After doing a bit of research, I found several issues that mention this problem suggesting that a call to .setProtocolV311() resolves the disconnect. The good news is that the suggested fix works. The bad news is that it appears inconsistent with creating an MqttConnectMessage() to override certain behaviour and setting the protocol/protocol version in the connectMessage.

This means that this code doesn't work as expected,

void initialize({
    required String server,
    required int port,
    required String clientId,
    bool mqttClientLogging = false,
  }) {
    client = MqttServerClient.withPort(server, clientId, port);
    client!.logging(on: mqttClientLogging);
    client!.keepAlivePeriod = 30;
    client!.connectTimeoutPeriod = 3000;

    var connectionMessage = MqttConnectMessage()
        .withProtocolName(MqttClientConstants.mqttV311ProtocolName)
        .withProtocolVersion(MqttClientConstants.mqttV311ProtocolVersion);

    connectionMessage.variableHeader!.connectFlags.cleanStart = false;
    client!.connectionMessage = connectionMessage;
  }

Whereas, this code does:

void initialize({
    required String server,
    required int port,
    required String clientId,
    bool mqttClientLogging = false,
  }) {
    client = MqttServerClient.withPort(server, clientId, port);
    client!.logging(on: mqttClientLogging);
    client!.setProtocolV311();
    client!.keepAlivePeriod = 30;
    client!.connectTimeoutPeriod = 3000;

    var connectionMessage = MqttConnectMessage();
    connectionMessage.variableHeader!.connectFlags.cleanStart = false;
    client!.connectionMessage = connectionMessage;
  }

Further digging indicates that the former example calls this code:

  /// Sets the name of the protocol to use.
  MqttConnectMessage withProtocolName(String protocolName) {
    variableHeader!.protocolName = protocolName;
    return this;
  }

  /// Sets the protocol version. (Defaults to v3, the only protcol
  /// version supported)
  MqttConnectMessage withProtocolVersion(int protocolVersion) {
    variableHeader!.protocolVersion = protocolVersion;
    return this;
  }

While the latter calls this:

  /// Set the protocol version to V3.1.1
  void setProtocolV311() {
    Protocol.version = MqttClientConstants.mqttV311ProtocolVersion;
    Protocol.name = MqttClientConstants.mqttV311ProtocolName;
  }

Is there a reason for the difference? From the documentation I really would have expected them to do the same thing.

@shamblett
Copy link
Owner

Yes you are correct, both methods should work, the problem is in the unsubscribe message -

void writeTo(MqttByteBuffer messageStream) {
    // If the protocol is V3.1.1 the following header fields
    // must be set as below as in this protocol they are reserved.
    if (Protocol.version == MqttClientConstants.mqttV311ProtocolVersion) {

We adjust the unsubscribe message header if the protocol is V311, however this checks Protocol, it should check the variable header -

 if (variableHeader!.protocolVersion ==
        MqttClientConstants.mqttV311ProtocolVersion) {

Also this comment is incorrect -

/// Sets the protocol version. (Defaults to v3, the only protcol
  /// version supported)

I'll fix this as well, thanks for reporting this.

@shamblett
Copy link
Owner

Sorry forget the above fix, the version setting is on the connect message not the unsubscribe message, we should get the connect message to update the Protocol also -

/// Sets the name of the protocol to use.
  MqttConnectMessage withProtocolName(String protocolName) {
    variableHeader!.protocolName = protocolName;
    Protocol.name = protocolName;
    return this;
  }

  /// Sets the protocol version.
  MqttConnectMessage withProtocolVersion(int protocolVersion) {
    variableHeader!.protocolVersion = protocolVersion;
    Protocol.version = protocolVersion;
    return this;
  }

@CordMemescape
Copy link
Author

Thanks for the feedback Steve.

@CordMemescape
Copy link
Author

Do you want a PR raised for the fix?

@shamblett
Copy link
Owner

No thanks, I've already raised one and incorporated the fix, see #452. I'll close this issue when I release the package at the next version with this in it.

@shamblett
Copy link
Owner

Incorporated into version 9.8.1

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

No branches or pull requests

2 participants