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

Support for websockets & minor changes #119

Merged
merged 3 commits into from
Oct 2, 2017
Merged

Conversation

ejvr
Copy link
Contributor

@ejvr ejvr commented Sep 30, 2017

  • Added support for websockets. This will only work for QT>5.7, because the library needs a version of the QWebSocket::open function which was added in this version. Websocket support is disabled by default (because is requires the QT websockets module which may not always be present). The readme has info on how to enable support. I have only tested this with the EMQ broker.
  • Fixed the unit tests
  • Added an example to the readme on how to create an SSL connection.

The test was broken in commit 420db41
because it introduced a 2bit bitfield to store the QoS. The unit
tests on QMQTT::Message assumed it was possible to store a QoS of 5,
which does not fit in the bitfield.
This feature will only be compiled if explicitly enabled:
qmake CONFIG+=QMQTT_WEBSOCKETS
or:
qbs build qmqtt.webSocketSupport:true

There are 2 reasons:
* websocket support relies on the QWebsocket class which is
  part of the websockets module. Previous versions of qmqtt
  did not rely on this module, so it may not be present on
  systems where old versions of the shared library are in use.
  Adding a new module dependency will make it harder to upgrade
  this systems.
* A call to QWebSocket::open(const QNetworkRequest &) is used,
  which was introduced in Qt 5.6. qmqtt itself requires version
  5.3 or newer.
@mwallnoefer mwallnoefer merged commit 3b22b35 into emqx:master Oct 2, 2017
@mwallnoefer
Copy link
Collaborator

Very good work @ejvr !
Also fine that it gets enabled only with this special switch to not break Qt 5.6 and lower.

@mwallnoefer mwallnoefer mentioned this pull request Oct 2, 2017
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

Successfully merging this pull request may close these issues.

2 participants