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

Add support for MQTT 3.1.1 #1060

Open
wants to merge 12 commits into
base: develop
from

Conversation

@alexandruioanp
Copy link
Contributor

commented Sep 8, 2019

This PR adds the changes needed for MQTT 3.1.1 (protocol level 4) support as described here.

Based on my understanding of the UTF-8 requirements, the client code should be compliant as-is.

@g-oikonomou g-oikonomou self-assigned this Sep 8, 2019

@g-oikonomou
Copy link
Member

left a comment

Thanks for this!!!

First pass of comments in this review.

We have an execution test for the MQTT client, using mosquitto on the server-side. Worth adding a new test so we can check MQTT 3.1 as well as 3.1.1?

examples/mqtt-client/mqtt-client.c Outdated Show resolved Hide resolved
examples/mqtt-client/mqtt-client.c Outdated Show resolved Hide resolved
os/net/app-layer/mqtt/mqtt.c Outdated Show resolved Hide resolved
os/net/app-layer/mqtt/mqtt.c Outdated Show resolved Hide resolved
os/net/app-layer/mqtt/mqtt.c Outdated Show resolved Hide resolved
os/net/app-layer/mqtt/mqtt.c Outdated Show resolved Hide resolved
os/net/app-layer/mqtt/mqtt.h Outdated Show resolved Hide resolved
os/net/app-layer/mqtt/mqtt.h Outdated Show resolved Hide resolved
@alexandruioanp

This comment has been minimized.

Copy link
Contributor Author

commented Sep 10, 2019

I have a commit that adds an execution test for V3.1.1 but it looks like it might have to wait for #1061.

@alexandruioanp alexandruioanp requested a review from g-oikonomou Sep 13, 2019

alexandruioanp added 10 commits Sep 7, 2019
[MQTT] Make CleanSession flag user configurable
CleanSession set to 1 will cause the server to discard:
- 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.

Creating a new session that is not "clean" is a potential optimisation
as the client does not need to re-subscribe to topics.
[MQTT311] Support MQTT Session Present
A new field in the CONNACK message after a CONNECT with
CleanSession=0 indicates if the server has a session stored
for the requested Client ID. This may be used to avoid
re-subscribing to topics.
[MQTT311] Support zero-byte Client ID
Client IDs may be zero bytes, in which case the server
assigns the client a unique Client ID.
[MQTT311] Support SUBACK return codes
SUBACK may indicate a failure via the return code byte.

@alexandruioanp alexandruioanp force-pushed the alexandruioanp:pr/mqtt311 branch from 6c0102d to 9e1d2f7 Sep 14, 2019

@alexandruioanp

This comment has been minimized.

Copy link
Contributor Author

commented Sep 14, 2019

#1062 must be merged first.

@alexandruioanp alexandruioanp force-pushed the alexandruioanp:pr/mqtt311 branch from 9e1d2f7 to 870e51b Sep 14, 2019

@g-oikonomou

This comment has been minimized.

Copy link
Member

commented Sep 15, 2019

Valgrind tests are really cool and it looks like the test is passing nicely with #1062. Only thing that's left AFAIK is the check for NULL chars inside strings as discussed offline.

@g-oikonomou

This comment has been minimized.

Copy link
Member

commented Sep 20, 2019

Anyone any idea why this is failing? I restarted but it failed in the same fashion again.

==== 06-native-coap.log ====
2315 Get .well-known/core
2316 Invalid URL. Protocol is not given or URL is malformed.
2317 Get test/push
2318 Invalid URL. Protocol is not given or URL is malformed.
2319 06-native-coap                   TEST FAIL    0/2

https://travis-ci.org/contiki-ng/contiki-ng/jobs/585770583

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.