+++ This bug was initially created as a clone of Bug #863098 +++ Implement the FM radio spec (attachment 742436 [details])

Hi, Brad, could you provide the speaker icons of on/off/pressed status for different screen sizes, including: - speaker off - speaker on - speaker pressed - speaker off @ 1.5x - spaker on @ 1.5x - speaker pressed @ 1.5x - speaker off @ 2x - spaker on @ 2x - speaker pressed @ 2x

Hi, Brad, one question about the alert screen, will the alert screen be shown when everytime user turn on the speaker, or just only once?

Hi, Brad, I found that the alert messages in your spec is different from the existing messages which is: Plug in a headset -------------------- FM Radio requires a plugged in headset to receive radio signals. should I just replace the text with what you proposed in your spec?

Randy said in bug 854753 comment 41: But some point may need UA to handle at this point. 1. FM may need to turn-off the speaker_on while stop it. 2. FM paused by audio channel, then music sound will output on speaker. should speaker be turned off when headset unplugged? If yes, then should we turn on the speaker back if headset plugged in back?

Hi Pin... It appears there's been a disconnect on this one as Brad is no longer working with Mozilla. As a result, we've missed these flags. Sorry about that. The most recent spec, along with exception handling is located in box.com here: https://mozilla.box.com/s/6yiq70tfptzadbvenade Unfortunately the preview window makes it difficult to read so I recommend downloading the file to view it. Since you didn't have the spec, it's unlikely the exception handling matches so I propose we meet early next week to discuss this and how to mitigate any issues within our timeframe. Are you in Oslo for the work week? Rob

Hi, Rob, I will check the new spec. I won't be Oslo next week, but we could have con-call if needed.

Let me paste the exceptions listed in the spec here for further discussion. = Exceptions = In all cases, the precondition is that the headphones are plugged in and the FM Radio speaker out is on. == Incoming call while in FM Radio == FM Radio pauses. Ringer audio comes through the speakers and headset. If the call is rejected, FM Radio resumes playback. If the call is answered, audio comes in through the headset. When the call disconnects, FM Radio audio remains paused. == User tasks away from FM Radio to dialer and makes a call == FM Radio pauses. Dialer audio comes through the headset. The user can tap the speaker button in the dialer to send audio to the speakers. When the call disconnects, FM Radio audio remains paused. == System Sounds == System sounds, including event reminders and incoming messages, play through speakers as they do normally. == User tasks away from FM Radio to Music Player and plays a track == FM Radio is paused when the user initiates Music Player playback. Music Player audio is sent to the headphones. == User tasks away from FM Radio to Video and plays a video == FM Radio is paused when the user initiates Video playback. Video audio is sent to the headphones. User tasks away from FM Radio to a website or game with audio == FM Radio continues playback through the speakers. == Game or website audio also comes through speakers, allowing the user to listen and surf/play at the same time.

Hi, Randy, I don't think the exceptions could be handled in Gaia layer, what do you think? Hi, Rob, could you provide the images listed in comment 1, and provide more detail info about the comment 4?

Hi Pin, Ya, Those cases I think the SpeakerManger should combine the audiochannel behavior, I.e Should automatic turn-off the speaker if this app turn on the speaker but audiochannel services pause the fmradio by other high priority apps. I'm trying to hook something between the audiochannel Services and SpeakerManager and can solve most of those cases.

