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

Status: RESOLVED FIXED

(Firefox OS Graveyard :: RIL, defect)

Description

[孟宪茂]

[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".

Comment 1

[孟宪茂]

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"

Comment 2

[孟宪茂]

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"

Comment 3

[siiaceon]

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.

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!

Comment 5

[shawn ku [:sku]]

ni? Edgar to see if he has time to check also since I will be in partner side next whole week.

Comment 6

[Edgar Chen [:edgar]]

(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

Comment 7

[Edgar Chen [:edgar]]

(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.

Comment 8

[Edgar Chen [:edgar]]

Attachment: [Gecko] WIP Patch, v1

Comment 9

[Edgar Chen [:edgar]]

Attachment: [Gaia] WIP Patch, v1

Comment 10

[Edgar Chen [:edgar]]

(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.

Comment 11

[Edgar Chen [:edgar]]

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

Comment 12

[孟宪茂]

(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!

Comment 13

[孟宪茂]

Hi Edgar,

The gecko and gaia patch can fix this issue,please help evaluate and merge the change.
Thanks!

Comment 14

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

Can you triage this?

Comment 15

[Wayne Chang [: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.

Comment 16

[Edgar Chen [:edgar]]

Thanks for your feedback, Fernando. I will send a PR in bug 1065401.

Comment 17

[Edgar Chen [:edgar]]

Attachment: Patch, v2

Comment 18

[Edgar Chen [:edgar]]

Comment on attachment 8487936 [details] [diff] [review]
Patch, v2

Hi Hsinyi, could you help to review this? Thank you.

Comment 19

[Hsin-Yi Tsai (she/her)[:hsinyi]]

Comment on attachment 8487936 [details] [diff] [review]
Patch, v2

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

Looks good to me, thank you!

Comment 20

[Edgar Chen [:edgar]]

Try server: https://tbpl.mozilla.org/?tree=Try&rev=46ba712a435b

Comment 21

[Edgar Chen [:edgar]]

https://hg.mozilla.org/integration/b2g-inbound/rev/b6cfed9179c4

Comment 22

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/b6cfed9179c4

Comment 23

[Edgar Chen [:edgar]]

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

Comment 24

[Ryan VanderMeulen [:RyanVM]]

Does this need to land on any other branches besides v1.4?

Comment 25

[Edgar Chen [:edgar]]

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.

Comment 26

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/33c183bacd64
https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/b5abdb7b887a

Comment 27

[bhavana bajaj [:bajaj]]

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.

Comment 28

[bhavana bajaj [:bajaj]]

NI Kli, for 2.0M.

Comment 29

[Kai-Zhen Li [:seinlin][:kli]]

http://hg.mozilla.org/releases/mozilla-b2g32_v2_0m/rev/79a3d8c13b22