Review Request: Add Telepathy Tube support to krdc

Abner Silva abnerf at gmail.com
Tue Sep 22 22:19:21 CEST 2009 


> On 2009-09-22 19:01:32, George Kiagiadakis wrote:
> > trunk/KDE/kdenetwork/krdc/CMakeLists.txt, line 25
> > <http://reviewboard.kde.org/r/1672/diff/1/?file=11752#file11752line25>
> >
> >     I guess the message here needs some love. There is no "Telepathy protocol plugin" in krdc. Maybe change to something like "telepathy tubes support" or whatever...

Probably copy and paste from kopete plugin ;)
Fixed.


> On 2009-09-22 19:01:32, George Kiagiadakis wrote:
> > trunk/KDE/kdenetwork/krdc/CMakeLists.txt, line 27
> > <http://reviewboard.kde.org/r/1672/diff/1/?file=11752#file11752line27>
> >
> >     You don't use KNotificationItem. These checks can be removed.

It was there for some reason, but I think it was removed during the development.
Fixed.


> On 2009-09-22 19:01:32, George Kiagiadakis wrote:
> > trunk/KDE/kdenetwork/krdc/CMakeLists.txt, line 58
> > <http://reviewboard.kde.org/r/1672/diff/1/?file=11752#file11752line58>
> >
> >     Again, no KNotificationItem used anywhere. This should be removed.

Same as above.
Fixed.


> On 2009-09-22 19:01:32, George Kiagiadakis wrote:
> > trunk/KDE/kdenetwork/krdc/CMakeLists.txt, line 70
> > <http://reviewboard.kde.org/r/1672/diff/1/?file=11752#file11752line70>
> >
> >     I'm not sure what -DBUILD_ZEROCONF is, but why did you remove it?

It was a ZEROCONF support and was removed during my tubes integration. It's probably in my patch for some 'merging' reason. But it's not in the kde upstream anymore.


> On 2009-09-22 19:01:32, George Kiagiadakis wrote:
> > trunk/KDE/kdenetwork/krdc/krdc_approver/approver.cpp, line 71
> > <http://reviewboard.kde.org/r/1672/diff/1/?file=11756#file11756line71>
> >
> >     Probably replace "its" with "his/her".

Nice tip.
Fixed.


> On 2009-09-22 19:01:32, George Kiagiadakis wrote:
> > trunk/KDE/kdenetwork/krdc/krdc_approver/approvermanager.cpp, line 58
> > <http://reviewboard.kde.org/r/1672/diff/1/?file=11758#file11758line58>
> >
> >     registerTypes() has already been called from main(). No need to call it again.

Fixed.


> On 2009-09-22 19:01:32, George Kiagiadakis wrote:
> > trunk/KDE/kdenetwork/krdc/tubesmanager.cpp, line 31
> > <http://reviewboard.kde.org/r/1672/diff/1/?file=11771#file11771line31>
> >
> >     There is not much point in "using namepsace Tp;" if you put Tp:: in front of each tpqt4 class, is there? I don't care if you use one style or another, but I think it's better to use only one style in each file and not mix them, as it can be confusing for others. This applies for the approver files as well.

My fault. I just forgot to clean up this thing.
Fixed.


- Abner


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/1672/#review2416
-----------------------------------------------------------


