Closed Bug 914076 Opened 6 years ago Closed 6 years ago

[B2G] [Bluetooth] Notification is missed after pausing the music

Categories

(Firefox OS Graveyard :: Bluetooth, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(blocking-b2g:koi+, firefox26 fixed)

RESOLVED FIXED
blocking-b2g koi+
Tracking Status
firefox26 --- fixed

People

(Reporter: gyeh, Assigned: gyeh)

References

Details

Attachments

(1 file, 1 obsolete file)

No description provided.
blocking-b2g: --- → koi?
blocking-b2g: koi? → koi+
Since the event wasn't correctly sent, the remote device like Carkit would fail to show correct play status.
Hi Eric, please take a look at this patch. Thanks.
Assignee: nobody → gyeh
Attachment #801537 - Flags: review?(echou)
Comment on attachment 801537 [details] [diff] [review]
Patch 1(v1): Send play status change event after pausing music

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

Looks good. r=me with nits picked.

::: dom/bluetooth/linux/BluetoothDBusService.cpp
@@ +3189,5 @@
> +  }
> +
> +  if (eventId != ControlEventId::EVENT_UNKNOWN) {
> +    UpdateNotification(eventId, data);
> +  }

We could make this part simpler:

uint32_t tempPlayStatus = playStatus;

if (playStatus != a2dp->GetPlayStatus()) {
  UpdateNotification(ControlEventId::EVENT_PLAYBACK_STATUS_CHANGED, tempPlayStatus);
} else if (aPosition != a2dp->GetPosition()) {
  UpdateNotification(ControlEventId::EVENT_PLAYBACK_POS_CHANGED, aPosition);
}
Attachment #801537 - Flags: review?(echou) → review+
(In reply to Eric Chou [:ericchou] [:echou] from comment #3)
> Comment on attachment 801537 [details] [diff] [review]
> Patch 1(v1): Send play status change event after pausing music
> 
> Review of attachment 801537 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good. r=me with nits picked.
> 
> ::: dom/bluetooth/linux/BluetoothDBusService.cpp
> @@ +3189,5 @@
> > +  }
> > +
> > +  if (eventId != ControlEventId::EVENT_UNKNOWN) {
> > +    UpdateNotification(eventId, data);
> > +  }
> 
> We could make this part simpler:
> 
> uint32_t tempPlayStatus = playStatus;
> 
> if (playStatus != a2dp->GetPlayStatus()) {
>   UpdateNotification(ControlEventId::EVENT_PLAYBACK_STATUS_CHANGED,
> tempPlayStatus);
> } else if (aPosition != a2dp->GetPosition()) {
>   UpdateNotification(ControlEventId::EVENT_PLAYBACK_POS_CHANGED, aPosition);
> }

Sounds good. Thanks!
nit addressed.
Attachment #801537 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/d5802977fc32
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.