Closed Bug 951586 Opened 7 years ago Closed 7 years ago

[DSDS][STK] Suport multiple cards in system STK code

Categories

(Firefox OS Graveyard :: Gaia::System, defect)

defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED
1.4 S1 (14feb)

People

(Reporter: contacto, Assigned: frsela)

References

Details

Attachments

(2 files)

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/31.0.1650.63 Safari/537.36

Steps to reproduce:

To solve the bug 934404, we have divided into three parts.

In the first, we need Gaia to work with multiple cards, rewriting the code that handles icc, and it depends on a IccId.

message object will be formed by command and ICCID to change handleSTKCommand.


Actual results:

No dual SIM management.


Expected results:

Prepare Gaia to work with dual SIM.
Depends on: 934404
Blocks: 951595
Blocks: 951599
Component: General → Gaia::System
No longer depends on: 934404
Blocks: 933404
Blocks: 934404
No longer blocks: 933404
Assignee: nobody → frsela
Hi,

This patch implements STK DSDS support into System app (settings app is still pending work)

I tested with all commands but since it's a big change, I would like to ask you (:cyang) for pass all STK conformance test again.

Thanks in advance.
Attachment #8361612 - Flags: feedback?(cyang)
Duplicate of this bug: 951595
Attachment #8361612 - Flags: feedback?(cyang) → feedback?(nsarkar)
Comment on attachment 8361612 [details] [review]
Added STK DSDS support on system app

Verified.
Attachment #8361612 - Flags: feedback?(nsarkar) → feedback+
Attachment #8361612 - Flags: review?(timdream)
(In reply to Nivi from comment #3)
> Comment on attachment 8361612 [details] [review]
> Added STK DSDS support on system app
> 
> Verified.

Thank you !
Comment on attachment 8361612 [details] [review]
Added STK DSDS support on system app

Could you move this into an app? We have talked about this before.

I don't think it makes any sense to land more code into System app for STK.
Attachment #8361612 - Flags: review?(timdream)
(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #5)
> Comment on attachment 8361612 [details] [review]
> Added STK DSDS support on system app
> 
> Could you move this into an app? We have talked about this before.
> 
> I don't think it makes any sense to land more code into System app for STK.

Hi Tim,

As we commented here https://bugzilla.mozilla.org/show_bug.cgi?id=875679#c13 this code should live in system.

Adding to the commented advantages above, we shall add the certification process which is very strict, for example, we shall display messages over any app when the SIM card requires it (also over the dialer) or the app should be permanent opened in case SIM card use events.

Moreover, this patch is no "new code", it just covers the required changes to support DSDS.

We can consider some enhancements in a near future to lazy load required modules, for example, if an Operator SIM Card doesn't use events, the events module won't be loaded; also we can lazy load per STK command and load in memory only operator required ones. Anyway it would be another different bug.

Regards.
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Target Milestone: --- → 1.4 S1 (14feb)
Hi Tim,

Could you please confirm if we can move forward with the current proposal?.Thanks!.
Flags: needinfo?(timdream)
(In reply to Noemí Freire (:noemi) from comment #7)
> Hi Tim,
> 
> Could you please confirm if we can move forward with the current
> proposal?.Thanks!.

Bug 875679 comment 13 is almost 8 months ago. I am very concerned, since there were no action done nor even any bug filed.

Given the fact this patch is relatively small, compare to the entire STK code, and this bug is sit on the 1.4 target milestone, I am going to let it slide one last time. However for next releases I want to see effort being made on your side to address this.
Flags: needinfo?(timdream)
Comment on attachment 8361612 [details] [review]
Added STK DSDS support on system app

Please correct the l10n IDs before landing.
Attachment #8361612 - Flags: review+
(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #8)

Hey Tim,

 Thanks for clarifying what the issue in here: you want STK to be an independent app, and I agree. However, let me try to clarify something:

 - We did not request STK to be part of settings instead of a separated app, this was something Mozilla UX decided.
 - We developed the STK implementation for 1.0 as it was requested by Mozilla because of the Mozilla lack of resources.
 - We did not like original UX or the technical approach either, but both decisions were not taken by us: The first was a Mozilla UX decision, the second one due to platform limitations.
 - We have adapted STK to support dual SIM because it was a request from other partner, not because this is interesting to us. To be clear: we are not selling any device with dual SIM.

Don't get me wrong, I just want to be clear as I have the impression you feel this is how we want this feature to be, and that is not the case.

> Given the fact this patch is relatively small, compare to the entire STK
> code, and this bug is sit on the 1.4 target milestone, I am going to let it
> slide one last time. However for next releases I want to see effort being
> made on your side to address this.

OK, I think this is going to be the last time you need to let slide it: now it is an excellent time Mozilla can do the refactor of this code as we all always wanted to. You have a very good starting point: a working implementation, supporting dual SIM and that passes certification/IOT tests.

My last contribution to this STK thread is one recommendation and one request:

 - Recommendation: Align your independent app approach early with to your UX team to validate it. 
 - Request: Ensure no certification requirements are broken because of the re-factor, otherwise this will cause problems (and delays) during IOT.

Thanks!
(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #9)
> Comment on attachment 8361612 [details] [review]
> Added STK DSDS support on system app
> 
> Please correct the l10n IDs before landing.

Fixed and landed:

https://github.com/mozilla-b2g/gaia/commit/78a6be16d018148eaed880ddae51a86d933acbdc
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: 1.4 S1 (14feb) → ---
(In reply to Daniel Coloma:dcoloma from comment #10)

Thanks. I will be bringing these comments to relevant people.
Target Milestone: --- → 1.4 S1 (14feb)
Can you attach a screenshot of this code in action? I'm wondering if the length of the strings is a concern.
(In reply to Axel Hecht [:Pike] from comment #13)
> Can you attach a screenshot of this code in action? I'm wondering if the
> length of the strings is a concern.

attached in comment #14
Thanks. I hope that's gonna work out with the length of localized strings.
Depends on: 942096
Duplicate of this bug: 934404
Works okay on Fugu
GAIA_REV=730670951e40b2317a167fcd07c398bb662d6e87
GECKO_REV=ce6783cbe8ffeaea447ac7a5b9b11c2c4881f194
BUILD_TAG=jenkins-B2G.v1.4.0.fugu-4
BuildID=20140324140010
Status: RESOLVED → VERIFIED
Is it possible to cherry-pick it to v1.3t?
That would add new strings to 1.3(t), we shouldn't do that.
Blocks: 999370
You need to log in before you can comment on or make changes to this bug.