Review Request: Enable call holding in ktp-call-ui

[Rohan Garg]

> On Aug. 4, 2012, 10:26 a.m., George Kiagiadakis wrote:
> > src/call-window.cpp, lines 466-469
> > <http://git.reviewboard.kde.org/r/105829/diff/4/?file=75962#file75962line466>
> >
> >     1) StatusArea does not implement the Error message type, so these are essentially no-op. I thought this was why you wanted the KMessageWidget...
> >     
> >     2) The messages are not very clear. You need to tell the user that the call is on hold, somehow. Especially in the case of ResourceNotAvailable, the string should be something like "Failed to put the call off hold: Some devices are not available", because this can only happen if you are trying to put the call off hold and the backend fails to open some input/output device (camera, sound card).

1) I planned to do that as a separate patch altogether, or I could add it to this one ... your choice :)

2) Will fix


> On Aug. 4, 2012, 10:26 a.m., George Kiagiadakis wrote:
> > src/call-window.cpp, lines 477-478
> > <http://git.reviewboard.kde.org/r/105829/diff/4/?file=75962#file75962line477>
> >
> >     This cannot happen in StateUnheld. It should also be handled as unknown error.

Why not? Let's say the user hold's a call and a couple of seconds later the driver responsible for Video/Audio capture crashes or the wires are not securely connected causing a input error. Won't the telepathy backend reply with a  Tp::LocalHoldStateReasonResourceNotAvailable error?


> On Aug. 4, 2012, 10:26 a.m., George Kiagiadakis wrote:
> > src/call-window.ui, lines 39-50
> > <http://git.reviewboard.kde.org/r/105829/diff/4/?file=75963#file75963line39>
> >
> >     Afaict, the errorWidget here is on the same page as the user avatar label, which means that if we are on a video call, this widget will not appear...

Derp. I'll fix this one ...


> On Aug. 4, 2012, 10:26 a.m., George Kiagiadakis wrote:
> > src/main.cpp, line 67
> > <http://git.reviewboard.kde.org/r/105829/diff/4/?file=75964#file75964line67>
> >
> >     This will cause a failure if the hold interface is not available in the call, but I think both gabble and rakia implement it so I think we can get away with it for now

Is it possible to query the backend if a particular feature is available? If yes, why not do it for all the features we add?


- Rohan


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


On Aug. 3, 2012, 9:56 p.m., Rohan Garg wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105829/
> -----------------------------------------------------------
> 
> (Updated Aug. 3, 2012, 9:56 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Description
> -------
> 
> Enables call holding in ktp-call-ui
> 
> 
> Diffs
> -----
> 
>   src/main.cpp 4ab3a23 
>   src/call-window.ui 14144cc 
>   src/call-window.h d8741c3 
>   src/call-window.cpp e4aed4d 
>   src/call-manager.cpp df6e701 
> 
> Diff: http://git.reviewboard.kde.org/r/105829/diff/
> 
> 
> Testing
> -------
> 
> Successfully held and unheld calls using ktp-call-ui
> 
> 
> Thanks,
> 
> Rohan Garg
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20120804/6b8f7892/attachment.html>