[Bug 28367] Add StreamTube interface support

bugzilla-daemon at freedesktop.org bugzilla-daemon at freedesktop.org
Fri Jun 4 16:52:53 CEST 2010


https://bugs.freedesktop.org/show_bug.cgi?id=28367

Olli Salli <ollisal at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ollisal at gmail.com

--- Comment #3 from Olli Salli <ollisal at gmail.com> 2010-06-04 07:52:52 PDT ---
Review of your latest commits:

Updating parameters on successful offer(): I think you did it in a really
complicated fashion :( Why not just stash the parameter map the app gives to
offer() in the PendingOpenTube instance and then if the underlying processes
are successful, upload it to StreamTubeChannel::parameters before doing
setFinished(), via a protected method (or a public one on the inherited private
class?). This would make the parameters update happen *before finished() is
emitted* - currently parameters getting updated to the real value happens an
arbitrary time after the PendingOpenTube finishes (when the Get parameter call
happens to return), with no corresponding signal or anything to indicate "it
now has the correct value". And no, I wouldn't like to add another signal for
this :)

Another thing where we actually might need real reintrospection for would be if
somebody else does offer() and we are just monitoring the tube. This would
require us to react to the TubeStateChanged to Offered and delay emitting it
until we've re-introspected the param. However, the current implementation
doesn't cater for it either. But, I don't think this use-case of monitoring a
tube getting offered AND needing to use the parameters for something actually
even exists, so I'm fine with fixing the needless reintrospection and
consequent indeterministic parameters update in the common case as outlined
above. Maybe put a FIXME in there explaining that parameters doesn't correctly
update if you're not the one offering the tube though.

Connection mapping functions: I guess you're correct about returning the hash
itself - after all, thanks to qt containers being implicitly shared this is
common practice in many other Qt/KDE APIs (in other places of TpQt4 too), and
allows using other functions in the hash api besides the lookup...

Naming changes ++, but please also change AllowedAddress to SpecifiedAddress
and reference the fact that supportsWithSpecifiedAddress also means "does
connection tracking work" in the supports function doc too.

More to come...

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.



More information about the telepathy-bugs mailing list