Closed Bug 786782 Opened 8 years ago Closed 8 years ago

B2G SMS: can't sent multipart messages in some countries

Categories

(Core :: DOM: Device Interfaces, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla19
blocking-basecamp +
Tracking Status
firefox18 --- fixed
firefox19 --- fixed

People

(Reporter: vicamo, Assigned: vicamo)

Details

(Whiteboard: [WebAPI:P0][LOE:L])

Attachments

(2 files, 13 obsolete files)

It is possible that the SMSC might ignore the SMS-DELIVERY-REPORT request and B2G waits for the signal that never comes to send the next message segment out. In Brazil, VIVO, the SMS-DELIVERY-REPORT is not always available and it's possible that we'll never receive a long long message.
This is a major defect in some countries, so should be marked as blocking-basecamp.
blocking-basecamp: --- → ?
(In reply to Vicamo Yang [:vicamo][:vyang] from comment #1)
> This is a major defect in some countries, so should be marked as
> blocking-basecamp.

Agreed.  Vicamo, can you own this?
Assignee: nobody → vyang
blocking-basecamp: ? → +
Attached patch WIP (obsolete) — Splinter Review
Attached patch debug patch (obsolete) — Splinter Review
text more than 10 characters will be divided into four more segments, so we can easily verify this issue ;)
Barone, could you help verifying the patch in your country?
Tested in Spain and it doesn't work. A part of that we have seen that now you're not able to send sms properly. You can only send 1 line with up to 11 chars
Maximum 5 lines with 1 chars
(In reply to mbarone from comment #6)
> Tested in Spain and it doesn't work. A part of that we have seen that now
> you're not able to send sms properly. You can only send 1 line with up to 11
> chars
> Maximum 5 lines with 1 chars

That's terrible. The patch goes fine here in Taiwan, too. Could you please help again and capture some log for me?
What kind of log do you need? with a Logcat is enough or you need further information?
(In reply to mbarone from comment #8)
> What kind of log do you need? with a Logcat is enough or you need further
> information?

logcat please, to be detailed:
1. apply the debug patch attached
2. turn on DEBUG_WORKER & DEBUG_RIL in ${B2G_HOME}/gecko/dom/system/gonk/ril_consts.js
3. send a message with ~12 characters, all in ASCII

Thank you :)
Whiteboard: [WebAPI:P0]
Attached file Logcat test - First 3 sms (obsolete) —
Attached file Logcat test - Last 5 sms (obsolete) —
Attached file Screenshots (obsolete) —
Keywords: feature
(I wouldn't call this a feature. This is a bug in an existing feature.)
Keywords: feature
Attached patch WIP: v2 (obsolete) — Splinter Review
1. merge _processSentSmsSegment into REQUEST_SEND_SMS handler.
2. don't store messageRef/ackPDU/errorCode for they're never used outside the handler. Read ackPDU ad errorCode in debug block, too.
3. save options in _pendingSentSmsMap at the last successfully sent segment as Android does. This way, we'll only have to process SMS-STATUS-REPORT status code once and simplifies error handling much more.
Attachment #657396 - Attachment is obsolete: true
Attached patch debug patch (obsolete) — Splinter Review
changed: rebase to latest mozilla/master & add more debug print in REQUEST_SEND_SMS handler.

@mbarone, could you help collect debug logs again with these renewed patches? The steps are exactly the same as described in comment #9. You only have to send a message with 12..15 characters. The debug patch shortens segment boundary so you don't have to click hundreds times.
Attachment #657420 - Attachment is obsolete: true
Attached file Logcat output (obsolete) —
logcat output
Attached image images sms (obsolete) —
Attached image images sms (obsolete) —
With the last patch we can send SMS with 5, 10, 11 and 32 chars. Still we are not able to send SMS with more that 160 chars.
(In reply to mbarone from comment #19)
> With the last patch we can send SMS with 5, 10, 11 and 32 chars. Still we
> are not able to send SMS with more that 160 chars.

Actually, with the debug patch shortens segment boundary to 10, if you are able to receive a message with more than 10 characters, say 11, the multipart logic works for you. In fact, from the logged messages, it does work as expected.

The only problem now will be why you fails at sending 25th segment. Your first try for 62 segments(up to ~200 characters) begins with message ref 213 and failed at 237. The second try begins at 238 and last successful send is 5. (255 - 238 + 1) + (5 - 0 + 1) = 24. So every time you try to send the 25th segment, it fails with GENERIC_ERROR. 24 segments, with the debug patch, has 72 characters.

> I/Gecko   (  507): RIL Worker: REQUEST_SEND_SMS: rilRequestError = 2

I'll have some experiment locally about the maximum number of segments.
Whiteboard: [WebAPI:P0] → [WebAPI:P0][LOE:M]
(In reply to Vicamo Yang [:vicamo][:vyang] from comment #20)
> (In reply to mbarone from comment #19)
> > With the last patch we can send SMS with 5, 10, 11 and 32 chars. Still we
> > are not able to send SMS with more that 160 chars.
> I'll have some experiment locally about the maximum number of segments.

I guess there are some magic rules in your local carrier. I have no idea why you have a hidden boundary and might need the exact number for some research/googling.

Could you please help verify following conditions? Note that new line '\n' is also counted as one character. You don't have to capture log for these. Just tell me the result.

1. don't apply debug patch, send a message with 72 characters.
2. don't apply debug patch, send a message with 73 characters.
3. don't apply debug patch, send a message with 74 characters.
4. don't apply debug patch, send a message with 75 characters.
5. apply debug patch, send a message with 72 characters.
6. apply debug patch, send a message with 73 characters.
7. apply debug patch, send a message with 74 characters.
8. apply debug patch, send a message with 75 characters.
(In reply to Vicamo Yang [:vicamo][:vyang] from comment #21)
> I guess there are some magic rules in your local carrier. I have no idea why
> you have a hidden boundary and might need the exact number for some
> research/googling.

I got the same error with you with the same patch & input, but failed in 38th segment. I still can send long messages without the debug patch applied.
Whiteboard: [WebAPI:P0][LOE:M] → [WebAPI:P0][LOE:L]
Attached file Data results (obsolete) —
(In reply to mbarone from comment #23)
> Created attachment 662565 [details]
> Data results

The unpatched results do not contain debug messages, but those of patched gecko are all sent and delivered successfully. I think I have to build a libril from source and see if we can find what's wrong in proprietary binaries. Thank you mbarone :)
Quick and crazy idea, could this issue be related with the fact that we are not using RIL_REQUEST_SEND_SMS_EXPECT_MORE for this?

https://github.com/mozilla-b2g/android-hardware-ril/blob/master/include/telephony/ril.h
There is no parcel ever created with it since the very first commit of the repository. It's neither used in codeaurora nor in CyanogenMod. There is, however, still some unofficial variants of reference-ril port that utilize RIL_REQUEST_SEND_SMS_EXPECT_MORE(AT+CMMS), but there is also a warning posted[1] on Nokia's developer forum indicating that we should not use AT+CMMS for it may not be supported by your local network. Besides, taking Samsung Galaxy S2 for example, you can't grep the strings "AT+CMMS" or "CMMS" alone in the extracted proprietary binary files, which means their RIL implementation doesn't support AT+CMMS at all!

[1]: http://www.developer.nokia.com/Community/Discussion/showthread.php?38201-At-cmms-2
Attachment #659239 - Attachment is obsolete: true
Attachment #659240 - Attachment is obsolete: true
Attachment #659241 - Attachment is obsolete: true
Attachment #660385 - Attachment is obsolete: true
Attachment #660386 - Attachment is obsolete: true
Attachment #660387 - Attachment is obsolete: true
Attachment #662565 - Attachment is obsolete: true
Attached file logcat-2000.txt (obsolete) —
Attached is logcat output captured at sending a text message of 2000 chars, 160 chars in each segment, and 14 segments in total.
Attached file logcat-radio-2000.txt (obsolete) —
Attached is the corresponding output of rild. It has following snatches in the end:

> RIL <--- WMS_MSG_EVENT_SUBMIT_REPORT(139280), RID 0, MID 0 --- AMSS
> EVENT_SUBMIT_REPORT: msg mode 1 rpt status 100 cause value 28
> [RID 0] ReqList entries : Empty 
> UI <--- RIL_REQUEST_SEND_SMS (25) Complete --- RIL [RID 0, Token 86,
>     Generic Failure, Len 12 ]
The status code 100(0x64) means "Quality of service not available" if it is a TP-Status value defined in 3GPP TS 23.040 section 9.2.3.15. The cause value is unknown for me.

In fact, I find the error is because I ran out of quota in that pre-paid SIM card. Another 1000 and 2000 chars messages was sent and received successfully with a new SIM card in the same device. However, now we know it's possible to find out the root cause by looking up "SUBMIT_REPORT" in the output of `adb logcat -b radio`.
I'm going to land this issue with an updated patch for the situation we have in bug description. The case we have for Spain and latterly reproduced in Taiwan is quite different and I've filed a new bug 798173 for it. I'll move log files and findings there.
Attachment #668122 - Attachment is obsolete: true
Attachment #668127 - Attachment is obsolete: true
Add one default disabled debug message for possible early return on rilRequestError.
Attachment #659964 - Attachment is obsolete: true
Attachment #668281 - Flags: review?(marshall)
Since we will no longer depends on SMS-STATUS-REPORT for sending multipart messages, there should be test cases for that in test_outgoing.js.
Attachment #668283 - Flags: review?(marshall)
Attachment #659965 - Attachment is obsolete: true
Attachment #668281 - Flags: review?(marshall) → review+
Comment on attachment 668283 [details] [diff] [review]
Part 2: test cases

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

Thanks for updating the test!

::: dom/sms/tests/marionette/test_outgoing.js
@@ +23,5 @@
> +                + "Within his bending sickle's compass come;\n"
> +                + "Love alters not with his brief hours and weeks,\n"
> +                + "But bears it out even to the edge of doom:\n\n"
> +                + "If this be error and upon me proved,\n"
> +                + "I never writ, nor no man ever loved. ";

I approve of this long test message :)
Attachment #668283 - Flags: review?(marshall) → review+
https://hg.mozilla.org/mozilla-central/rev/e1d22d477afc
https://hg.mozilla.org/mozilla-central/rev/a732d3fc3e28
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
You need to log in before you can comment on or make changes to this bug.