Closed Bug 1062720 Opened 5 years ago Closed 5 years ago

[GCF] STK‘27.22.4.2.5/1’fail,GET INKEY, "Yes/No" Response for the input error

Categories

(Firefox OS Graveyard :: RIL, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(blocking-b2g:1.4+, firefox33 wontfix, firefox34 fixed, firefox35 fixed, b2g-v1.4 fixed, b2g-v2.0 wontfix, b2g-v2.0M fixed, b2g-v2.1 fixed, b2g-v2.2 fixed)

RESOLVED FIXED
2.1 S5 (26sep)
blocking-b2g 1.4+
Tracking Status
firefox33 --- wontfix
firefox34 --- fixed
firefox35 --- fixed
b2g-v1.4 --- fixed
b2g-v2.0 --- wontfix
b2g-v2.0M --- fixed
b2g-v2.1 --- fixed
b2g-v2.2 --- fixed

People

(Reporter: xianmao.meng, Assigned: edgar)

References

Details

(Whiteboard: sprd347118 )

Attachments

(1 file, 2 obsolete files)

[Blocking Requested - why for this release]:

when response Yes,RIL send AT> AT+SPUSATTERMINAL="8103012204020282818301008D0104";

when response No,RIL send AT> AT+SPUSATTERMINAL="810301220402028281830100";

All these do not contain response infomation.It should send "8103012204820282818301008D020401" and "8103012204820282818301008D020400".
log:
11-15 03:27:43.200   107   107 I Gecko   : -*- RadioInterfaceLayer: Received 'RIL:SendStkResponse' message from content process
11-15 03:27:43.200   107   522 I Gecko   : RIL Worker: [0] Received chrome message {"resultCode":0,"input":1,"command":{"commandNumber":1,"typeOfCommand":34,"commandQualifier":4,"rilMessageType":"stkcommand","options":{"text":"Enter YES","minLength":1,"maxLength":1,"isYesNoRequested":true},"rilMessageClientId":0},"rilMessageClientId":0,"rilMessageToken":19,"rilMessageType":"sendStkTerminalResponse"}
11-15 03:27:43.200   107   522 I Gecko   : RIL Worker: [0] New outgoing parcel of type 70
11-15 03:27:43.200   107   522 I Gecko   : RIL Worker: Outgoing parcel: 0,0,0,76,70,0,0,0,86,0,0,0,30,0,0,0,56,0,49,0,48,0,51,0,48,0,49,0,50,0,50,0,48,0,52,0,48,0,50,0,48,0,50,0,56,0,50,0,56,0,49,0,56,0,51,0,48,0,49,0,48,0,48,0,56,0,68,0,48,0,49,0,48,0,52,0,0,0,0,0

11-15 03:27:43.200   139   565 D use-Rlog/RLOG-RILC: [w] PCB request code 70 token 86
11-15 03:27:43.200   139   565 D use-Rlog/RLOG-RILC: [w] [0086]> STK_SEND_TERMINAL_RESPONSE (8103012204020282818301008D0104)
11-15 03:27:43.200   139   565 D use-Rlog/RLOG-RIL: [w] onRequest: STK_SEND_TERMINAL_RESPONSE sState=4
11-15 03:27:43.200   139   565 D use-Rlog/RLOG-RIL: [w] channel1 state: '0' 
11-15 03:27:43.200   139   565 D use-Rlog/RLOG-RIL: [w] get Channel ID '1'
11-15 03:27:43.200   139   565 D use-Rlog/RLOG-AT: [w] Channel1: AT> AT+SPUSATTERMINAL="8103012204020282818301008D0104"
Flags: needinfo?(cyang)
log:
11-15 03:27:43.200   107   107 I Gecko   : -*- RadioInterfaceLayer: Received 'RIL:SendStkResponse' message from content process
11-15 03:27:43.200   107   522 I Gecko   : RIL Worker: [0] Received chrome message {"resultCode":0,"input":1,"command":{"commandNumber":1,"typeOfCommand":34,"commandQualifier":4,"rilMessageType":"stkcommand","options":{"text":"Enter YES","minLength":1,"maxLength":1,"isYesNoRequested":true},"rilMessageClientId":0},"rilMessageClientId":0,"rilMessageToken":19,"rilMessageType":"sendStkTerminalResponse"}
11-15 03:27:43.200   107   522 I Gecko   : RIL Worker: [0] New outgoing parcel of type 70
11-15 03:27:43.200   107   522 I Gecko   : RIL Worker: Outgoing parcel: 0,0,0,76,70,0,0,0,86,0,0,0,30,0,0,0,56,0,49,0,48,0,51,0,48,0,49,0,50,0,50,0,48,0,52,0,48,0,50,0,48,0,50,0,56,0,50,0,56,0,49,0,56,0,51,0,48,0,49,0,48,0,48,0,56,0,68,0,48,0,49,0,48,0,52,0,0,0,0,0

11-15 03:27:43.200   139   565 D use-Rlog/RLOG-RILC: [w] PCB request code 70 token 86
11-15 03:27:43.200   139   565 D use-Rlog/RLOG-RILC: [w] [0086]> STK_SEND_TERMINAL_RESPONSE (8103012204020282818301008D0104)
11-15 03:27:43.200   139   565 D use-Rlog/RLOG-RIL: [w] onRequest: STK_SEND_TERMINAL_RESPONSE sState=4
11-15 03:27:43.200   139   565 D use-Rlog/RLOG-RIL: [w] channel1 state: '0' 
11-15 03:27:43.200   139   565 D use-Rlog/RLOG-RIL: [w] get Channel ID '1'
11-15 03:27:43.200   139   565 D use-Rlog/RLOG-AT: [w] Channel1: AT> AT+SPUSATTERMINAL="8103012204020282818301008D0104"
Flags: needinfo?(cyang)
Whiteboard: sprd347118
Summary: STK‘27.22.4.2.5/1’fail,GET INKEY, "Yes/No" Response for the input error → [dolphin][GCF] STK‘27.22.4.2.5/1’fail,GET INKEY, "Yes/No" Response for the input error
main cause is operation for string '1' and number 1. String '1' should be 0x31 , no issue. 
But for number 1 it should be translated to 0x01, but there is a bug for GetinKey command response.
Hi shawn,

I found this issue may due to the following code process in ril_worker.js.

case STK_TEXT_CODING_GSM_8BIT:
  for (let i = 0; i < text.length; i++) {
     GsmPDUHelper.writeHexOctet(text.charCodeAt(i));
  }

if the GET INKEY response is YES or No,we process it as 0x01 and 0x00,but we can not write it as the way of writing string. We should write the response as:
 GsmPDUHelper.writeHexOctet(0x01);

Besides,the following code in ril_worker.js may has some error,

if (response.isYesNo !== undefined) {

when the GET_INKEY responses,response.isYesNo is always undefined.

Please help check the issue.
Thanks!
Flags: needinfo?(sku)
ni? Edgar to see if he has time to check also since I will be in partner side next whole week.
Flags: needinfo?(echen)
(In reply to 孟宪茂 from comment #4)
> Hi shawn,
> 
> I found this issue may due to the following code process in ril_worker.js.
> 
> case STK_TEXT_CODING_GSM_8BIT:
>   for (let i = 0; i < text.length; i++) {
>      GsmPDUHelper.writeHexOctet(text.charCodeAt(i));
>   }
> 
> if the GET INKEY response is YES or No,we process it as 0x01 and 0x00,but we
> can not write it as the way of writing string. We should write the response
> as:
>  GsmPDUHelper.writeHexOctet(0x01);
> 
> Besides,the following code in ril_worker.js may has some error,
> 
> if (response.isYesNo !== undefined) {
> 
> when the GET_INKEY responses,response.isYesNo is always undefined.
> 
> Please help check the issue.
> Thanks!

Hmm, Gaia should use |isYesNo| [1] to response the "Yes or No" request (when |isYesNoRequested| is true), instead of using |input| [2].

[1] http://dxr.mozilla.org/mozilla-central/source/dom/webidl/MozStkCommandEvent.webidl#574-579
[2] https://github.com/mozilla-b2g/gaia/blob/b630b8bcaf9653885539d4449bc65c3b592bd752/apps/system/js/icc.js#L549-L561
Flags: needinfo?(sku)
Flags: needinfo?(echen)
(In reply to 孟宪茂 from comment #4)
> Hi shawn,
> 
> I found this issue may due to the following code process in ril_worker.js.
> 
> case STK_TEXT_CODING_GSM_8BIT:
>   for (let i = 0; i < text.length; i++) {
>      GsmPDUHelper.writeHexOctet(text.charCodeAt(i));
>   }
> 
> if the GET INKEY response is YES or No,we process it as 0x01 and 0x00,but we
> can not write it as the way of writing string. We should write the response
> as:
>  GsmPDUHelper.writeHexOctet(0x01);

Yes, this is an issue that we need to fix in ril_worker.
Thank you.
Attached patch [Gecko] WIP Patch, v1 (obsolete) — Splinter Review
Attached patch [Gaia] WIP Patch, v1 (obsolete) — Splinter Review
(In reply to Edgar Chen [:edgar][:echen] from comment #8)
> Created attachment 8486211 [details] [diff] [review]
> [Gecko] WIP Patch, v1

(In reply to Edgar Chen [:edgar][:echen] from comment #9)
> Created attachment 8486212 [details] [diff] [review]
> [Gaia] WIP Patch, v1

Hi xianmao,

Could you help to apply both gecko and gaia patch and test again to see if they fix this issue?
Thank you.
Flags: needinfo?(xianmao.meng)
Comment on attachment 8486212 [details] [diff] [review]
[Gaia] WIP Patch, v1

(In reply to Edgar Chen [:edgar][:echen] from comment #6)
> (In reply to 孟宪茂 from comment #4)
> > 
> > Besides,the following code in ril_worker.js may has some error,
> > 
> > if (response.isYesNo !== undefined) {
> > 
> > when the GET_INKEY responses,response.isYesNo is always undefined.
> > 
> > Please help check the issue.
> > Thanks!
> 
> Hmm, Gaia should use |isYesNo| [1] to response the "Yes or No" request (when
> |isYesNoRequested| is true), instead of using |input| [2].

Hi Fernando,

I am not sure if it's a good way to solve this. Could you kindly give me some feedback about this patch.
(I will file a separated bug for Gaia, but I would like to have your feedback here first)

Thank you.

> 
> [1]
> http://dxr.mozilla.org/mozilla-central/source/dom/webidl/MozStkCommandEvent.
> webidl#574-579
> [2]
> https://github.com/mozilla-b2g/gaia/blob/
> b630b8bcaf9653885539d4449bc65c3b592bd752/apps/system/js/icc.js#L549-L561
Attachment #8486212 - Flags: feedback?(frsela)
Assignee: nobody → echen
(In reply to Edgar Chen [:edgar][:echen] from comment #10)
> (In reply to Edgar Chen [:edgar][:echen] from comment #8)
> > Created attachment 8486211 [details] [diff] [review]
> > [Gecko] WIP Patch, v1
> 
> (In reply to Edgar Chen [:edgar][:echen] from comment #9)
> > Created attachment 8486212 [details] [diff] [review]
> > [Gaia] WIP Patch, v1
> 
> Hi xianmao,
> 
> Could you help to apply both gecko and gaia patch and test again to see if
> they fix this issue?
> Thank you.

Hi Edgar,

I have tested GET INKEY with AT command,it shows the following results.

When responsed "YES" and "NO":
D/use-Rlog/RLOG-AT(  129): [w] Channel1: AT> AT+SPUSATTERMINAL="8103012204020282818301008D020401"
D/use-Rlog/RLOG-AT(  129): [w] Channel1: AT> AT+SPUSATTERMINAL="8103012204020282818301008D020400"

It works fine.
Thanks!
Hi Edgar,

The gecko and gaia patch can fix this issue,please help evaluate and merge the change.
Thanks!
Flags: needinfo?(xianmao.meng) → needinfo?(echen)
Can you triage this?
Flags: needinfo?(wchang)
Taking for GCF.

Edgar,

If applicable (i.e. impact is generic across the platform), can you also request for 2.0 approval? I think this is valuable.
blocking-b2g: 1.4? → 1.4+
Flags: needinfo?(wchang)
Blocks: 1065401
Attachment #8486212 - Flags: feedback?(frsela) → feedback+
Thanks for your feedback, Fernando. I will send a PR in bug 1065401.
Attached patch Patch, v2Splinter Review
Attachment #8486211 - Attachment is obsolete: true
Flags: needinfo?(echen)
Attachment #8486212 - Attachment is obsolete: true
Comment on attachment 8487936 [details] [diff] [review]
Patch, v2

Hi Hsinyi, could you help to review this? Thank you.
Attachment #8487936 - Flags: review?(htsai)
Comment on attachment 8487936 [details] [diff] [review]
Patch, v2

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

Looks good to me, thank you!
Attachment #8487936 - Flags: review?(htsai) → review+
https://hg.mozilla.org/mozilla-central/rev/b6cfed9179c4
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.1 S5 (26sep)
Comment on attachment 8487936 [details] [diff] [review]
Patch, v2

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): NA
User impact if declined: GCF STK ‘7.22.4.2.5/1’ fail (GET INKEY)
Testing completed: Partner had already help to test it and we also wrote a xpcshell test for this change.
Risk to taking this patch (and alternatives if risky): No
String or UUID changes made by this patch: No
Attachment #8487936 - Flags: approval-mozilla-b2g30?
Does this need to land on any other branches besides v1.4?
status-b2g-v2.0: --- → ?
status-b2g-v2.1: --- → ?
Flags: needinfo?(echen)
Comment on attachment 8487936 [details] [diff] [review]
Patch, v2

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #24)
> Does this need to land on any other branches besides v1.4?

Yes, I think it is valuable for all other branches.

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
See comment #23.
Attachment #8487936 - Flags: approval-mozilla-b2g32?
Attachment #8487936 - Flags: approval-mozilla-aurora?
Flags: needinfo?(echen)
Attachment #8487936 - Flags: approval-mozilla-b2g30?
Attachment #8487936 - Flags: approval-mozilla-b2g30+
Attachment #8487936 - Flags: approval-mozilla-aurora?
Attachment #8487936 - Flags: approval-mozilla-aurora+
Comment on attachment 8487936 [details] [diff] [review]
Patch, v2

Got feedback from CAF that its too late to late this change on 2.0 at this point CAF users wont hit this..So if needed we can land this on 2.0M only and hence I am switching the blocking flag to that.
Attachment #8487936 - Flags: approval-mozilla-b2g32? → approval-mozilla-b2g32-
NI Kli, for 2.0M.
Flags: needinfo?(kli)
Blocks: b2g-stk
Summary: [dolphin][GCF] STK‘27.22.4.2.5/1’fail,GET INKEY, "Yes/No" Response for the input error → [GCF] STK‘27.22.4.2.5/1’fail,GET INKEY, "Yes/No" Response for the input error
You need to log in before you can comment on or make changes to this bug.