search

moscajs / aedes-persistence

In-memory implementation of an Aedes persistence, with abstract tests
MIT License
13 stars 22 forks source link

Subscribe to a topic with a QoS 0 #1

Closed marvinroger closed 8 years ago

marvinroger commented 8 years ago

In the README, for addSubscriptions, there is a note saying Any subscriptions with qos: 0 will be ignored.. Why? It is legal to subscribe to a topic with a QoS of 0.

mcollina commented 8 years ago

This module is to support persistent/offline subscription. Those are only QoS 1 and 2. Check in the MQTT spec.

Would you mind sending a PR to express this in the README? Thanks! Il giorno sab 5 dic 2015 alle 05:38 Marvin Roger notifications@github.com ha scritto:

In the README https://github.com/mcollina/aedes-persistence#instanceaddsubscriptionsclient-subscriptions-callbackerr-client, for addSubscriptions, there is a note saying Any subscriptions with qos: 0 will be ignored.. Why? It is legal to subscribe to a topic with a QoS of 0.

— Reply to this email directly or view it on GitHub https://github.com/mcollina/aedes-persistence/issues/1.

marvinroger commented 8 years ago

I am sorry, I don't see such a thing. Maybe I am missing something? From the spec (anchor tag):

The Session state in the Server consists of:
·  The existence of a Session, even if the rest of the Session state is empty.
·  The Client’s subscriptions.
·  QoS 1 and QoS 2 messages which have been sent to the Client, but have not been completely acknowledged.
·  QoS 1 and QoS 2 messages pending transmission to the Client.
·  QoS 2 messages which have been received from the Client, but have not been completely acknowledged.
·  Optionally, QoS 0 messages pending transmission to the Client.

QoS 0 message persistence is optional, but it's not written that QoS 0 subscriptions persistence is optional/non-existent. Am I wrong?

mcollina commented 8 years ago

The relevant section is 3.1.2.4. QoS 0 persistence is optional, but to achieve speed delivering QoS 0 I'm really against supporting it for QoS 0. Beware that this change impacts a a bit of Aedes code more than here.

Most brokers do not do it anyway.

mcollina commented 8 years ago

In fact, you are right and I am wrong.

QoS 0 subscriptions needs to be restored, but no message need to be stored in there. I need to add some unit test for this :).

mcollina commented 8 years ago

Fixed.

marvinroger commented 8 years ago

Glad to help! By the way, thanks for your hard work supporting MQTT in node.

mcollina commented 8 years ago

Any contribution to this project is really welcomed. Also, a second pair of eyes regarding MQTT spec compliance would be really helpful. Il giorno sab 5 dic 2015 alle 15:13 Marvin Roger notifications@github.com ha scritto:

Glad to help! By the way, thanks for your hard work supporting MQTT in node.

— Reply to this email directly or view it on GitHub https://github.com/mcollina/aedes-persistence/issues/1#issuecomment-162255971 .