Add Support for WebSockets

[JarredAllen]

This PR adds support for WebSockets (as requested in #10).

This supports doing mqtt over an insecure websocket or over a secure websocket with default TLS configuration (I didn't implement custom TLS config for secure websockets because it would be more work and I don't need that functionality for my use cases, but it shouldn't be too hard to add if someone else wants it).

I did also have to move erroring on a failed read from happening immediately to only raising the error on an attempt to read an incoming packet, since the websocket implementation would otherwise raise an error when you disconnect. Hopefully that's acceptable.

[fluffysquirrels]

Thanks very much for this, I'll take a look at it in detail shortly.

[JarredAllen]

Are there any updates on this? I don't mean to pressure you (thanks for your work on this great library), but I'd greatly appreciate it if you could give an update on the status of this PR.

[fluffysquirrels]

Hi,

Sorry for the delay on this.

One problem I have is that in WebSockets mode the config is set differently than in TCP or TLS mode. In TCP or TLS mode .host() sets hostname and .port() sets port number, but in WebSockets mode .host() sets a URL and .port() is ignored. At the very least I think these should be consistent, but I think I prefer using URL for all modes, as this makes it simpler for a user to use their own config file and switch between protocols without caring about the details of what protocol is in use. This will require a breaking change, but the library is fairly young, and only a simple usage of format! should be required in the consumer. The MQTT standard suggests mqtt and mqtts protocols for the URL, which I like, and then we can use ws and wss for WebSockets URLs.

Similarly, your patch as it is supports setting TLS client config for TLS mode, but not for secure WebSockets mode (i.e. wss). I'd like to see ClientBuilder.set_tls_client_config() pass through the configuration to tokio-tungstenite. If your configuration is anything like mine, you're using self-signed certificates on your servers and setting TLS client config to securely connect to self-signed certificates requires custom TLS client config.

I have a few other comments that I'll make against the appropriate lines of code.

[JarredAllen]

Okay, I've done those changes. I agree that it's nicer to use a url to set the remote host (I only didn't do that originally because I was hesitant to suggest a breaking change). I've also added in tls client configuration.

I am a bit hesitant on what port should be used when no port is specified. I left it just using 1883 by default, regardless of the connection type, since that's what it did before. However, I feel like it could be nicer if it defaulted to the port for the corresponding type (80 for ws, 443 for wss, 1883 for mqtt, 8883 for mqtts). If you agree that this would be better, I can do that change pretty easily.

[fluffysquirrels]

However, I feel like it could be nicer if it defaulted to the port for the corresponding type (80 for ws, 443 for wss, 1883 for mqtt, 8883 for mqtts).

I agree, please go ahead with this change.

[JarredAllen]

However, I feel like it could be nicer if it defaulted to the port for the corresponding type (80 for ws, 443 for wss, 1883 for mqtt, 8883 for mqtts).

I agree, please go ahead with this change.

Okay, done!

[stabler]

Hi @fluffysquirrels - I'm eagerly following this PR, just wanted to register interest :)

[fluffysquirrels]

@JarredAllen Just merged this into master with only a slight tweak to the set_url docs to mention the default port numbers.

Thanks again for your support and sorry for the delays in getting it merged. I'll be releasing v0.3.0 shortly with your changes.