search

adafruit / Adafruit_MQTT_Library

Arduino library for MQTT support
MIT License
566 stars 292 forks source link

Client disconected due protocol error when counting PUBLISH packets from 0 #203

Open apsite opened 2 years ago

apsite commented 2 years ago

Hello, I discovered strange problem during publishing messages. Every first PUBLISH packet gets no response and additionally server closes connection with:

Client 7879f4a.... disconnected due to protocol error.

...when packets are counted from 0. When I set packet_id_counter variable initially to 1 in Adafruit_MQTT::Adafruit_MQTT, everything works OK

Adafruit_MQTT::Adafruit_MQTT(const char *server, uint16_t port, const char *cid,
                             const char *user, const char *pass) {
  servername = server;
  portnum = port;
  clientid = cid;
  username = user;
  password = pass;

  // reset subscriptions
  for (uint8_t i = 0; i < MAXSUBSCRIPTIONS; i++) {
    subscriptions[i] = 0;
  }

  will_topic = 0;
  will_payload = 0;
  will_qos = 0;
  will_retain = 0;

  keepAliveInterval = MQTT_CONN_KEEPALIVE;

  // changed code
  packet_id_counter = 1;
}

This is also related to QOS level. packet_id_counter can be set to 0 but only when QOS 0 is used.

I suspect that this is exact same case as described in MQTT docs:

SUBSCRIBE, UNSUBSCRIBE, and PUBLISH (in cases where QoS > 0) Control Packets MUST contain a non-zero 16-bit Packet Identifier [MQTT-2.3.1-1].

lvqier commented 2 years ago

Same problem on sending SUBSCRIBE immediately after connect.

Mollayo commented 2 years ago

I agree with you both. Actually, the other MQTT library (pubsubclient) is using a packet id counter which is always different from 0. See here:

@brentru : This bug requires modifying few lines. I can make the modifications myself and make a pull request.

brentru commented 2 years ago

@Mollayo Thank you for finding this, please make a pull request and tag me so I can review it.

Mollayo commented 2 years ago

@brentru I have submitted the pull request. It is available for review. See here: https://github.com/adafruit/Adafruit_MQTT_Library/pull/207

Mollayo commented 2 years ago

@brentru I have submitted the pull request. It is available for review. See here: https://github.com/adafruit/Adafruit_MQTT_Library/pull/207

einglis commented 1 year ago

@Mollayo @brentru This looks like it stalled, and all three (!) PRs were closed without explanation. Is there anything you or I could do to move it forward?