On 2009-09-22 20:14:46, Abner Silva wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1672/
> -----------------------------------------------------------
> 
> (Updated 2009-09-22 20:14:46)
> 
> 
> Review request for telepathy, Urs Wolfer and George Goldberg.
> 
> 
> Summary
> -------
> 
> What is Telepathy:
> "Telepathy[1] is a flexible, modular communications framework that enables real-time communication via pluggable protocol backends. Telepathy creates the idea of communication as a desktop service. It uses D-Bus  to separate components running in separate processes. Telepathy clients use this D-Bus API (usually via a convenience library — e.g. telepathy-glib) to share connections between multiple clients (e.g. an instant messaging program, presence in email application, collaboration in word processor)" from Telepathy Developer's Manual[2]
> 
> What is Telepathy Tubes:
> "Telepathy Tubes provide a user-to-user or user-to-group networking layer which applications can use to transfer data. Unlike a traditional peer-to-peer network, which requires IP addresses or a service discovery mechanism for locating peers, Telepathy Tubes provides contact-to-contact data transfer. Telepathy connection managers handle network traversal using the same technology as for file transfer and streamed media." from Telepathy Developer's Manual[2]
> 
> Use case:
> Alice and Bob have each other added as friend in their IM account (lets consider jabber) and they are using kopete to keep in contact. So Alice decided to share her desktop with Bob to show him something cool. Alice open a chat window with Bob and tell him she is going to send him an invitation to see her desktop and click in "Share Desktop" button inside Kopete's chat window. In that moment Bob will receive a notification (in kde notification area) saying that Alice wants to share her desktop with him. Bob can accept or reject this invitation. Once Bob has accepted the invitation Krdc will be automaticly (DBus) launched and a normal RFB (vnc) session will be started.
> 
> What's up with Krdc:
> This patch implements the Krdc (client) side support and this feature is consider optional during the build. In that case if the user doesn't have Telepathy support in his machine krdc will be compiled without this functionality. The patch was made to be as less intrusive as possible and it is totally limited by #ifdef's.
> 
> Dependencies:
> - Telepathy Qt4 client library.[3]
> - Telepathy Mission Control.[4]
> - Rfb server with tubes support (Krfb is almost done and there's also vino that already supports it).
> - IM Client able to start the session (In the use case above we talked about kopete with already have this support but still on playground).
> 
> References:
> [1] - http://telepathy.freedesktop.org/spec.html
> [2] - http://people.collabora.co.uk/~danni/telepathy-book/chapter.tubes.html
> [3] - http://telepathy.freedesktop.org/wiki/Telepathy-Qt4
> [4] - http://telepathy.freedesktop.org/wiki/Mission Control
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdenetwork/krdc/CMakeLists.txt 1026358 
>   trunk/KDE/kdenetwork/krdc/cmake/modules/FindTelepathyQt4.cmake PRE-CREATION 
>   trunk/KDE/kdenetwork/krdc/krdc_approver/CMakeLists.txt PRE-CREATION 
>   trunk/KDE/kdenetwork/krdc/krdc_approver/approver.h PRE-CREATION 
>   trunk/KDE/kdenetwork/krdc/krdc_approver/approver.cpp PRE-CREATION 
>   trunk/KDE/kdenetwork/krdc/krdc_approver/approvermanager.h PRE-CREATION 
>   trunk/KDE/kdenetwork/krdc/krdc_approver/approvermanager.cpp PRE-CREATION 
>   trunk/KDE/kdenetwork/krdc/krdc_approver/krdc_rfb_approver.client PRE-CREATION 
>   trunk/KDE/kdenetwork/krdc/krdc_approver/krdc_rfb_approver.notifyrc PRE-CREATION 
>   trunk/KDE/kdenetwork/krdc/krdc_approver/main.cpp PRE-CREATION 
>   trunk/KDE/kdenetwork/krdc/krdc_approver/org.freedesktop.Telepathy.Client.krdc_rfb_approver.service.in PRE-CREATION 
>   trunk/KDE/kdenetwork/krdc/krdc_rfb_handler.client PRE-CREATION 
>   trunk/KDE/kdenetwork/krdc/main.cpp 1026358 
>   trunk/KDE/kdenetwork/krdc/mainwindow.h 1026358 
>   trunk/KDE/kdenetwork/krdc/mainwindow.cpp 1026358 
>   trunk/KDE/kdenetwork/krdc/org.freedesktop.Telepathy.Client.krdc_rfb_handler.service.in PRE-CREATION 
>   trunk/KDE/kdenetwork/krdc/tube.h PRE-CREATION 
>   trunk/KDE/kdenetwork/krdc/tube.cpp PRE-CREATION 
>   trunk/KDE/kdenetwork/krdc/tubesmanager.h PRE-CREATION 
>   trunk/KDE/kdenetwork/krdc/tubesmanager.cpp PRE-CREATION 
> 
> Diff: http://reviewboard.kde.org/r/1672/diff
> 
> 
> Testing
> -------
> 
> Krdc was tested using:
> - Telepathy Mission Control 5.3.0+ (or trunk): http://git.collabora.co.uk/?p=telepathy-mission-control.git;a=summary
> - Telepathy Qt4 Abner's Tube branch (till it get merged on upstream): http://git.collabora.co.uk/?p=user/abner/telepathy-qt4.git;a=shortlog;h=refs/heads/tubes
> - Krfb tubes branch: http://git.collabora.co.uk/?p=user/gberg/krfb-telepathy.git;a=shortlog;h=refs/heads/incoming-tubes-notification
> - Kopete telepathy branch: http://websvn.kde.org/trunk/playground/network/kopete/protocols/telepathy
> - Vino trunk: git://git.gnome.org/vino
> 
> Use cases:
> - Alice send an invitation to Bob and he accept it.
> - Alice send an invitation to Bob and he reject it.
> - During a desktop sharing session Bob closed the session.
> - During a desktop sharing session Alice closed the session.
> - Closing the whole application during a session.
> - Server killed during a session.
> 
> All tests had good results.
> 
> 
> Screenshots
> -----------
> 
> Desktop Sharing invitation received
>   http://reviewboard.kde.org/r/1672/s/207/
> KRDC estabilishing a RFB session
>   http://reviewboard.kde.org/r/1672/s/208/
> Remote View via Telepathy Tubes
>   http://reviewboard.kde.org/r/1672/s/209/
> KRFB peer view
>   http://reviewboard.kde.org/r/1672/s/210/
> 
> 
> Thanks,
> 
> Abner
> 
>

More information about the KDE-Telepathy mailing list