search

firebug / websocket-monitor

Not compatible with Firefox Quantum (57 and newer)
Other
147 stars 18 forks source link

Support the MQTT over Websocket subprotocol #56

Closed tuukka closed 8 years ago

tuukka commented 8 years ago

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

janodvarko commented 8 years ago

I like that idea.

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

Honza

eliihen commented 8 years ago

@janodvarko

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

janodvarko commented 8 years ago

Awesome!

eliihen commented 8 years ago

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 https://github.com/mqttjs/mqtt-packet/issues/11, asking the author on his opinion on supporting the browser - let's see what he says.

tuukka commented 8 years ago

@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 commented 8 years ago

@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 commented 8 years ago

@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 commented 8 years ago

@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 commented 8 years ago

@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