[Development] websockets (was RE: Qt 5.3 Feature freeze is coming quite soon...)

Fri Jan 17 18:10:56 CET 2014

Fredag 17. januar 2014 17.56.50 skrev Kurt Pattyn:
> On 17 Jan 2014, at 14:10, Gladhorn Frederik <Frederik.Gladhorn at digia.com> 
wrote:
> > Hi Kurt,
> > 
> > I'm quite impressed with the state of the websockets module.
> 
> Thanks!
> 
> > I tried reviewing the code, but I have to admit that I just started
> > reading the RFC a bit, so here are some first random notes.
> > 
> > When websockets becomes part of a Qt module you can consider using
> > QObjectPrivate which makes the d pointer implementation slightly cleaner
> > and removes the need for duplicate d pointers (one from qobject one for
> > the class's own implementation.
> Okay, will fix that.
> 
> > In the public interface I'd generally declare signals with const refs,
> > alone for consistency.
> Done. Your patch has been reviewed and merged (thanks for that).
> 
> > While not really harmful there is generally no point in explicitly
> > initializing empty strings, I'd personally remove the QString() calls
> > from the class initialzers just to remove a few lines.
> > 
> > qsslserver newEncryptedConnection is never emitted, I didn't look into
> > what it means though.
> Hmmm, I’ll have a look at that one.
> 
> > qwebsockethandshakeresponse: m_acceptedExtension doesn't seem used,
> > instead there is a string list passed through, I suppose it can simply be
> > removed, it also has a getter that will always return an empty string.
> Will check this also.
> 
> > I was reading the RFC a bit, in section 9.1 there are some notes about the
> > extensions:
> > 
> > const QList<QString> matchingExtensions =
> > 
> >                     supportedExtensions.toSet().intersect(request.extensio
> >                     ns().toSet()).toList();> 
> > does no longer guarantee the order of extentions - according to sec 9.1.
> > this is important.
> Right. Will fix this.
> 
> > The same seems to go for matchingProtocols - the first one is picked,
> > since QSet doesn't guarantee any order it is basically chosen by random.
> > I don't know how significant that is but I assume the client's first
> > choice (?) should be taken.
> Hmm, the list is sorted in descending order, and the server picks the latest
> protocol version that it supports. That seems fine to me, but I’ll
> double-check the RFC.

My bad, you are right, I overlooked the sorting.

Greetings,
Frederik

> 
> 
> Cheers,
> 
> Kurt
> 
> > Greetings,
> > Frederik
> > 
> > From: development-bounces+frederik.gladhorn=digia.com at qt-project.org
> > [development-bounces+frederik.gladhorn=digia.com at qt-project.org] on
> > behalf of Kurt Pattyn [pattyn.kurt at gmail.com] Sent: Thursday, January 16,
> > 2014 12:51 PM
> > To: development at qt-project.org
> > Cc: Heikkinen Jani; 'thiago.macieira at intel.com'
> > (thiago.macieira at intel.com); richmoore44 at gmail.com;
> > releasing at qt-project.org Subject: Re: [Development] Qt 5.3 Feature freeze
> > is coming quite soon...
> > 
> > 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.
> > 
> > Cheers,
> > 
> > Kurt
> > 
> > On 16 Jan 2014, at 09:28, Heikkinen Jani <Jani.Heikkinen at digia.com> wrote:
> >> Hi all,
> >> 
> >> I want to remind you all that Qt 5.3 feature freeze is coming pretty
> >> soon. Feature freeze for Qt 5.3 is 14th Feb 2014 so there is only 4
> >> weeks left for implementing new features.
> >> 
> >> Qt 5.3 schedule can be found here:
> >> http://qt-project.org/wiki/Qt-5.3-release . There is also link to the Qt
> >> 5.3 new features page
> >> (http://qt-project.org/wiki/New-Features-in-Qt-5.3). You can start
> >> collecting list of new features there…
> >> 
> >> Note: Let’s keep the feature freeze date! If your feature isn’t ready at
> >> that point let’s then move it to Qt 5.4 release instead of taking in it
> >> in Qt 5.3 and fighting with it whole release time… This is also action
> >> point for each maintainer: Make sure your component is ready for feature
> >> freeze at that date.
> >> 
> >> Just a reminder:
> >> In the feature freeze all new functionality must
> >> 
> >> - Compile on all reference platforms (If a module/feature is only for one
> >> platform, make sure qmake/make does nothing on the other platforms) -
> >> Have tests. Automated tests should cover as much as possible of the new
> >> functionality. If certain areas are not covered by automated tests,
> >> there must be clarification how testing will be done for those - Have
> >> documentation. No undocumented public API. Basic docs have to be there,
> >> only polishing should still be required after the freeze - Have
> >> examples. Have some examples showing how to use the API. Examples need
> >> to be linked to from documentation.
> >> 
> >> In addition, new modules need to
> >> 
> >> - Follow the branching scheme. dev/stable/release should be there. A new
> >> module can be ok to only have dev, with stable being created at
> >> branching time. - Have a CI system. New modules that are going to be
> >> part of Qt releases need to have a CI system set up
> >> 
> >> Br,
> >> Jani
> >> _______________________________________________
> >> Development mailing list
> >> Development at qt-project.org
> >> http://lists.qt-project.org/mailman/listinfo/development

-- 
Best regards,
Frederik Gladhorn
Senior Software Engineer - Digia, Qt
Visit us on: http://qt.digia.com