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

New Websocket transport #106

Open
ejscunha opened this issue May 14, 2019 · 7 comments
Open

New Websocket transport #106

ejscunha opened this issue May 14, 2019 · 7 comments

Comments

@ejscunha
Copy link

ejscunha commented May 14, 2019

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
Copy link
Owner

gausby commented May 20, 2019

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
Copy link
Owner

gausby commented May 20, 2019

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
Copy link
Author

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
Copy link
Owner

gausby commented May 22, 2019

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
Copy link
Author

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

👍for the organisation

@ejscunha
Copy link
Author

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
Copy link
Author

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)?

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

No branches or pull requests

2 participants