Can anyone run telepathy-glib/examples/client/dbus-tubes/.libs/offerer without noticing the following issues: At sometimes, the offerer fails to create the channel- Breakpoint 1, channel_created (source=0x8060c18, result=0x805cc40, user_data=0x0) at offerer.c:155 155 GError *error = NULL; (gdb) n 159 TP_ACCOUNT_CHANNEL_REQUEST (source), result, NULL, &error); (gdb) 158 channel = tp_account_channel_request_create_and_handle_channel_finish ( (gdb) 160 if (channel == NULL) (gdb) 162 g_debug ("Failed to create channel: %s", error->message); (gdb) p error-> code domain message (gdb) p error->message $5 = (gchar *) 0x805e3e8 "We are supposed to handle only one channel" This is happening since the CD is calling HandleChannels of the temporary handler with more than 1 channel bundle. At other times it works fine. Its a hit and miss. Next, everytime I escape the above, I get- (process:18402): GLib-GObject-WARNING **: invalid cast from `TpChannel' to `TpDBusTubeChannel' (process:18402): tp-glib-CRITICAL **: tp_dbus_tube_channel_offer_async: assertion `TP_IS_DBUS_TUBE_CHANNEL (self)' failed for the channel the offerer creates to begin with. This is really wierd since- 1.) The TpDBusTubeChannel gets created! Breakpoint 1, channel_created (source=0x8060c18, result=0x805cc40, user_data=0x0) at offerer.c:155 155 GError *error = NULL; (gdb) n 159 TP_ACCOUNT_CHANNEL_REQUEST (source), result, NULL, &error); (gdb) 158 channel = tp_account_channel_request_create_and_handle_channel_finish ( (gdb) 160 if (channel == NULL) (gdb) 168 g_debug ("Channel created: %s", tp_proxy_get_object_path (channel)); (gdb) 170 tube = TP_DBUS_TUBE_CHANNEL (channel); (gdb) (process:18426): GLib-GObject-WARNING **: invalid cast from `TpChannel' to `TpDBusTubeChannel' Breakpoint 2, channel_created (source=0x8060c18, result=0x805cc40, user_data=0x0) at offerer.c:172 172 g_signal_connect (tube, "invalidated", (gdb) 175 tp_dbus_tube_channel_offer_async (tube, NULL, tube_offered, NULL); (gdb) p channel $7 = (TpChannel *) 0x805e8f8 See ^ , then; 2.) The created channel object has its object path published on the bus: (gdb) p tp_proxy_get_object_path (channel) $8 = ( const gchar *) 0x806cbf0 "/org/freedesktop/Telepathy/Connection/gabble/jabber/chandniverma2112_40gmail_2ecom_2fd999e8c3/DBusTubeChannel/13/966479375" 3.) And, the channel_type is also as expected! (gdb) p channel $10 = (TpChannel *) 0x805e8f8 (gdb) p ($10)->priv->channel_type $16 = 344 (gdb) p g_quark_to_string ( ($10)->priv->channel_type ) $17 = (const gchar *) 0x8056782 "org.freedesktop.Telepathy.Channel.Type.DBusTube" (gdb) l 170 tube = TP_DBUS_TUBE_CHANNEL (channel); 171 172 g_signal_connect (tube, "invalidated", 173 G_CALLBACK (tube_invalidated_cb), NULL); 174 175 tp_dbus_tube_channel_offer_async (tube, NULL, tube_offered, NULL); 176 } 177 178 int 179 main (int argc, (gdb) n (process:18426): tp-glib-CRITICAL **: tp_dbus_tube_channel_offer_async: assertion `TP_IS_DBUS_TUBE_CHANNEL (self)' failed 176 } (gdb) n g_simple_async_result_complete (simple=0x805cc40) at gsimpleasyncresult.c:778 778 g_main_context_pop_thread_default (simple->context); (gdb) c There is definitely some deeper set problem requiring a thorough investigation!
(In reply to comment #0) > Can anyone run telepathy-glib/examples/client/dbus-tubes/.libs/offerer > without noticing the following issues: > > > At sometimes, the offerer fails to create the channel- > > Breakpoint 1, channel_created (source=0x8060c18, result=0x805cc40, > user_data=0x0) at offerer.c:155 > 155 GError *error = NULL; > (gdb) n > 159 TP_ACCOUNT_CHANNEL_REQUEST (source), result, NULL, &error); > (gdb) > 158 channel = tp_account_channel_request_create_and_handle_channel_finish > ( > (gdb) > 160 if (channel == NULL) > (gdb) > 162 g_debug ("Failed to create channel: %s", error->message); > (gdb) p error-> > code domain message > (gdb) p error->message > $5 = (gchar *) 0x805e3e8 "We are supposed to handle only one cDrive-by:hannel" > > > This is happening since the CD is calling HandleChannels of the temporary > handler with more than 1 channel bundle. At other times it works fine. Its a > hit and miss. This is bug 47760. I worked around it in LibreOffice by always requesting the legacy Tubes channel for a contact before requesting the Tube channel, like this: http://pastie.org/4588272 . It will be resolved when bug 32612 is resolved. > Next, everytime I escape the above, I get- > > (process:18402): GLib-GObject-WARNING **: invalid cast from `TpChannel' to > `TpDBusTubeChannel' > > (process:18402): tp-glib-CRITICAL **: tp_dbus_tube_channel_offer_async: > assertion `TP_IS_DBUS_TUBE_CHANNEL (self)' failed > > > for the channel the offerer creates to begin with. This sounds like the wrong channel factory is being used, so only a plain TpChannel object is being created rather than a TpDBusTubeChannel subclass of that object.
Thanks! I could get TP_IFACE_CHANNEL_TYPE_DBUS_TUBE channels created with TpAutomaticClientFactory instead. The attached patches (including the optional temporary workaround you suggested) can be applied to make the example work presently.
Created attachment 66226 [details] [review] Use TpAutomaticClientFactory to request a channel instead of TpSimpleClientFactory
Created attachment 66227 [details] [review] Workaround to ignore the extraneous legacy tubes channel
Ok, I just noticed that bug 47760 was fixed an hour ago! So we don't need the temporary workaround now.
*** Bug 66222 has been marked as a duplicate of this bug. ***
Sorry, I didn't spot that there were patches to be reviewed here - please add "patch" to the Keywords if stuff needs review. The stream tube example has the same bug; I just fixed both.
Created attachment 85327 [details] [review] tube offerers: use an automatic client factory The TpSimpleClientFactory base class doesn't use channel-type-specific subclasses, which these examples rely on. --- This is basically an independent implementation of Chandni's Attachment #66226 [details], but with the cast the other way round. I prefer my version, because on the 'next' (Telepathy 1.0) branch, tp_automatic_client_factory_new() returns a pointer to TpClientFactory (the new name for TpSimpleClientFactory), in the same way that e.g. gtk_button_new() returns a GtkWidget.
Created attachment 85328 [details] [review] tube offering examples: don't dump core on invalid arguments --- While I'm there.
Created attachment 85329 [details] [review] tube examples: use g_message(), not g_debug() g_debug() is invisible unless you set G_MESSAGES_DEBUG, which is unhelpful when smoke-testing example code.
Created attachment 85330 [details] [review] TpAccountManager: fix documentation of how many accounts are prepared The documentation had fewer guarantees than the implementation: since 0.16, we've delayed notification of new accounts until they're prepared. --- While I was looking at this, I tidied up quite a lot of documentation, which will follow - I wanted to make sure we had a consistent story for what a "normal" application should do. That turns out to be "call tp_account_manager_dup() and you'll get a TpAutomaticClientFactory".
Created attachment 85331 [details] [review] TpAccountManager: tighten up guarantees about factories It always has a factory, and if none is specified at construction, the default is to use a TpAutomaticClientFactory. If we document that, then client code won't need to mess about with tp_account_manager_set_default() unless it has special requirements, namely: either it has a factory of its own with more features and/or subclasses, or a TpSimpleClientFactory is sufficient and low-overhead is important.
Created attachment 85333 [details] [review] TpAccount: document interaction with factories a bit better
Created attachment 85334 [details] [review] TpChannelDispatchOperation, TpChannelRequest: remove boilerplate This text made sense before we implemented more stuff, but is obsolete now.
Created attachment 85335 [details] [review] TpConnection: remove woefully outdated feature list TpConnection does much, much more than a simple D-Bus proxy now.
Created attachment 85336 [details] [review] TpConnection: connections should come from accounts
Created attachment 85337 [details] [review] TpSimpleClientFactory: suggest getting objects from "larger" objects
Created attachment 85338 [details] [review] TpCDO, TpChannelRequest: mention that they should come from "larger" objects
Created attachment 85339 [details] [review] TpCDO, TpChannelRequest: always has a factory
Created attachment 85340 [details] [review] TpConnection: describe factory behaviour
Created attachment 85341 [details] [review] TpConnection: don't recommend that applications call Connect() For most applications, the rule is "the account manager does that".
Created attachment 85342 [details] [review] TpSimpleClientFactory: document "connection must be ours" tp_simple_client_factory_ensure_channel and tp_simple_client_factory_ensure_contact already enforced that via a check, but didn't document it. tp_simple_client_factory_upgrade_contacts_async didn't previously either document or enforce it, and strictly speaking there's no reason why it shouldn't work, so I'm using a warning instead of a critical-and-return - but it probably indicates an error, so I think it should warn.
Created attachment 85343 [details] [review] TpSimpleClientFactory: drop doc-comments for private functions
Created attachment 85344 [details] [review] extended-client: comment that we're going behind MC's back
Created attachment 85345 [details] [review] Python text handler: ensure that we get a Tp.AutomaticClientFactory We want a Tp.TextChannel, not just a Tp.Channel, and the easiest way to do that is to use Tp.AccountManager.dup() (which provides a Tp.AutomaticClientFactory). Tp.SimpleHandler.new() is deprecated, for approximately this reason.
Created attachment 85346 [details] [review] C text handler: comment why the subclass / feature-prep works
(In reply to comment #24) > Created attachment 85344 [details] [review] > extended-client: comment that we're going behind MC's back See also Bug #51687. That's all the patches for now...
Comment on attachment 85327 [details] [review] tube offerers: use an automatic client factory Review of attachment 85327 [details] [review]: ----------------------------------------------------------------- ++
Comment on attachment 85328 [details] [review] tube offering examples: don't dump core on invalid arguments Review of attachment 85328 [details] [review]: ----------------------------------------------------------------- ++
Comment on attachment 85329 [details] [review] tube examples: use g_message(), not g_debug() Review of attachment 85329 [details] [review]: ----------------------------------------------------------------- ++
Comment on attachment 85330 [details] [review] TpAccountManager: fix documentation of how many accounts are prepared Review of attachment 85330 [details] [review]: ----------------------------------------------------------------- ++
Comment on attachment 85333 [details] [review] TpAccount: document interaction with factories a bit better Review of attachment 85333 [details] [review]: ----------------------------------------------------------------- ++
Comment on attachment 85334 [details] [review] TpChannelDispatchOperation, TpChannelRequest: remove boilerplate Review of attachment 85334 [details] [review]: ----------------------------------------------------------------- ++
Comment on attachment 85335 [details] [review] TpConnection: remove woefully outdated feature list Review of attachment 85335 [details] [review]: ----------------------------------------------------------------- I just learned new the word 'woefully' :) ++
Comment on attachment 85336 [details] [review] TpConnection: connections should come from accounts Review of attachment 85336 [details] [review]: ----------------------------------------------------------------- ++
Comment on attachment 85337 [details] [review] TpSimpleClientFactory: suggest getting objects from "larger" objects Review of attachment 85337 [details] [review]: ----------------------------------------------------------------- ++
Comment on attachment 85338 [details] [review] TpCDO, TpChannelRequest: mention that they should come from "larger" objects Review of attachment 85338 [details] [review]: ----------------------------------------------------------------- ++
Comment on attachment 85339 [details] [review] TpCDO, TpChannelRequest: always has a factory Review of attachment 85339 [details] [review]: ----------------------------------------------------------------- ++
Comment on attachment 85340 [details] [review] TpConnection: describe factory behaviour Review of attachment 85340 [details] [review]: ----------------------------------------------------------------- ++
Comment on attachment 85341 [details] [review] TpConnection: don't recommend that applications call Connect() Review of attachment 85341 [details] [review]: ----------------------------------------------------------------- ++
Comment on attachment 85342 [details] [review] TpSimpleClientFactory: document "connection must be ours" Review of attachment 85342 [details] [review]: ----------------------------------------------------------------- ++
Comment on attachment 85343 [details] [review] TpSimpleClientFactory: drop doc-comments for private functions Review of attachment 85343 [details] [review]: ----------------------------------------------------------------- ++
Comment on attachment 85344 [details] [review] extended-client: comment that we're going behind MC's back Review of attachment 85344 [details] [review]: ----------------------------------------------------------------- ++
Comment on attachment 85345 [details] [review] Python text handler: ensure that we get a Tp.AutomaticClientFactory Review of attachment 85345 [details] [review]: ----------------------------------------------------------------- ++
Comment on attachment 85346 [details] [review] C text handler: comment why the subclass / feature-prep works Review of attachment 85346 [details] [review]: ----------------------------------------------------------------- ++
Fixed in git for 0.21.2, thanks
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.