search

gausby / tortoise

A MQTT Client written in Elixir
Apache License 2.0
314 stars 54 forks source link

New Websocket transport #106

Open ejscunha opened 5 years ago

ejscunha commented 5 years ago

Hey đź‘‹

First of all thank you for this amazing library, it is really well written and organised, and you can learn a lot just by scrolling around.

I'm interested in having a MQTT library with support for Websockets, a colleague and I already did some work on Hulaaki to try to achieve it, but it felt a bit hackish. So I looked into this library to see if it would be better suited for it. The Tortoise.Transport behaviour is heavily attached to gen_tcp module API, so I'm developing a library that wraps a Websocket client (gun) and provides an API similar to gen_tcp. The problem now is on testing, since the Tortoise.Transport behaviour requires some functions that are essentially used for testing and not for the normal function of the client. Basically I would have to implement a Websocket server (or use a third party library) to listen and accept connections for testing.

I would like to know if you would be interested in having support for Websocket transport in this library, and if so if you have any insights on how the support for Websockets could be achieved (refactoring the Tortoise.Transport behaviour an option?)?

gausby commented 5 years ago

Thanks. I would love to support websockets in this library, if we can get it working.

Perhaps I should scale the abstraction for the transports a bit back so they don't need to implement the server parts, but it has been neat for testing. Perhaps there is a websocket server implementation we could add as a test dependency and include in the official websocket Tortoise transport—if we choose to include it—websockets could perhaps be a package one could add as a dependency; ssl and tcp does rely on stuff already in OTP so I think it is reasonable to support them out of the box.

Anyways, as you might now: I am working on MQTT 5 support. I hope to get it stable soon, because it is kinda hard to accept pull requests right now, as most of Tortoise will change in the near future (because MQTT 5 demands it)

I promise I will keep an eye on my issues :)

gausby commented 5 years ago

As such I think implementing the server part would be best, as it would allow us to test the transport without having a MQTT server with the given transport enabled.

ejscunha commented 5 years ago

That was my first approach, I used the (Socket)[https://github.com/meh/elixir-socket] package to implement the server part. I just wanted to know if you were comfortable with this approach.

I still have somethings to finish before I can open a PR, but I can open a PR on your MQTT 5 branch or if you prefer I can wait until it is merged to master.

gausby commented 5 years ago

The system should support adding transports as dependencies; so I don't think a PR to Tortoise core should be necessary. By doing this we can update the websocket transport without having to push a new version of Tortoise.

Perhaps I should make an organisation for Tortoise such that we can have the dependencies under one org. What do you think of this plan ?

ejscunha commented 5 years ago

Oh I didn't understand the transport as a dependency before, that makes sense.

đź‘Ťfor the organisation

ejscunha commented 5 years ago

Hey, I created this repository https://github.com/ejscunha/tortoise_websocket (we can later move it the Tortoise organisation) which will provide a Tortoise.Transport.Websocket module. It has a PR open with the client and transport implementation.

ejscunha commented 5 years ago

Also, I think that Tortoise will need minor changes in order to support websockets. In the guards where it checks the transport type (i.e. when transport in [:tcp, :ssl]), we need to add :websockets. Do you have any suggestion to make this better (maybe an overridable configuration)?