Last Comment Bug 709862 - RIL: no more "incoming" events after the first
: RIL: no more "incoming" events after the first
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM: Device Interfaces (show other bugs)
: Trunk
: ARM Gonk (Firefox OS)
: -- normal (vote)
: mozilla11
Assigned To: Philipp von Weitershausen [:philikon]
:
Mentors:
Depends on:
Blocks: b2g-telephony
  Show dependency treegraph
 
Reported: 2011-12-12 10:34 PST by Philipp von Weitershausen [:philikon]
Modified: 2011-12-16 13:49 PST (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
v1 (2.85 KB, patch)
2011-12-12 13:29 PST, Philipp von Weitershausen [:philikon]
kyle: review+
Details | Diff | Splinter Review

Description Philipp von Weitershausen [:philikon] 2011-12-12 10:34:49 PST
Need to investigate what's happening here. Don't think this is the RIL since this was working fine before we added audio support. So likely culprit here is nsIAudioManager.
Comment 1 Philipp von Weitershausen [:philikon] 2011-12-12 11:24:06 PST
I misremembered: this isn't after making a call, this is after receiving a call. Investigating now.
Comment 2 Kyle Machulis [:kmachulis] [:qdot] 2011-12-12 11:28:04 PST
And the phone doesn't really get wedged, it just doesn't bring up the "incoming call" screen again. Everything looks fine on audio, ril, etc... Pretty sure it's just a gaia issue due to the extremely rapid implementation.
Comment 3 Philipp von Weitershausen [:philikon] 2011-12-12 11:31:32 PST
Yes, I just looked at the radio logcat and indeed that stuff works fine. So either Gaia is broken (I just quickly hacked the "incoming" event listener in, so that may be broken). In that case we can close this bug as RESO/WFM and defer to the Gaia Github issue tracker. Or the DOM stuff is broken somehow and doesn't send the event, in which case we leave this bug open to fix that.
Comment 4 Philipp von Weitershausen [:philikon] 2011-12-12 11:45:42 PST
Looks like a RIL issue. Here's the logcat for when I call it for the first time and then immediately hang up when it's ringing:

I/Gecko   ( 2587): -*- TelephonyWorker component: Received message: {"type":"callstatechange","callState":"incoming","callIndex":1,"number":"xxx","name":null}
I/ServiceManager( 2563): (B2G) Returning fake true permission: android.permission.MODIFY_AUDIO_SETTINGS from uid=0 pid=2587
I/ServiceManager( 2563): (B2G) Returning fake true permission: android.permission.MODIFY_AUDIO_SETTINGS from uid=0 pid=2587
V/AudioHardwareYamaha( 2563): AudioHardware::setMode(mode=1)
D/AudioHardwareInterface( 2563): setMode(RINGTONE)
E/AudioHardwareYamaha( 2563): ### wbamrStatus = 0###
I/Gecko   ( 2587): -*- TelephonyWorker component: Received message: {"type":"callstatechange","callState":"disconnected","callIndex":1,"number":"4155256471","name":null}
I/ServiceManager( 2563): (B2G) Returning fake true permission: android.permission.MODIFY_AUDIO_SETTINGS from uid=0 pid=2587
I/ServiceManager( 2563): (B2G) Returning fake true permission: android.permission.MODIFY_AUDIO_SETTINGS from uid=0 pid=2587
V/AudioHardwareYamaha( 2563): AudioHardware::setMode(mode=0)
D/AudioHardwareInterface( 2563): setMode(NORMAL)

And now when I call it the subsequent times:

E/AudioHardwareYamaha( 2563): ### wbamrStatus = 0###
E/AudioHardwareYamaha( 2563): ### wbamrStatus = 0###
I/Gecko   ( 2587): -*- TelephonyWorker component: Received message: {"type":"callstatechange","callState":"disconnected","callIndex":1,"number":"4155256471","name":null}
I/ServiceManager( 2563): (B2G) Returning fake true permission: android.permission.MODIFY_AUDIO_SETTINGS from uid=0 pid=2587
I/ServiceManager( 2563): (B2G) Returning fake true permission: android.permission.MODIFY_AUDIO_SETTINGS from uid=0 pid=2587
V/AudioHardwareYamaha( 2563): AudioHardware::setMode(mode=0)
W/AudioPolicyManager( 2563): setPhoneState() setting same state 0
E/AudioHardwareYamaha( 2563): ### wbamrStatus = 0###
E/AudioHardwareYamaha( 2563): ### wbamrStatus = 0###
I/Gecko   ( 2587): -*- TelephonyWorker component: Received message: {"type":"callstatechange","callState":"disconnected","callIndex":1,"number":"4155256471","name":null}
I/ServiceManager( 2563): (B2G) Returning fake true permission: android.permission.MODIFY_AUDIO_SETTINGS from uid=0 pid=2587
I/ServiceManager( 2563): (B2G) Returning fake true permission: android.permission.MODIFY_AUDIO_SETTINGS from uid=0 pid=2587
V/AudioHardwareYamaha( 2563): AudioHardware::setMode(mode=0)
W/AudioPolicyManager( 2563): setPhoneState() setting same state 0

The TelephonyWorker isn't receiving the "incoming" message, which is sent by the RIL worker. Will look for a problem in the RIL worker's call state management.
Comment 5 Philipp von Weitershausen [:philikon] 2011-12-12 13:29:17 PST
Created attachment 581031 [details] [diff] [review]
v1

Found the bug in the RIL. We weren't updating the call state correctly when a call disconnected. So when a new call came in, we still saw the old call and didn't send an event to the DOM thread. This fixes that.

I also noticed a bug in the UNSOLICITED_CALL_RING processing. The info data is only sent for CDMA, so let's disable that for now. (This is actually specified in ril.h, I must've overlooked it previously.)
Comment 6 Kyle Machulis [:kmachulis] [:qdot] 2011-12-12 14:27:44 PST
Comment on attachment 581031 [details] [diff] [review]
v1

Review of attachment 581031 [details] [diff] [review]:
-----------------------------------------------------------------

Might want to put a space after the // stuff unless there's a //XXX prefix rule I don't know about, otherwise looks good.
Comment 7 Philipp von Weitershausen [:philikon] 2011-12-12 14:36:12 PST
Thanks, Kyle! The //XXX stuff is kind of a convention, depending on who you ask... and at any rate we want to clean those out sooner rather than later anyway ;)

Pushed: https://hg.mozilla.org/mozilla-central/rev/03cd6329e4f9
Comment 8 Mounir Lamouri (:mounir) 2011-12-13 03:39:52 PST
(In reply to Philipp von Weitershausen [:philikon] from comment #7)
> Thanks, Kyle! The //XXX stuff is kind of a convention, depending on who you
> ask... and at any rate we want to clean those out sooner rather than later
> anyway ;)

No, XXX was a convention but AFAIK it's forbidden now. We should use:
// TODO: explanation message, bug NUMBER.

IOW, those kind of comments should come with a filed bug.

Note You need to log in before you can comment on or make changes to this bug.