Bug 36334 - high-level interface for Conn.I.Balance
Summary: high-level interface for Conn.I.Balance
Status: RESOLVED FIXED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: tp-glib (show other bugs)
Version: git master
Hardware: Other All
: medium normal
Assignee: Telepathy bugs list
QA Contact: Telepathy bugs list
URL: http://cgit.collabora.com/git/user/ca...
Whiteboard:
Keywords: patch
Depends on:
Blocks: 37358
  Show dependency treegraph
 
Reported: 2011-04-17 20:12 UTC by Danielle Madeley
Modified: 2011-05-30 00:44 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments

Description Danielle Madeley 2011-04-17 20:12:51 UTC
cassidy writes:

> Can you please open a tp-glib bug asking for high-level API for Balance and
> link it in the code? I try to keep track of places where we are using the raw
> API.
Comment 2 Guillaume Desmottes 2011-05-12 01:00:46 UTC
We usually put the test CM in tests/lib and only the actual test in tests/dbus/.

Any chance to convince you to wait that my refactoring in bug #31583 is merged and rebase your branch on top of it? It would make your code easier and would save me another conflict hell...
Comment 3 Guillaume Desmottes 2011-05-12 03:47:34 UTC
I just merged my refactoring so I suggest you to rebase your branch on top of master and use the new feature API.
Comment 4 Emilio Pozuelo Monfort 2011-05-17 10:04:55 UTC
I've rebased it here:
http://cgit.collabora.com/git/user/pochu/telepathy-glib.git/log/?h=balance-feature-36334

A review would be very welcome
Comment 5 Danielle Madeley 2011-05-17 15:30:17 UTC
-      goto finally;
+      g_simple_async_result_set_from_error (result, in_error);
     }

Immediately noticed you've removed the goto finally; here. You don't want to do this because @props will be NULL.
Comment 6 Emilio Pozuelo Monfort 2011-05-19 03:15:34 UTC
(In reply to comment #5)
> -      goto finally;
> +      g_simple_async_result_set_from_error (result, in_error);
>      }
> 
> Immediately noticed you've removed the goto finally; here. You don't want to do
> this because @props will be NULL.

Right, amended (added a return; ).

I wonder if we need the g_object_freeze_notify() / g_object_thaw_notify() around tp_connection_unpack_balance(), given that tp_connection_unpack_balance() calls them itself. Is that so notify::balance-uri is emitted together with the other notify signals? Is it a problem if they aren't? If so, we could avoid that by adding an optional const gchar *balance_uri parameter to tp_connection_unpack_balance(). That would keep the code somewhat cleaner IMHO.
Comment 7 Danielle Madeley 2011-05-24 23:59:34 UTC
I don't see a return. Did you forget to push?

The freeze/thaw is so that all of the property notifications are emitted together. I don't know if this is needed, but it felt nice to have.

Since some combination of the 3 properties can update together, binding to 3 properties, and thus getting 3 callbacks, seems suboptimal. Perhaps we also need a ::balance-changed(balance, scale, currency) signal.
Comment 8 Emilio Pozuelo Monfort 2011-05-25 01:19:00 UTC
Oops, pushed now.
Comment 9 Danielle Madeley 2011-05-25 06:11:14 UTC
-      goto finally;
+      g_simple_async_result_set_from_error (result, in_error);
+      return;

Never calls g_simple_async_result_complete()... goto finally; would be better.

Who is going to add the signal, you or I?
Comment 10 Guillaume Desmottes 2011-05-27 05:06:47 UTC
Finally I did it as I'd really like to get this in for the next release:
http://cgit.collabora.com/git/user/cassidy/telepathy-glib/log/?h=balance-feature-36334
Comment 11 Danielle Madeley 2011-05-29 18:25:22 UTC
(In reply to comment #10)
> Finally I did it as I'd really like to get this in for the next release:
> http://cgit.collabora.com/git/user/cassidy/telepathy-glib/log/?h=balance-feature-36334

+   * @balance_scale: the value of the #TpConnection:balance-currency property
+   * @balance_currency: the value of the #TpConnection:balance-scale property

Backwards!
Comment 12 Guillaume Desmottes 2011-05-30 00:44:53 UTC
Fixed and merged to master; will be in 0.15.1


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.