Created attachment 68639 [details] DBus log Gabble's implementation of the hangup virtual method sets the state to TP_CALL_STATE_ENDED. See call_session_terminated_cb in telepathy-gabble. This causes an extra CallStateChanged signal to be emitted on the bus. See attached log.
Created attachment 68640 [details] [review] base-call-channel: Set state to ENDED in hangup only if not set Against telepathy-glib-0.20 branch. (I am wondering why tp_base_call_channel_set_state does not check and ignore transitions where the current and destination states are the same.)
Created attachment 68678 [details] [review] base-call-channel: Ignore spurious transitions Against telepathy-glib-0.20 branch. Lets make tp_base_call_channel_set_state check and ignore this, as hinted earlier. -- However, this causes the following failure in tp-gabble's test suite: Traceback (most recent call last): File "/home/rishi/devel/telepathy/git/telepathy-gabble/tests/twisted/gabbletest.py", line 634, in exec_test_deferred fun(queue, bus, conns[0], streams[0]) File "/home/rishi/devel/telepathy/git/telepathy-gabble/tests/twisted/jingle/call_helper.py", line 809, in run_call_test test.run() File "/home/rishi/devel/telepathy/git/telepathy-gabble/tests/twisted/jingle/call_helper.py", line 802, in run self.pickup() File "./jingle/call-basics.py", line 116, in pickup CallTest.pickup(self) File "/home/rishi/devel/telepathy/git/telepathy-gabble/tests/twisted/jingle/call_helper.py", line 644, in pickup signal = self.q.expect('dbus-signal', signal='CallStateChanged') File "/home/rishi/devel/telepathy/git/telepathy-gabble/tests/twisted/servicetest.py", line 173, in expect event = self.wait([pattern.subqueue]) File "/home/rishi/devel/telepathy/git/telepathy-gabble/tests/twisted/servicetest.py", line 307, in wait raise TimeoutError TimeoutError FAIL: jingle/call-basics.py
In tp_base_call_channel_set_state() the 2 if conditions at the end actually check if self->priv->state != old_state, which indicate there were a reason for not early return. It's probably me who wrote that, but I rekon I don't remember the reason. A failing unit test seems to also indicate that there is a reason for not early returning. Olivier may know, I'll ask him.
Ok, reading the unit test, I actually see the reason (good think this is all well tested). So in tp_base_call_channel_set_ringing() we call tp_base_call_channel_set_state() not to change the state, but to add the RINGING flag. This actually makes sense, the state of the call did not actually change, but the UI is reporting that it is now playing ringing sound.
Created attachment 68689 [details] [review] base-call-channel: Ignore transitions with same flags and state Compare the flags too and special case TP_CALL_FLAG_LOCALLY_RINGING.
Created attachment 68700 [details] [review] base-call-channel: Ignore transitions with the same state
Created attachment 68701 [details] [review] base-call-channel: Don't call set_state from set_ringing and set_queued
Created attachment 68707 [details] [review] base-call-channel: Ignore transitions with the same state
Looks good, ship it :) Thanks.
Pushed to both master and telepathy-glib-0.20 branches. commit 48a6895f21fd3a3ef996e3e18ded4ed8a8f9f576 Author: Debarshi Ray <rishi@gnu.org> Date: Wed Oct 17 14:48:11 2012 +0200 base-call-channel: Don't call set_state from set_ringing and set_queued ... because they only change the flags and not the state. Instead emit CallStateChanged directly. Fixes: https://bugs.freedesktop.org/56044 commit dfca38a93e2c504c1a4643eec291c4ddb4368f4f Author: Debarshi Ray <rishi@gnu.org> Date: Wed Oct 17 10:50:24 2012 +0200 base-call-channel: Ignore transitions with the same state Fixes: https://bugs.freedesktop.org/56044
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.