794404 - B2G RIL: detect if ril supports RIL_REQUEST_STK_SEND_ENVELOPE_WITH_STATUS

Status: RESOLVED DUPLICATE of bug 795199

(Core :: DOM: Device Interfaces, defect)

Description

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

"RIL_REQUEST_STK_SEND_ENVELOPE_WITH_STATUS" is required for some STK features, like SMS PP Download (bug 736076) and call control (bug 791939). However, some ril doesn't support this. We need to know if "RIL_REQUEST_STK_SEND_ENVELOPE_WITH_STATUS" offered by ril. If not, we can at least use "RIL_REQUEST_STK_SEND_ENVELOPE" as our second choice.

Comment 1

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

(In reply to Hsin-Yi Tsai [:hsinyi] from comment #0)
> "RIL_REQUEST_STK_SEND_ENVELOPE_WITH_STATUS" is required for some STK
> features, like SMS PP Download (bug 736076) 
                                 ^^^^^^^^^^^^ 
sorry, the correct number is bug 736706 ^^"

> and call control (bug 791939).
> However, some ril doesn't support this. We need to know if
> "RIL_REQUEST_STK_SEND_ENVELOPE_WITH_STATUS" offered by ril. If not, we can
> at least use "RIL_REQUEST_STK_SEND_ENVELOPE" as our second choice.

Comment 2

[Philipp von Weitershausen [:philikon]]

(In reply to Hsin-Yi Tsai [:hsinyi] from comment #0)
> "RIL_REQUEST_STK_SEND_ENVELOPE_WITH_STATUS" is required for some STK
> features, like SMS PP Download (bug 736076) and call control (bug 791939).
> However, some ril doesn't support this. We need to know if
> "RIL_REQUEST_STK_SEND_ENVELOPE_WITH_STATUS" offered by ril. If not, we can
> at least use "RIL_REQUEST_STK_SEND_ENVELOPE" as our second choice.

Do we have a way of finding out ahead of time? If not, we'll just could just retry the same `options` object with a different request type at run-time, no?

Comment 3

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

Can we write some property into build.prop, like what we did for enabling RILQUIRKS_CALLSTATE_EXTRA_UINT32? eg. when config otoro, we disable RIL_REQUEST_STK_SEND_ENVELOPE_WITH_STATUS by writing property 'ro.moz.stk_send_envelope_with_status'?

Comment 4

[Philipp von Weitershausen [:philikon]]

(In reply to Hsin-Yi Tsai [:hsinyi] from comment #3)
> Can we write some property into build.prop, like what we did for enabling
> RILQUIRKS_CALLSTATE_EXTRA_UINT32? eg. when config otoro, we disable
> RIL_REQUEST_STK_SEND_ENVELOPE_WITH_STATUS by writing property
> 'ro.moz.stk_send_envelope_with_status'?

Fine by me!

Comment 5

[Philipp von Weitershausen [:philikon]]

Keep in mind that propdb keys are limited to 32 chars.

Comment 6

[Vincent Liu[:vliu]]

Hi Hsin-Yi,
May I have some discussions. Currently RadioInterfaceLayer issues REQUEST_STK_SEND_ENVELOPE_COMMAND by calling sendStkMenuSelection().But not for RIL_REQUEST_STK_SEND_ENVELOPE_WITH_STATUS. Combined with comment 3, my idea is using quirk in sendStkMenuSelection() to decide what commands we used for different ril. What do you think?

Comment 7

[Vincent Liu[:vliu]]

Attachment: WIP V1

Hi Hsin-Yi,
Thanks for discussion with you and I realized code architecture for this issue. Please see the attached patch file. For otoro, we don't do any modification in /device folder because the ril in otoro doesn't support this command. For the project which support it will also do code modification.

Hi Philikon,
Can you please help me review this patch? Thanks! For sgs2 and Nexux, I will do code modification in /device folder.

Comment 8

[Philipp von Weitershausen [:philikon]]

Comment on attachment 665790 [details] [diff] [review]
WIP V1

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

::: dom/system/gonk/ril_worker.js
@@ +3307,5 @@
>              // ME shall pass the message transparently to the UICC using the
>              // ENVELOPE (SMS-PP DOWNLOAD).` ~ 3GPP TS 31.111 7.1.1.1
> +            // Use RILQUIRKS_STK_ENVELOPE_WITH_STATUS to decide if ril is support
> +            // or not. If ril supports this command, fill this variable to true in
> +            // device/vendor/full_xxx.mk. If else, do nothing.

You're just describing how device-specific quirks work. That's not very useful. It would be more useful to explain *why* we need the quirk in the first place. Maybe something like:

  // Not all RIL implementations support the STK_ENVELOPE_WITH_STATUS
  // request type.

@@ +3310,5 @@
> +            // or not. If ril supports this command, fill this variable to true in
> +            // device/vendor/full_xxx.mk. If else, do nothing.
> +            if (RILQUIRKS_STK_ENVELOPE_WITH_STATUS) {
> +              this.dataDownloadViaSMSPP(message);
> +            }

I can't say anything to the technical details of this. Vicamo, can you review?

Comment 9

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 665790 [details] [diff] [review]
WIP V1

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

I think the original purpose of implementing SMS-PP download is to support SIM activation in Brasil. So, you can't just skip it if the rild doesn't support it. That way, SIM activation still fails. We might be able to replace RIL_REQUEST_STK_SEND_ENVELOPE_WITH_STATUS with RIL_REQUEST_STK_SEND_ENVELOPE_COMMAND, but I'm not fully sure that.

Comment 10

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Philipp von Weitershausen [:philikon] from comment #8)
> You're just describing how device-specific quirks work. That's not very
> useful. It would be more useful to explain *why* we need the quirk in the
> first place. Maybe something like:
> 
>   // Not all RIL implementations support the STK_ENVELOPE_WITH_STATUS
>   // request type.

I don't think this is necessary. rild will give you an unsupported error, and we'll know and handle it. That is, if this issue is going to be solved by skipping SMS-PP, then I think we don't bother here. That's already default behaviour. We should make it possible if it's possible for Otoro instead.

vliu, do you mind re-assign this bug to me?

Comment 11

[Vincent Liu[:vliu]]

Hi Vicamo,

Sure. Actually no one is in Assigned field.

Comment 12

[Vincent Liu[:vliu]]

Hi Vicamo, Hsin-Yi,

After our discussion, we decided we still need a quirk. May I confirm we called it ro.moz.stk.envelope_with_status?

Comment 13

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

(In reply to vliu from comment #11)
> Hi Vicamo,
> 
> Sure. Actually no one is in Assigned field.
(In reply to vliu from comment #12)
> Hi Vicamo, Hsin-Yi,
> 
> After our discussion, we decided we still need a quirk. May I confirm we
> called it ro.moz.stk.envelope_with_status?
I would prefer ro.moz.ril.XXX
ro.moz.ril.stk_envelope_w_status? clear enough?

Comment 14

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

(In reply to Hsin-Yi Tsai [:hsinyi] from comment #13)
> (In reply to vliu from comment #11)
> > Hi Vicamo,
> > 
> > Sure. Actually no one is in Assigned field.
> (In reply to vliu from comment #12)
> > Hi Vicamo, Hsin-Yi,
> > 
> > After our discussion, we decided we still need a quirk. May I confirm we
> > called it ro.moz.stk.envelope_with_status?
> I would prefer ro.moz.ril.XXX
> ro.moz.ril.stk_envelope_w_status? 
the length exceeds the limit... plz ignore my suggestion here...

Comment 15

[Vincent Liu[:vliu]]

Hi Hsin-Yi,

How about the ro.moz.ril.stk_envelope_w_stat ?

Comment 16

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

(In reply to vliu from comment #15)
> Hi Hsin-Yi,
> 
> How about the ro.moz.ril.stk_envelope_w_stat ?
Good for me!

Comment 17

[Vincent Liu[:vliu]]

(In reply to Hsin-Yi Tsai [:hsinyi] from comment #16)
> (In reply to vliu from comment #15)
> > Hi Hsin-Yi,
> > 
> > How about the ro.moz.ril.stk_envelope_w_stat ?
> Good for me!

Hi Hsin-Yi,
I just put request ro.moz.ril.stk_envelope_w_stat in github for galaxys2 and crespo. Please have a reference.

For galaxys2:
https://github.com/msliu/android-device-galaxys2/commit/a6d45c08eff1c67cec7a21a813ea6d3044966541
For crespo:
https://github.com/msliu/android-device-crespo/commit/03f8ebb8c2d0d07ed95a5493b2c01ada7065fa5e

Comment 19

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

(In reply to vliu from comment #17)
> (In reply to Hsin-Yi Tsai [:hsinyi] from comment #16)
> > (In reply to vliu from comment #15)
> > > Hi Hsin-Yi,
> > > 
> > > How about the ro.moz.ril.stk_envelope_w_stat ?
> > Good for me!
> 
> Hi Hsin-Yi,
> I just put request ro.moz.ril.stk_envelope_w_stat in github for galaxys2 and
> crespo. Please have a reference.
> 
> For galaxys2:
> https://github.com/msliu/android-device-galaxys2/commit/
> a6d45c08eff1c67cec7a21a813ea6d3044966541
> For crespo:
> https://github.com/msliu/android-device-crespo/commit/
> 03f8ebb8c2d0d07ed95a5493b2c01ada7065fa5e

vliu, thanks :)