Last Comment Bug 740238 - B2G SMS: Emulator-to-emulator SMS not working
: B2G SMS: Emulator-to-emulator SMS not working
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM: Device Interfaces (show other bugs)
: Trunk
: ARM Gonk (Firefox OS)
: -- normal (vote)
: mozilla16
Assigned To: Vicamo Yang [:vicamo][:vyang]
:
Mentors:
Depends on: 744306
Blocks: b2g-sms 736701
  Show dependency treegraph
 
Reported: 2012-03-28 19:42 PDT by Vicamo Yang [:vicamo][:vyang]
Modified: 2012-06-21 04:03 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-
-


Attachments
use null SMSC by default (3.26 KB, patch)
2012-04-03 03:12 PDT, Vicamo Yang [:vicamo][:vyang]
no flags Details | Diff | Splinter Review
use null SMSC by default : V2 (3.23 KB, patch)
2012-06-13 00:03 PDT, Vicamo Yang [:vicamo][:vyang]
philipp: review+
Details | Diff | Splinter Review
use null SMSC by default : V3 (3.32 KB, patch)
2012-06-20 04:13 PDT, Vicamo Yang [:vicamo][:vyang]
philipp: review+
Details | Diff | Splinter Review

Description Vicamo Yang [:vicamo][:vyang] 2012-03-28 19:42:20 PDT
See github issue https://github.com/andreasgal/B2G/issues/248

