Bug 27871 - TpSimpleObserver - observe channels without writing a TpBaseClient subclass
Summary: TpSimpleObserver - observe channels without writing a TpBaseClient subclass
Status: RESOLVED FIXED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: tp-glib (show other bugs)
Version: unspecified
Hardware: Other All
: medium enhancement
Assignee: Guillaume Desmottes
QA Contact: Telepathy bugs list
URL: http://git.collabora.co.uk/?p=user/ca...
Whiteboard: review+
Keywords: patch
Depends on: 25236
Blocks: 24214 27881
  Show dependency treegraph
 
Reported: 2010-04-28 08:42 UTC by Simon McVittie
Modified: 2010-04-29 07:29 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments

Description Simon McVittie 2010-04-28 08:42:17 UTC
Cloned from Bug #25236. Executive summary:

* TpBaseClient provides flexible support for being an Observer by subclassing
* ... but subclassing in C is hard and most observers are expected to be somewhat simple
* so, write a subclass that just needs a callback and some user_data

Guillaume is already working on it.
Comment 1 Guillaume Desmottes 2010-04-29 00:23:25 UTC
http://git.collabora.co.uk/?p=user/cassidy/telepathy-glib;a=shortlog;h=refs/heads/simple-observer
should be ready for review.

It includes a example call observer. I can split it to a different branch for bug #24214 if needed but I think that should be easy enough to review.
Comment 2 Simon McVittie 2010-04-29 05:07:12 UTC
+ * |[
+ * client = tp_simple_observer_new (dbus, TRUE, "MyObserver", FALSE,
+ *    my_observe_channels, user_data);

I'd like this comment to include a stub implementation of my_observe_channels:

static void
my_observe_channels (TpSimpleObserver *self,
    TpAccount *account,
    TpConnection *connection,
    GList *channels,
    TpChannelDispatchOperation *dispatch_operation,
    GList *requests,
    TpObserveChannelsContext *context,
    gpointer user_data)
{
  /* ... do something useful with the channels here ... */

  tp_observe_channels_context_accept (context);
}

I'd also prefer a more realistic filter: matching every TEXT channel is likely to be a mistake (unless your observer is written very carefully), and matching CONTACT TEXT channels would be more normal.

> + * but are not garanteed to be prepared.

"guaranteed"

> + * @channels: a #GPtrArray of #TpChannel having all %TP_CHANNEL_FEATURE_CORE
> + * prepared

"#GList of #TpChannel, all having"

> + * @user_data: arbitrary user-supplied data passed to the callback

This *is* the callback. "passed to tp_simple_observer_new()"

> +      case PROP_OBSERV_IMPL:

I'd prefer this property to be called "callback".

> +tp_simple_observer_new (TpDBusDaemon *dbus,

Shouldn't this take a GDestroyNotify for the user_data, too?

> + * Returns: a new #TpSimpleObserver

"Returns: (type Tp.SimpleObserver): a new #TpSimpleObserver" would be better, I think; we're deliberately "getting the type wrong" for C's benefit, but g-i (and documentation) can benefit from knowing more specifically.

It would be good to update telepathy-doc's example to use this, once it's in.
Comment 3 Guillaume Desmottes 2010-04-29 05:58:04 UTC
(In reply to comment #2)
> + * |[
> + * client = tp_simple_observer_new (dbus, TRUE, "MyObserver", FALSE,
> + *    my_observe_channels, user_data);
> 
> I'd like this comment to include a stub implementation of my_observe_channels:
> 
> static void
> my_observe_channels (TpSimpleObserver *self,
>     TpAccount *account,
>     TpConnection *connection,
>     GList *channels,
>     TpChannelDispatchOperation *dispatch_operation,
>     GList *requests,
>     TpObserveChannelsContext *context,
>     gpointer user_data)
> {
>   /* ... do something useful with the channels here ... */
> 
>   tp_observe_channels_context_accept (context);
> }

added (without the /* */, C comments can't be nested).

> I'd also prefer a more realistic filter: matching every TEXT channel is likely
> to be a mistake (unless your observer is written very carefully), and matching
> CONTACT TEXT channels would be more normal.

fixed.

> > + * but are not garanteed to be prepared.
> 
> "guaranteed"

fixed (one day I'll be able to spell this word).

> > + * @channels: a #GPtrArray of #TpChannel having all %TP_CHANNEL_FEATURE_CORE
> > + * prepared
> 
> "#GList of #TpChannel, all having"
Fixed.

> > + * @user_data: arbitrary user-supplied data passed to the callback
> 
> This *is* the callback. "passed to tp_simple_observer_new()"

Fixed.

> > +      case PROP_OBSERV_IMPL:
> 
> I'd prefer this property to be called "callback".

done.

> > +tp_simple_observer_new (TpDBusDaemon *dbus,
> 
> Shouldn't this take a GDestroyNotify for the user_data, too?

added.

> > + * Returns: a new #TpSimpleObserver
> 
> "Returns: (type Tp.SimpleObserver): a new #TpSimpleObserver" would be better, I
> think; we're deliberately "getting the type wrong" for C's benefit, but g-i
> (and documentation) can benefit from knowing more specifically.

done.

> It would be good to update telepathy-doc's example to use this, once it's in.

I opened bug #27888
Comment 4 Simon McVittie 2010-04-29 06:26:49 UTC
Looks good to me!
Comment 5 Guillaume Desmottes 2010-04-29 07:29:18 UTC
Merged to master! Will be in 0.11.5.


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.