881675 - STK/ICC Mocked API

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: Gaia, defect)

Description

[Fernando R. Sela (no CC, needinfo please) [:frsela]]

Since STK development needs special SIM Cards which had all STK commands implemented is difficult to test all the commands.

The suggestion is to add support for a mocked version of ICC API exposed by mozIccManager in Gaia so can be tested on desktop + developers can test all STK commands without need special SIM Cards.

I'd implemented a first version I'm currently adding the most common commands. I'll upload the patch asap

Comment 1

[Fernando R. Sela (no CC, needinfo please) [:frsela]]

I'll use this mock for driving the STK refactoring on 875679

Comment 2

[Fernando R. Sela (no CC, needinfo please) [:frsela]]

As I told before, I'm working on this topic.

I've this skeleton and I'll add the commands ASAP:

https://github.com/frsela/gaia/compare/STK;Bug881675

@kaze, Now can be used to patch Gaia on developer machines, anyway, do you think this mock could be merged (when finished) in gaia? so STK can be tested on desktop too ;)

Comment 3

[Dietrich Ayala (:dietrich)]

This is to allow testing, will not ship in the build - giving leo+ to approve landing this code.

Comment 4

[Fernando R. Sela (no CC, needinfo please) [:frsela]]

Attachment: STK mock for testing and desktop

Comment 5

[Fernando R. Sela (no CC, needinfo please) [:frsela]]

Previously added patch implements a STK fake menu used to test all STK commands in an easy way.

In my mind is to add a GAIA compile parameter to add or not this mock into the final build.

Also, adding this kind of mock to the R2D2B2G (FFOS Simulator) addon will be great ;)

Comment 6

[Fernando R. Sela (no CC, needinfo please) [:frsela]]

Comment on attachment 775635 [details]
STK mock for testing and desktop

Removed review, I forgot one topic. Sorry.

Comment 7

[Fernando R. Sela (no CC, needinfo please) [:frsela]]

Comment on attachment 775635 [details]
STK mock for testing and desktop

Now is Ok for review ;)

Comment 8

[Tim Guan-tin Chien [:timdream] (please needinfo)]

This bug is super complex so I would need some more time to review....

Comment 9

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 775635 [details]
STK mock for testing and desktop

OK. There is too much unrelated changes (e.g. bookmark.js) in the patch thus this is impossible to do a proper review. Could you move these changes to their individual bugs? Thanks.

Comment 10

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Dietrich Ayala (:dietrich) from comment #3)
> This is to allow testing, will not ship in the build - giving leo+ to
> approve landing this code.

Is there any other way to test the same functionality here?

I would also like to know Vivien's opinion for including these mock code.

Comment 11

[Fernando R. Sela (no CC, needinfo please) [:frsela]]

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) from comment #9)
> Comment on attachment 775635 [details]
> STK mock for testing and desktop
> 
> OK. There is too much unrelated changes (e.g. bookmark.js) in the patch thus
> this is impossible to do a proper review. Could you move these changes to
> their individual bugs? Thanks.

Fixed. Merging issue added more unrelated changes. Rebased.

Comment 12

[Fernando R. Sela (no CC, needinfo please) [:frsela]]

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) from comment #10)
> (In reply to Dietrich Ayala (:dietrich) from comment #3)
> > This is to allow testing, will not ship in the build - giving leo+ to
> > approve landing this code.
> 
> Is there any other way to test the same functionality here?
> 

I would like to add a build parameter so it will be included or not at build time. WDYT?

Also, this allows testing all commands without SIM cards (no all SIMs uses all STK commands so it's difficult to reproduce errors on dev. environment)

Also, this patch allows testing on Nightly desktop browser which is awesome -> about this, could be great to add this mock (adapted of course) to the FFOS Simulator, so a bunch of buttons will launch STK commands for testing.

> I would also like to know Vivien's opinion for including these mock code.

Comment 13

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to Fernando R. Sela [:frsela] from comment #12)
> (In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) from comment #10)
> > (In reply to Dietrich Ayala (:dietrich) from comment #3)
> I would like to add a build parameter so it will be included or not at build
> time. WDYT?

This will be done automatically if you follow the suggestions above.
> 
> Also, this allows testing all commands without SIM cards (no all SIMs uses
> all STK commands so it's difficult to reproduce errors on dev. environment)
> 
> Also, this patch allows testing on Nightly desktop browser which is awesome
> -> about this, could be great to add this mock (adapted of course) to the
> FFOS Simulator, so a bunch of buttons will launch STK commands for testing.
>

There is a $GAIA/tools/extensions/desktop-helper/content/data/lib folder that has been created for such things. The extensions there are used by both Gaia and the simulator.

So instead of polluting applications I would rather prefer to land a mock in this folder.

CC'ing Myk and ochameau so they know what's going on. You guys would love to have STK API support for free no? :)

Comment 14

[Alexandre Poirot [:ochameau]]

+1, it will also allow you to avoid having each app to try loading the mock before using navigator.mozIccManager. So we can avoid doing this:
https://github.com/mozilla-b2g/gaia/pull/10978/files#L4R63
and just always use navigator.mozIccManager in all apps.

It should quite easy for you to port your current patch to the addon, just create a new js file, you can take example on existing one:
https://github.com/mozilla-b2g/gaia/blob/master/tools/extensions/desktop-helper/content/data/lib/cameras.js#L74
The only thing to know is that you need to call this magic method:
  FFOS_RUNTIME.makeNavigatorShim('mozIccManager', IccManager);
That will replace all app navigator.mozIccManager, with IccManager!
And add the new js file to this list:
https://github.com/mozilla-b2g/gaia/blob/master/tools/extensions/desktop-helper/bootstrap.js#L28

Comment 15

[Myk Melez [:myk] [@mykmelez]]

(In reply to Vivien Nicolas (:vingtetun) (:21) from comment #13)
> CC'ing Myk and ochameau so they know what's going on. You guys would love to
> have STK API support for free no? :)

You bet I would!

Comment 16

[Fernando R. Sela (no CC, needinfo please) [:frsela]]

Cool, I'll work on this... I agree with all suggestions, thanks !

Comment 17

[Jason Smith [:jsmith]]

No longer need to block on this - we're past the point of taking these code changes to 1.1.

Comment 18

[BMO Automation]

Firefox OS is not being worked on