(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.
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!