(In reply to Randy Lin [:rlin] from comment #9) > Hi Pin, > Ya, Those cases I think the SpeakerManger should combine the audiochannel > behavior, I.e > Should automatic turn-off the speaker if this app turn on the speaker but > audiochannel services pause the fmradio by other high priority apps. > I'm trying to hook something between the audiochannel Services and > SpeakerManager and can solve most of those cases. Hi, Randy, thanks for your speedy response, :)

(In reply to Pin Zhang [:pzhang] from comment #8) > Hi, Randy, I don't think the exceptions could be handled in Gaia layer, what > do you think? > > Hi, Rob, could you provide the images listed in comment 1, and provide more > detail info about the comment 4? Rob, ping

Hi Pin... A quick update... Eric Pang is working on the assets for this today. I'll submit additional feedback this afternoon Oslo time. Rob

(In reply to Pin Zhang [:pzhang] from comment #4) > Randy said in bug 854753 comment 41: > > But some point may need UA to handle at this point. > 1. FM may need to turn-off the speaker_on while stop it. > 2. FM paused by audio channel, then music sound will output on speaker. > > should speaker be turned off when headset unplugged? If yes, then should we > turn on the speaker back if headset plugged in back? Q: "should speaker be turned off when headset unplugged" A: If I understand this correctly, in this scenario the FM radio would turn off in any case as no headset/antenna is available. Please flag me if I've misunderstood the question. Q: "If yes, then should we turn the speaker back if the headset is plugged in back?" A: In this case, resume FM radio playback through the headset as if the FM radio is being started for the first time. The user can then tap the speaker icon if they want to playback through the speakers.

(In reply to Rob MacDonald [:robmac] from comment #13) > (In reply to Pin Zhang [:pzhang] from comment #4) > > Randy said in bug 854753 comment 41: > > > > But some point may need UA to handle at this point. > > 1. FM may need to turn-off the speaker_on while stop it. > > 2. FM paused by audio channel, then music sound will output on speaker. > > > > should speaker be turned off when headset unplugged? If yes, then should we > > turn on the speaker back if headset plugged in back? > > Q: "should speaker be turned off when headset unplugged" > A: If I understand this correctly, in this scenario the FM radio would turn > off in any case as no headset/antenna is available. Please flag me if I've > misunderstood the question. > Yes, FM radio would be turn off if the headset is unplugged, but I am not sure if the speaker could be turned off automatically in Gecko layer, need Randy for more information. Hi Randy, should I turn off the speaker through SpeakerManager in Gaia explicitly, or it will be handled in Gecko layer? > Q: "If yes, then should we turn the speaker back if the headset is plugged > in back?" > A: In this case, resume FM radio playback through the headset as if the FM > radio is being started for the first time. The user can then tap the speaker > icon if they want to playback through the speakers. OK

Q: "should speaker be turned off when headset unplugged" If FM turn off, on gecko side, it would invoke audio channel to stop playing and also trigger SpeakerManager to turn off speaker. On the other hand, if SpeakerManager object's inner setting is on, then User turn on FMRadio, the speaker would set to be on. BTW, this behaviour require this bug's patch. Bug 862899 - AudioChannelAgent should be per process in the FMRadio

(In reply to Randy Lin [:rlin] from comment #15) > Q: "should speaker be turned off when headset unplugged" > If FM turn off, on gecko side, it would invoke audio channel to stop playing > and also trigger SpeakerManager to turn off speaker. > On the other hand, if SpeakerManager object's inner setting is on, then User > turn on FMRadio, the speaker would set to be on. > BTW, this behaviour require this bug's patch. > Bug 862899 - AudioChannelAgent should be per process in the FMRadio OK, then I wouldn't turn off the speaker from Gaia, thanks.

Hi Pin, I've attached the icons. Please let me know if anything else is needed. Thanks!

(In reply to Eric Pang [:epang] from comment #17) > Created attachment 802344 [details] > speaker icons.zip > > Hi Pin, I've attached the icons. Please let me know if anything else is > needed. Thanks! Hi Eric, thanks for your icons, and please help to check if the speaker button looks OK: http://pinzhang.github.io/gaia/apps/fm/

(In reply to Pin Zhang [:pzhang] from comment #18) > (In reply to Eric Pang [:epang] from comment #17) > > Created attachment 802344 [details] > > speaker icons.zip > > > > Hi Pin, I've attached the icons. Please let me know if anything else is > > needed. Thanks! > > Hi Eric, thanks for your icons, and please help to check if the speaker > button looks OK: > http://pinzhang.github.io/gaia/apps/fm/ Hi Pin, The speaker icon looks good, thanks! The favorite icon doesn't seem right tho - is this just with the link or does it need to be fixed?

(In reply to Eric Pang [:epang] from comment #19) > Hi Pin, > > The speaker icon looks good, thanks! The favorite icon doesn't seem right > tho - is this just with the link or does it need to be fixed? The code is almost the latest, I'm afraid it need to be fixed if you really think it's a problem, so please file a separate bug for that.

Hi Randy, please help to check if the permission settings is correct.

Sorry I just changed the name from SpeakerManger to MozSpeakerManager. So you may need to change the interface name. >>>>var speakerManager = new SpeakerManager();

Hi Pin, I've opened this bug for the issue 915082

(In reply to Randy Lin [:rlin] from comment #22) > Sorry I just changed the name from SpeakerManger to MozSpeakerManager. > So you may need to change the interface name. > >>>>var speakerManager = new SpeakerManager(); Hi Randy, it has been fixed and PR is updated, please check it again, thanks.

Comment on attachment 802913 [details] PR 12111 LGTM. I don't have the r+ permission for this part, so I change to fb+.

(In reply to Randy Lin [:rlin] from comment #25) > Comment on attachment 802913 [details] > PR 12111 > > LGTM. > I don't have the r+ permission for this part, so I change to fb+. OK, thanks. Hi @Tim, could you please help to check the revised version for comment 22?

Looks good. ni? me if you need me to merge your pull request.

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) from comment #27) > Looks good. ni? me if you need me to merge your pull request. OK, I will ping you if bug 854753 is landed.

Nominate it to v1.3 as the targeted 1.3 feature

Plus this bug for v1.3 since it is committed one

Pin - bug 854753 just landed. What's left here that's needed in order to land this?

(In reply to Jason Smith [:jsmith] from comment #31) > Pin - bug 854753 just landed. What's left here that's needed in order to > land this? Thanks for your reminding, actually, I am working on this right now, since the last patch is based on Randy's WIP patch, and the interface has changed. I will update my patch and ask for another review in minutes.

Comment on attachment 802913 [details] PR 12111 Hi Tim, I have updated my patch and rebased it on top of master, please help review it again, thanks.

(In reply to Pin Zhang [:pzhang] from comment #33) > Comment on attachment 802913 [details] > PR 12111 > > Hi Tim, I have updated my patch and rebased it on top of master, please help > review it again, thanks. Note - we should get UX to review this as well. Can you flag ui-review for the UX lead on the FM Radio app to do a ui review of your patch?

Comment on attachment 802913 [details] PR 12111 :fxosux will be in Taipei next week -- we might have to wait until then.

Moving to non-blocking - this is a targeted feature, not a committed feature, so we don't need to block on this. However - we should finish this off, as the gecko API is finished & gaia work is complete outside of a UI review.

(In reply to Pin Zhang [:pzhang] from comment #33) > Comment on attachment 802913 [details] > PR 12111 > > Hi Tim, I have updated my patch and rebased it on top of master, please help > review it again, thanks. Hi Pin... Could you please package this app as per James Burke's instructions? https://github.com/jrburke/gaia-dev-zip? I haven't had much luck in applying patches from github so this really helps. Thanks! Rob

(In reply to Rob MacDonald [:robmac] from comment #37) > Hi Pin... > > Could you please package this app as per James Burke's instructions? > https://github.com/jrburke/gaia-dev-zip? I haven't had much luck in applying > patches from github so this really helps. > > Thanks! > Rob Hi Rob, I packaged the FM app as a zip file by following JB's instructions, please check if it works for you, thanks.

(In reply to Pin Zhang [:pzhang] from comment #39) > > Hi Rob, I packaged the FM app as a zip file by following JB's instructions, > please check if it works for you, thanks. Hi Pin - I'll review this on Wednesday and send my feedback. Thanks!

Hi Pin - Apologies but I'm having trouble applying the patch and getting the audio to work. I'll contact you directly about meeting to review this tomorrow over Skype. - Rob

Hi Rob, if you can't apply the patch, you can add my repository as one of your remote, and switch to the branch from which I sent the PR: $ cd $GAIA_HOME $ git remote add pzhang https://github.com/PinZhang/gaia.git $ git fetch pzhang $ git checkout pzhang/bug912317_add_speaker $ adb reset-gaia

(In reply to Pin Zhang [:pzhang] from comment #44) > Created attachment 8343557 [details] > speaker-on.png Hi Rob, please check if the two screenshots look good to you, thanks.

(In reply to Pin Zhang [:pzhang] from comment #45) > (In reply to Pin Zhang [:pzhang] from comment #44) > > Created attachment 8343557 [details] > > speaker-on.png > > Hi Rob, please check if the two screenshots look good to you, thanks. Hi Pin - Eric from visual design has okay'd the screenshots/visuals. William has kindly offered to set up a device for us which we'll review early next week to verify exception handling. - Rob

(In reply to Rob MacDonald [:robmac] from comment #46) > (In reply to Pin Zhang [:pzhang] from comment #45) > > (In reply to Pin Zhang [:pzhang] from comment #44) > > > Created attachment 8343557 [details] > > > speaker-on.png > > > > Hi Rob, please check if the two screenshots look good to you, thanks. > > Hi Pin - Eric from visual design has okay'd the screenshots/visuals. William > has kindly offered to set up a device for us which we'll review early next > week to verify exception handling. - Rob Hi Rob, please give ui-review+ for it. The exception handling should be handled in SpeakerManager API (bug854753), if there is any problem, we need to file new bug to handle it, so let's land this.

Comment on attachment 802913 [details] PR 12111 Plusing based on visual design only. UX will review the exceptions next week and create any new bugs if necessary. Thanks!

Needinfo Fabrice for the approval to land this feature in sprint 6. This is the FM radio app UI change to add on switch button to allow user to listen to radio with speaker.

(In reply to Ivan Tsay (:ITsay) from comment #49) > Needinfo Fabrice for the approval to land this feature in sprint 6. > > This is the FM radio app UI change to add on switch button to allow user to > listen to radio with speaker. I'm also adding needinfo to John to see if he can land this as well.

(In reply to Ivan Tsay (:ITsay) from comment #49) > Needinfo Fabrice for the approval to land this feature in sprint 6. > > This is the FM radio app UI change to add on switch button to allow user to > listen to radio with speaker. Thanks, feel free to merge.

master: ccfdedc

Initial test pass finished on this - the gaia portion looks good to me. The followups that were found testing this were found in the gecko portion of the code, not the Gaia portion.