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 5 #1250

Merged
merged 47 commits into from May 23, 2020
Merged

Conversation

alexandruioanp
Copy link
Contributor

@alexandruioanp alexandruioanp commented Apr 25, 2020

This PR adds support for MQTT 5.0 (protocol level 5) as summarised here.

Most of these new features are implemented via Properties. A large part of this diff is comprised of code to handle the encoding and decoding of these properties. I was considering pulling out the property handling code to a separate set of files (mqtt-prop.c/h?)

EDIT:
See here for how to print properties using the mosquitto client. You should get something like this (see the properties field):

# mosquitto_sub  --verbose -t "#" -F %J -V 5

{"tst":1587821578,"topic":"iot-2/evt/status/fmt/json","qos":0,"retain":0,"payloadlen":93,"properties":{"user-properties":{"Contiki":"v4.5+"}},"payload":{"d":{"Platform":"native","Seq #":3,"Uptime (sec)":418,"Def Route":"fd00::1","RSSI (dBm)":0}}}

@g-oikonomou
Copy link
Member

!

I was considering pulling out the property handling code to a separate set of files (mqtt-prop.c/h?)

Sounds like a reasonable idea. I see you have wrapped them around a cpp #if for conditional compilation - I don't think you need to necessarily do that, perhaps you can always compile and let the linker garbage collect those symbols.

I'm not sure why the diff is showing up with all those file renames.

@alexandruioanp
Copy link
Contributor Author

Regarding the renames, I made some space for contiguous MQTT test numbering (02 through 07, though I see I missed 07-native-ping.sh).

The compiler was throwing errors for unused functions, so I had to wrap some of them in ifdefs, but I think it might be my container misbehaving.

@g-oikonomou
Copy link
Member

The compiler was throwing errors for unused functions, so I had to wrap some of them in ifdefs, but I think it might be my container misbehaving.

Hmmm I this this will happen for unused static functions. But if you were to move them to a separate file and make them non-static it would go away.

Copy link
Member

@g-oikonomou g-oikonomou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall great work, just some bureaucracy I'm afraid Alex...

os/net/app-layer/mqtt/mqtt-prop.h Outdated Show resolved Hide resolved
os/net/app-layer/mqtt/mqtt-prop.h 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
Copy link
Member

@g-oikonomou g-oikonomou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's all fine, just need to get the macro/type/function names prefixed with the module name properly

os/net/app-layer/mqtt/mqtt-prop.h Outdated Show resolved Hide resolved
os/net/app-layer/mqtt/mqtt-prop.h Outdated Show resolved Hide resolved
os/net/app-layer/mqtt/mqtt-prop.h Outdated Show resolved Hide resolved
os/net/app-layer/mqtt/mqtt-prop.h Outdated Show resolved Hide resolved
os/net/app-layer/mqtt/mqtt-prop.h Outdated Show resolved Hide resolved
os/net/app-layer/mqtt/mqtt-prop.h Outdated Show resolved Hide resolved
os/net/app-layer/mqtt/mqtt-prop.h Outdated Show resolved Hide resolved
@g-oikonomou g-oikonomou merged commit 6375bdf into contiki-ng:develop May 23, 2020
@vera vera mentioned this pull request Jun 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants