(removing releasing and other dev subscribed folks from CC)

On Thursday 16. January 2014 12.51.28 Kurt Pattyn wrote:
> I would like to propose the QtWebSockets module as a new feature for Qt 5.3
> (see https://qt.gitorious.org/qtplayground/websockets/source/master)
> 
> There are a number of requests in Jira:
> https://bugreports.qt-project.org/issues/?jql=labels%20%3D%20websockets
> asking to include web socket functionality in Qt, so maybe it is a good
> time to include it now. Besides that, EnginIo could make use of this module
> as well (currently it uses an own implementation of web sockets).
> 
> QtWebSockets is fully RFC6455 compliant (see
> http://tools.ietf.org/html/rfc6455) and is successfully tested against the
> latest Autobahn TestSuite. Performance is OK as well (test report is not
> included because of too big).
> 
> There is a C++ interface as well as a QML module.
> The C++ API consists of the following classes:
> QWebSocket
> QWebSocketServer
> QWebSocketProtocol
> 
> The QML API consists of the following component:
> WebSocket (included in the Qt.WebSockets 1.0 module).
> 
> I see 2 options: either add the functionality to the QtNetwork module, or
> add it as a Qt add-on. Maybe adding it to QtNetwork would be a ‘natural’
> place: QWebSocket would then live besides QTcpSocket, QWebSocketServer
> would live besides QTcpServer.
> 
> The module compiles on all reference platforms, has automated unit tests and
> manual unit tests (against Autobahn TestSuite), is fully documented, and
> has a number of examples. The module is included in the CI system of Qt
> (see http://testresults.qt-project.org/ci/WebSockets_master_Integration/).
> 
> 
> Please raise your votes.

I think it would be a great feature to have in Qt.

I've had a brief look at the API and I have a few comments:

* qwebsocketprotocol.h:
   * V_Unknow should probably be V_Unknow_n_ :)
   * CC_GOING_AWAY, etc: This should probably use camel casing instead of all 
upper case, in order to be consistent with the rest of the Qt API.
   * I suggest to remove inline Version currentVersion() { return V_LATEST; } - 
the function adds no value IMHO as the latest enum value will always be that.

* qwebsocket.h:

  * qint64 QWebSocket::write(const char *message)
    This function appears to send "message" as a text message and expects utf-8 
encoding. But unfortunately I think it looks misleading because 
QIODevice::write has the same signature and means binary content - no 
assumption about encodings are done. I suggest to rename the
method to writeTextMessage(const QString &) and just writeBinaryMessage(const 
QByteArray &) - without any const char * overloads. It makes it very easy to 
understand the client code when reading it ("ahh, it's sending a text message" 
or "ahh, it's sending a binary message").

   * The API appears to miss a complete the notion of fragments. writeMessage 
appears to always send a frame, but on the receiving end the concept of 
fragments appears - this looks inconsistent. I'll elaborate a bit more about 
that separately.

I can think of two different ways of using web sockets:

1) Sending/receiving messages of a given protocol
2) Sending/receiving streams

It appears that the web socket protocol encapsulates both, with the notion of 
messages consisting of a variable (potentially unlimited) number of fragments. 
I think it is important to make both use-cases very easy. When I'm implementing
a message based protocol (for example chat client), I really want the 
underlying framework to deal with fragments and only deliver entire frames to 
me. When implementing for example a video streaming service, I need full 
control (on both ends)
over fragmentation. And to make it worse, it seems that over the same 
connection I could be sending a stream (variable fragments of the same frame) 
intermixed with single fragments. Is my interpretation of the spec correct here?


Either way what I'm missing from the API is making these two use-cases easier. 
Perhaps it would make sense to offer convenience, such as a boolean property to 
indicate that all text frames should only be signalled (textFrameReceived) as 
entire
frame instead of fragments.

I have more thoughts on this, but let's start the discussion here maybe :)

Simon
_______________________________________________
Development mailing list
[email protected]
http://lists.qt-project.org/mailman/listinfo/development

Reply via email to