> Attempting to send an SMS from one emulator to another silently fails. The
> REQUEST_GET_SMSC_ADDRESS (parcel type 100) request isn't supported by the
> emulator (it returns with ERROR_REQUEST_NOT_SUPPORTED = 6). So far we've
> assumed that you need the SMSC to send an SMS, bug inter-emulator SMS works
> similar to inter-emulator telephony; no SMSC should be required. See
> http://developer.android.com/guide/developing/devices/emulator.html#calling.
Comment 1 Vicamo Yang [:vicamo][:vyang] 2012-03-29 01:06:31 PDT
Is anyone working with qemu? I can't drag screen at all!
Comment 2 Vicamo Yang [:vicamo][:vyang] 2012-03-29 01:11:27 PDT
(In reply to Vicamo Yang [:vicamo] from comment #0)
> So far we've assumed that you need the SMSC to send an SMS, bug
> inter-emulator SMS works similar to inter-emulator telephony; no SMSC should be
> required. See http://developer.android.com/guide/developing/devices/emulator.html#calling.

Android only sets SMSC address when a TP-RP is available (see also bug 736701 for detailed information about TP-RP) no matter its in emulator or in physical phone. I suggest we follow this to prevent further compatibility issues.
Comment 3 Vicamo Yang [:vicamo][:vyang] 2012-04-01 20:49:48 PDT
(In reply to Vicamo Yang [:vicamo] from comment #1)
> Is anyone working with qemu? I can't drag screen at all!

Somehow m-c synced today with tip=3e46009daea3 works. I'll continue looking into this issue.
Comment 4 Vicamo Yang [:vicamo][:vyang] 2012-04-03 02:03:07 PDT
Let this issue blocks bug 736701, which will introduce additional SMSC parameter to sendText(). In the end, I'll remove SMSC dependency in this issue, and add an optional SMSC parameter back in bug 736701.
Comment 5 Vicamo Yang [:vicamo][:vyang] 2012-04-03 03:12:22 PDT
Created attachment 611760 [details] [diff] [review]
use null SMSC by default
Comment 6 Vicamo Yang [:vicamo][:vyang] 2012-04-04 22:39:42 PDT
(In reply to Vicamo Yang [:vicamo] from comment #4)
> Let this issue blocks bug 736701, which will introduce additional SMSC
> parameter to sendText(). In the end, I'll remove SMSC dependency in this
> issue, and add an optional SMSC parameter back in bug 736701.

It seems SMSC fetching is a work-around for Akami and I don't have one at hand. I'll try to reach others in Taipei if possible, but help from other place is also appreciated.
Comment 7 Vicamo Yang [:vicamo][:vyang] 2012-04-05 04:39:07 PDT
I had an Akami built with latest gonk & m-c & gaia (with pull request
1130 merged) now, but the screenlock just can't be removed with dragging.
Comment 8 Vicamo Yang [:vicamo][:vyang] 2012-04-06 00:51:57 PDT
(In reply to Vicamo Yang [:vicamo] from comment #7)
> I had an Akami built with latest gonk & m-c & gaia (with pull request
> 1130 merged) now, but the screenlock just can't be removed with dragging.

With bug 741038 attachment 611119 [details] [diff] [review], the screenlock can then be unlock. But there seems to be something wrong with Akami's RIL stack. Full log is available in http://pastebin.com/bTziLNhr . The log is captured with gecko built WITHOUT attachment 611760 [details] [diff] [review]. 

Line 124: rild crash at boot.

Line 663: Parcel (size 8): 1,0,0,0,10,4,0,0

Line 666: got UNSOLICITED_RIL_CONNECTED(1034), but it is actually UNSOLICITED_VOICE_RADIO_TECH_CHANGED(also 1034) for Akami. Since all payload is already consumed, parcel guard throws an exception in next read attempt.

Line 1035: got 'sendSMS' dom message from RadioInterfaceLayer.

Line 1036: sending REQUEST_GET_SMSC_ADDRESS(100)

Line 1043: got response of REQUEST_GET_SMSC_ADDRESS(100) with error ERROR_GENERIC_FAILURE(2)

The Akami I had can't be rolled back to official rom, can't figure out what's its normal workflow. I think I should mark this issue blocked by bug 726233 (B2G RIL: akami support).
Comment 9 Dietrich Ayala (:dietrich) 2012-06-11 21:08:18 PDT
Would be nice for testing, but we wouldn't hold the release for this, blocking-.
Comment 10 Vicamo Yang [:vicamo][:vyang] 2012-06-13 00:03:12 PDT
Created attachment 632582 [details] [diff] [review]
use null SMSC by default : V2

Rebase previous work. Verified on both qemu & galaxy-s2.
Comment 11 Vicamo Yang [:vicamo][:vyang] 2012-06-13 00:11:06 PDT
(In reply to Vicamo Yang [:vicamo] from comment #10)
> Created attachment 632582 [details] [diff] [review]
> use null SMSC by default : V2
> 
> Rebase previous work. Verified on both qemu & galaxy-s2.

Hi Philipp,

Since there seems no active Akami support plan and Akami RIL support is still broken by now, I suggest we may land this first and allow emulator-based automated SMS tests to go on. I've also removed dependency to bug 745692. If you don't feel like this way, please also give suggestions.
Comment 12 Philipp von Weitershausen [:philikon] 2012-06-15 15:30:00 PDT
Comment on attachment 632582 [details] [diff] [review]
use null SMSC by default : V2

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

Even on the Akami this code should not have been necessary anymore, so yes, let's remove it. Please make sure SMS do work on the Otoro, though.\

r=me with nits below addressed.

::: dom/system/gonk/ril_worker.js
@@ +1651,5 @@
>    /**
>     * Get the Short Message Service Center address.
>     *
>     * @param pendingSMS
>     *        Object containing the parameters of an SMS waiting to be sent.

Please update this comment as well.

@@ +3163,5 @@
>  RIL[REQUEST_DEVICE_IDENTITY] = null;
>  RIL[REQUEST_EXIT_EMERGENCY_CALLBACK_MODE] = null;
>  RIL[REQUEST_GET_SMSC_ADDRESS] = function REQUEST_GET_SMSC_ADDRESS(length, options) {
>    if (options.rilRequestError) {
> +    //TODO: notify main thread if we fail retrieving the SMSC

Please file a bug for this and mention the bug number here.
Comment 13 Vicamo Yang [:vicamo][:vyang] 2012-06-18 19:15:27 PDT
(In reply to Philipp von Weitershausen [:philikon] from comment #12)
> Comment on attachment 632582 [details] [diff] [review]
> use null SMSC by default : V2
> 
> Review of attachment 632582 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Even on the Akami this code should not have been necessary anymore, so yes,
> let's remove it. Please make sure SMS do work on the Otoro, though.\

Ok, I've borrowed one from Steve. I'll update this patch if any result got.

> r=me with nits below addressed.
> 
> ::: dom/system/gonk/ril_worker.js
> @@ +1651,5 @@
> >    /**
> >     * Get the Short Message Service Center address.
> >     *
> >     * @param pendingSMS
> >     *        Object containing the parameters of an SMS waiting to be sent.
> 
> Please update this comment as well.

I will.

> @@ +3163,5 @@
> >  RIL[REQUEST_DEVICE_IDENTITY] = null;
> >  RIL[REQUEST_EXIT_EMERGENCY_CALLBACK_MODE] = null;
> >  RIL[REQUEST_GET_SMSC_ADDRESS] = function REQUEST_GET_SMSC_ADDRESS(length, options) {
> >    if (options.rilRequestError) {
> > +    //TODO: notify main thread if we fail retrieving the SMSC
> 
> Please file a bug for this and mention the bug number here.

SMSC address is actually never referenced in RadioInterfaceLayer or other code in ril_worker. By unlinking getSMSCAddress and sendSMS with this patch, all SMSC related codes will be orphaned and no need to notify anyone on failure. I think we can just remove this TODO.
Comment 14 Vicamo Yang [:vicamo][:vyang] 2012-06-20 04:13:16 PDT
Created attachment 634841 [details] [diff] [review]
use null SMSC by default : V3

1. verified on Otoro
2. address review comment #12
Comment 15 Philipp von Weitershausen [:philikon] 2012-06-20 16:45:05 PDT
Comment on attachment 634841 [details] [diff] [review]
use null SMSC by default : V3

Thanks for addressing the comments and testing the patch! No need to request review again, though, I already r+'ed in comment 12. :)
Comment 16 Vicamo Yang [:vicamo][:vyang] 2012-06-20 19:50:09 PDT
http://hg.mozilla.org/integration/mozilla-inbound/rev/d5b372a52140
Comment 17 Ed Morley [:emorley] 2012-06-21 04:03:07 PDT
https://hg.mozilla.org/mozilla-central/rev/d5b372a52140

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