In bug 876396, we add a console command, 'modem tech', to switch radio technology. So that we can use this command to switch emulator to cdma mode and run test cases. But when I try to write a marionette tests, I meet some problems as below: - Currently the preferred Network Type is GSM/WCDMA, so it is failed to switch to cdma by using 'modem tech' console command. To fix this, maybe we could consider to make 'modem tech' can also support switching preferred network type. - There is no way to know the radio switching process is finished. If we start test before it is ready, we may got some unexpected timing issue. My idea is using voice registration event [1], when radio technology changes, qemu should report voice registration change with the current radio technology [2]. But this mechanism seems not work properly now. [1] https://github.com/mozilla-b2g/platform_hardware_ril/blob/master/include/telephony/ril.h#L3354 [2] https://github.com/mozilla-b2g/platform_hardware_ril/blob/master/include/telephony/ril.h#L1248

Working branch: https://github.com/EdgarChen/platform_external_qemu/commits/bug_891707 https://github.com/EdgarChen/platform_hardware_ril/commits/bug_891707

(In reply to Edgar Chen [:edgar][:echen] from comment #0) > - Currently the preferred Network Type is GSM/WCDMA, so it is failed to > switch to cdma by using 'modem tech' console command. > To fix this, maybe we could consider to make 'modem tech' can also support > switching preferred network type. File a separated bug for this.

(In reply to Edgar Chen [:edgar][:echen] from comment #4) > (In reply to Edgar Chen [:edgar][:echen] from comment #0) > > - Currently the preferred Network Type is GSM/WCDMA, so it is failed to > > switch to cdma by using 'modem tech' console command. > > To fix this, maybe we could consider to make 'modem tech' can also support > > switching preferred network type. > File a separated bug for this. Bug 897015. Thanks

Do you think these PRs are ready? Looks good for me.

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #6) > Do you think these PRs are ready? Looks good for me. It is almost ready, but I would like to do more test before request review. Thanks.

(In reply to Edgar Chen [:edgar][:echen] from comment #7) > (In reply to Vicamo Yang [:vicamo][:vyang] from comment #6) > > Do you think these PRs are ready? Looks good for me. > > It is almost ready, but I would like to do more test before request review. > Thanks. I found voice registration does not update correctly if the switch event is triggered by setting preferred network type from gecko.

(In reply to Edgar Chen [:edgar][:echen] from comment #8) > (In reply to Edgar Chen [:edgar][:echen] from comment #7) > > (In reply to Vicamo Yang [:vicamo][:vyang] from comment #6) > > > Do you think these PRs are ready? Looks good for me. > > > > It is almost ready, but I would like to do more test before request review. > > Thanks. > > I found voice registration does not update correctly if the switch event is > triggered by setting preferred network type from gecko. Provide the solution in part1 patch of qemu pull request.

Comment on attachment 8617776 [details] [diff] [review] Part 1: Waiting for |UNSOLICITED_RESPONSE_VOICE_NETWORK_STATE_CHANGED| when changing modem tech Review of attachment 8617776 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/telephony/test/marionette/head.js @@ +281,5 @@ > */ > function changeModemTech(aTech, aPreferredMask) { > + let mobileConn = navigator.mozMobileConnections[0]; > + > + function checkTech(aMobileConnection) { Use closure for both |mobileConn| and |aTech| or passing them as function parameters. Let's apply the same way for these two variables. @@ +282,5 @@ > function changeModemTech(aTech, aPreferredMask) { > + let mobileConn = navigator.mozMobileConnections[0]; > + > + function checkTech(aMobileConnection) { > + switch(mobileConn.voice.type) { not |mobileConn|, should be |aMobileConnection| @@ +314,5 @@ > + return false; > + } > + } > + > + let promise1 = checkTech(mobileConn) ? Promise.resolve() The naming of checkTech is not good enough. By reading the line let promise1 = checkTech(mobileConn) ... You have no idea what will be done in the function. The function mixed two purposes, one is converting voice type to tech, another one is comparing the tech. I'll suggest something like function voiceTypeToTech(aType) { case "...": return "gsm"; ... } function isCompleted() { return aTech === voiceTypeToTech(mobileConn.voice.type); }

Comment on attachment 8622180 [details] [diff] [review] Part 1: Waiting for |UNSOLICITED_RESPONSE_VOICE_NETWORK_STATE_CHANGED| when changing modem tech (V2). r=aknow Review of attachment 8622180 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/telephony/test/marionette/head.js @@ +277,5 @@ > } > > /** > + * @param aVoiceType > + * The voice type of a mobileConnection, which can be obtained from This line is too long.

Hi Edgar, Can you help review these patches? They are made from simply rebasing previous works.

Since you are working on this bug now, I change the assignee to you. Thank you, Ben.

Comment on attachment 8624054 [details] [external/qemu] pull request 150 (for Emulator-KK) The link of pull request seems incorrect.

Comment on attachment 8628143 [details] [external/qemu] pull request 149 (for Emulator-ICS) Sorry for being late. Per comments on github, could you please file a follow-up bug to find a better solution for the registration state. Thank you.

(In reply to Edgar Chen [:edgar][:echen] from comment #23) > Comment on attachment 8628142 [details] > [hardware/ril] pull request 60 (for Emulator-ICS) > > Sorry for being late. Per comments on github, could you please file a > follow-up bug to find a better solution for the registration state. Thank > you. ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ (Please ignore my previous comment, It should belong to the pull request of external/qemu. :P) r=me with comments on github addressed. Thank you.

Link updated :P

Since there are some defects in current gecko, some of the test will fail after applying other patches for this issue. As a result, I suggest first disable some test here, and re-enable them in Bug 1190274.

Comment on attachment 8642270 [details] [diff] [review] Part 3: Disable some tests Review of attachment 8642270 [details] [diff] [review]: ----------------------------------------------------------------- r=me with comment addressed, thank you. ::: dom/mobileconnection/tests/marionette/test_mobile_voice_state.js @@ -125,5 @@ > > startTestCommon(function() { > log("Test initial voice connection info"); > > - verifyVoiceInfo(INITIAL_STATES); Please also remove |INITIAL_STATES|.

b2g-ril_v7: https://github.com/mozilla-b2g/platform_hardware_ril/commit/2d58f4b9206b50b8fda0d5036da6f0c62608db7c b2g-ics: https://github.com/mozilla-b2g/platform_external_qemu/commit/d70e4bfdcb65e7514de0f9315b74aea1c811678d

https://hg.mozilla.org/integration/b2g-inbound/rev/664df831252b https://hg.mozilla.org/integration/b2g-inbound/rev/bbaf3960224b https://hg.mozilla.org/integration/b2g-inbound/rev/b32bac59574b

https://hg.mozilla.org/mozilla-central/rev/664df831252b https://hg.mozilla.org/mozilla-central/rev/bbaf3960224b https://hg.mozilla.org/mozilla-central/rev/b32bac59574b

b2g-kitkat: https://github.com/mozilla-b2g/platform_hardware_ril/commit/147b71ad0e03b30318baae4bf07981aa0e96ca1d https://github.com/mozilla-b2g/platform_hardware_ril/commit/3453472769d6e9b43c84f98b56f8c63afe2b129e https://github.com/mozilla-b2g/platform_external_qemu/commit/96f78964a9a317a6c8b494ab71e10e0276d02490