799851 - B2G STKUI: Support Proactive command 'SET UP EVENT LIST'

Status: RESOLVED FIXED

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

Description

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

Implement the required top level features (on STKUI) in order to support STK Events.

Comment 1

[Andreas Gal :gal]

This is a feature bug and not stabilization work. We should only take this if we can't ship the product without it and in that case we should also surface Stk as not feature complete and identify the remaining work.

Comment 2

[Marcelino Veiga Tuimil [:sonmarce]]

(In reply to Andreas Gal :gal from comment #1)
> This is a feature bug and not stabilization work. We should only take this
> if we can't ship the product without it and in that case we should also
> surface Stk as not feature complete and identify the remaining work.

STK is a must for VIVO as they already have a number of applications we need to support. We already landed some of the functionality related to STK, and we need to continue with the process. Besides, this task exposes what is being implemented in platform task https://bugzilla.mozilla.org/show_bug.cgi?id=790543 that is blocking-basecamp+. If we do not implement this gaia task, platform task is useless.

Comment 3

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

(In reply to Andreas Gal :gal from comment #1)
> Stk as not feature complete and identify the remaining work.

Since this started before the move to bugzilla, Yoshi and me had the list of issues in my Github account: https://github.com/frsela/gaia/issues

Comment 4

[Beatriz Rodríguez [:brg]]

(In reply to Andreas Gal :gal from comment #1)
> This is a feature bug and not stabilization work. We should only take this
> if we can't ship the product without it and in that case we should also
> surface Stk as not feature complete and identify the remaining work.

We were on Brasil last week to properly implement this functionality and UX with Yoshi and the operator support. This is a important feature, that was the reason of the trip. The trip was delayed because of several reasons that is why we could not make this improvements in advance.
Please change this bug to blocking-basecamp+.

Comment 5

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

Just rebased on-working code on: https://github.com/frsela/gaia/commits/stkui_events

Comment 6

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

Attachment: Patch to supoprt Events registration

This patch didn't work on Otoro so Faked method is added for allow us continue working

NOTE: If blocking-basecamp+ is set, just land it for now.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: 
Testing completed: 
Risk to taking this patch (and alternatives if risky):

Comment 7

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

Blocking-basecamp+, you don't need my approval. You still need a review though :)

Comment 8

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

Comment on attachment 671823 [details] [diff] [review]
Patch to supoprt Events registration

Oh! thats true :)

well... a r? is going to you :)

Comment 9

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

Comment on attachment 671823 [details] [diff] [review]
Patch to supoprt Events registration

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

::: apps/settings/js/icc.js
@@ +82,5 @@
> +                          icc.STK_EVENT_TYPE_LOCAL_CONNECTION,
> +                          icc.STK_EVENT_TYPE_NETWORK_SEARCH_MODE_CHANGED,
> +                          icc.STK_EVENT_TYPE_BROWSING_STATUS,
> +                          icc.STK_EVENT_TYPE_FRAMES_INFORMATION_CHANGED]);
> +      }

This is very unclear why you need that. If this is debug only I would live that out of the PR and you may want to write a mochitests instead.

@@ +227,5 @@
> +   * Process STK Events
> +   */
> +  function processSTKEvents(eventList) {
> +    for (var evt in eventList) {
> +      debug(' STK Registering event: ' + JSON.stringify(eventList[evt]));

JSON.stringify would be called even if you're not in debug mode. You may want to use the debug done by kaze.

Comment 10

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

(In reply to Vivien Nicolas (:vingtetun) from comment #9)
> Comment on attachment 671823 [details] [diff] [review]
> Patch to supoprt Events registration
> 
> Review of attachment 671823 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: apps/settings/js/icc.js
> @@ +82,5 @@
> > +                          icc.STK_EVENT_TYPE_LOCAL_CONNECTION,
> > +                          icc.STK_EVENT_TYPE_NETWORK_SEARCH_MODE_CHANGED,
> > +                          icc.STK_EVENT_TYPE_BROWSING_STATUS,
> > +                          icc.STK_EVENT_TYPE_FRAMES_INFORMATION_CHANGED]);
> > +      }
> 
> This is very unclear why you need that. If this is debug only I would live
> that out of the PR and you may want to write a mochitests instead.
> 

Well, this FAKE approach is because nowadays Otoro handsets didn't support STK Events but Galaxy SII support them so the implementation should be different on each of them ...

We're talking with manufacturer to solve this. With out FAKE events we cann't test the patches on bug #800264

I can remove this, but how to proceed? a different patch on #800264 for add the fake registration?

> @@ +227,5 @@
> > +   * Process STK Events
> > +   */
> > +  function processSTKEvents(eventList) {
> > +    for (var evt in eventList) {
> > +      debug(' STK Registering event: ' + JSON.stringify(eventList[evt]));
> 
> JSON.stringify would be called even if you're not in debug mode. You may
> want to use the debug done by kaze.

Yeap, on rebase I forgot to change this one ...

Comment 11

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

Attachment: Patch to supoprt Events registration V1.1

Rebased and removed FAKE registration

Comment 12

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

PR: https://github.com/mozilla-b2g/gaia/pull/5976
Merged: https://github.com/mozilla-b2g/gaia/commit/b19f83cd0e93de0b88481ebad0abb193775c22f4