Skip to content
This repository has been archived by the owner on Aug 13, 2018. It is now read-only.

Support the MQTT over Websocket subprotocol #56

Closed
tuukka opened this issue Mar 30, 2016 · 9 comments
Closed

Support the MQTT over Websocket subprotocol #56

tuukka opened this issue Mar 30, 2016 · 9 comments
Assignees

Comments

@tuukka
Copy link
Contributor

tuukka commented Mar 30, 2016

MQTT over Websocket is sent as binary data frames (OASIS and ISO standardised): http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc398718127

It would be nice to decode and display these based on the MQTT packet types: http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc398718018

Would this be a suitable protocol parser? https://www.npmjs.com/package/mqtt-packet

@tuukka tuukka changed the title Support the MQTT over Websocket subprocotol Support the MQTT over Websocket subprotocol Mar 30, 2016
@janodvarko
Copy link
Member

I like that idea.

@esphen: do you want to take a look at this?

Honza

@eliihen
Copy link
Member

eliihen commented Mar 30, 2016

@janodvarko

Sure thing. I may have some free time this weekend. If so, I'll look at it then.

@janodvarko
Copy link
Member

Awesome!

@eliihen
Copy link
Member

eliihen commented Apr 1, 2016

All right, I've had a look at it today, and here are my initial findings.

mqtt-packet looks nice, but it has dependencies on some node.js stuff, which means we can't simply use it in the browser. I tried to hack it to work, but to no avail. We would have to rewrite mqtt-packet to work in the browser, which seems like a bit of work.

Another option is to find some other parser utility that supports the browser. I tried searching for one, but amazingly, I couldn't find anything at all. I did find MQTT.js, but that is a whole client/server-thing, and that is way overkill. We just need to parse the messages.

Looking at the source of mqtt-packet, it looks like a lot of work to implement ourselves.. I made an issue over at mqttjs/mqtt-packet#11, asking the author on his opinion on supporting the browser - let's see what he says.

@tuukka
Copy link
Contributor Author

tuukka commented Apr 1, 2016

@esphen mqtt-packet is one part of MQTT.js! As MQTT.js works in the browser via browserify or webpack, I'd expect standalone mqtt-packet to work as well. I haven't tested this scenario though.

@tuukka
Copy link
Contributor Author

tuukka commented Apr 3, 2016

@esphen Glad to hear at at mqttjs/mqtt-packet#11 that you got browserify working. I had a look at your branch for this issue - exciting times :-) The browser hangs with 100% CPU after 500 messages or so, tested at http://dev.hsl.fi/mqtt/browser. The packet type is often shown wrong (e.g. "publish" shown as "connack" - async issue?), tested with a filtered down GUI version of the previous MQTT feed at http://beta.digitransit.fi/linjat/HSL:2550:0:01. One next step might be to try to decode the MQTT.payload Buffer as UTF-8, then try to parse the result string as JSON.

@eliihen
Copy link
Member

eliihen commented Apr 3, 2016

@tuukka

Awesome that you tested the branch, that's just what we need :)

Yeah, I noticed the slowdown and occasional locking as well, quite annoying. That's why I didn't send a PR quite yet. In contrast with the other protocol parsers we are using, mqtt-packet works async, and sends an event when it is done parsing. This first commit tries to make it work synchronously, but I think that may be causing the performance issues. I'll have a look at implementing it async instead.

Good thing you are able to tell me that the packet types are wrong, I don't work with mqtt, so I really don't have any frame of reference to test whether it works correctly. Not quite sure why that happens, but I'll have a look into that as well.

@eliihen
Copy link
Member

eliihen commented Apr 4, 2016

@tuukka stepped in with a great pull request and seemingly nailed the performance problem I was having. Good job, mate.

As I understand it, you would like to have the MQTT payload buffer decoded as well before we finish up this patch. I'll have a look into that on wednesday, don't think I have time before then. Unless you would like to have a go at another patch, @Tukka?

@janodvarko
Copy link
Member

@tuukka @esphen

New 0.6.0 version including MQTT support released and uploaded to AMO (awaiting full review), thank you guys!

@tuukka list of contributors updated :)

@esphen I am assigning the bug to you if that's ok

Honza

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants