Closed Bug 1180555 Opened 8 years ago Closed 8 years ago

[Bluetooth] Handle PBAP replies from Gaia and pass the results to PbapManager.

Categories

(Firefox OS Graveyard :: Bluetooth, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(feature-b2g:2.2r+, firefox43 fixed, b2g-v2.2r fixed)

RESOLVED FIXED
FxOS-S6 (04Sep)
feature-b2g 2.2r+
Tracking Status
firefox43 --- fixed
b2g-v2.2r --- fixed

People

(Reporter: jaliu, Assigned: jaliu)

References

Details

Attachments

(2 files, 5 obsolete files)

Handle PBAP replys from Gaia and pass the results to PbapManager, the PBAP reply includes replyTovCardPulling, replyToPhonebookPulling and replyTovCardListing.

This bug should also support vCardSelecting.
Assignee: nobody → jaliu
Status: NEW → ASSIGNED
Blocks: 892179
Summary: [Bluetooth] Handle PBAP replys from Gaia and pass the results to PbapManager. → [Bluetooth] Handle PBAP replies from Gaia and pass the results to PbapManager.
Attachment #8630940 - Flags: review?(btian)
Comment on attachment 8630940 [details] [diff] [review]
Handle PBAP replys and pass the results through IPC to PbapManager. (v1)

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

Please see my comment below.

::: dom/bluetooth/bluedroid/BluetoothPbapManager.cpp
@@ +753,5 @@
>    return propSelector;
>  }
>  
>  void
> +BluetoothPbapManager::ReplyToPullPhonebook(BlobParent* aActor, uint16_t aPhonebookSize)

nit: this line exceeds 80 characters. Please check all whole for too long lines.

@@ +766,4 @@
>  BluetoothPbapManager::ReplyToPullPhonebook(Blob* aBlob, uint16_t aPhonebookSize)
>  {
>    // TODO: Implement this function (Bug 1180556)
> +void

I think here results in compilation error. Can you confirm it?

::: dom/bluetooth/bluedroid/BluetoothPbapManager.h
@@ +62,5 @@
>    static const int MAX_PACKET_LENGTH = 0xFFFE;
>  
>    static BluetoothPbapManager* Get();
>    bool Listen();
> +  void ReplyToPullPhonebook(BlobParent* aActor, uint16_t aPhonebookSize);

Leave comment to state the different usages.

::: dom/bluetooth/bluedroid/BluetoothServiceBluedroid.cpp
@@ +1892,5 @@
> +  if (!pbap) {
> +    DispatchReplyError(aRunnable,
> +                       NS_LITERAL_STRING("Replay to vCardPulling failed"));
> +    return;
> +  } else {

Use guard clause instead of nested condition, here and following.

  if (!pbap) {
    ...
    return;
  }

  pbap->ReplyToPullvCardEntry(aBlobParent);

::: dom/bluetooth/bluetooth1/BluetoothAdapter.cpp
@@ +647,5 @@
>    }
>  
> +  nsRefPtr<BluetoothPbapRequestHandle> handle =
> +    BluetoothPbapRequestHandle::Create(GetOwner());
> +  init.mHandle = handle;

Assign directly, here and following

  init.mHandle = BluetoothPbapRequestHandle::Create(GetOwner());
Attachment #8630940 - Flags: review?(btian)
- revise previous patch based on #comment 3.

Thank Ben for the comments.
Attachment #8630940 - Attachment is obsolete: true
Attachment #8635244 - Flags: review?(btian)
Comment on attachment 8635244 [details] [diff] [review]
Handle PBAP replys and pass the results through IPC to PbapManager. (v2)

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

r=me with comment addressed.

::: dom/bluetooth/bluedroid/BluetoothPbapManager.h
@@ +64,5 @@
>    static BluetoothPbapManager* Get();
>    bool Listen();
> +
> +  /**
> +   * Reply vCard object to the *IPC* 'pullphonebook' request .

nit: remove extra space before period.

@@ +66,5 @@
> +
> +  /**
> +   * Reply vCard object to the *IPC* 'pullphonebook' request .
> +   *
> +   * @param aActor [in] a blob actor contained the vCard objects

nit: 'containing', here and following.

@@ +67,5 @@
> +  /**
> +   * Reply vCard object to the *IPC* 'pullphonebook' request .
> +   *
> +   * @param aActor [in] a blob actor contained the vCard objects
> +   * @param aPhonebookSize [in]  the number of vCard indexes in the blob.

nit: remove the period. Also please align these two lines, here and following.

   * @param aActor [in]          a blob actor containing the vCard objects
   * @param aPhonebookSize [in]  the number of vCard indexes in the blob

@@ +97,4 @@
>    void ReplyToPullvCardListing(Blob* aBlob, uint16_t aPhonebookSize);
> +
> +  /**
> +   * Reply vCard object to the *IPC* 'pullvcardentry' request .

nit: remove extra space before period.

::: dom/bluetooth/bluedroid/BluetoothServiceBluedroid.cpp
@@ +1890,5 @@
> +{
> +  BluetoothPbapManager* pbap = BluetoothPbapManager::Get();
> +  if (!pbap) {
> +    DispatchReplyError(aRunnable,
> +                       NS_LITERAL_STRING("Replay to vCardPulling failed"));

nit: 'Reply', here and following.

@@ +1939,5 @@
> +  uint16_t aPhonebookSize,
> +  BluetoothReplyRunnable* aRunnable)
> +{
> +  BluetoothPbapManager* pbap = BluetoothPbapManager::Get();
> +  if (!pbap) {

Guard clause please.
Attachment #8635244 - Flags: review?(btian) → review+
- revise previous patch based on #comment 5

Thank Ben for reviewing the patch.
Attachment #8635244 - Attachment is obsolete: true
Attachment #8637045 - Attachment description: Handle PBAP replys and pass the results through IPC to PbapManager. (v3) → Handle PBAP replys and pass the results through IPC to PbapManager. (v3), r=btian
- Rebase
- Add few empty functions to BluetoothDBusService to solve the compile errors of ICS emulator.
Attachment #8647909 - Attachment is obsolete: true
Mark feature-b2g: 2.2r+ since PBAP is required bluetooth feature.
feature-b2g: --- → 2.2r+
Backed out 3 changesets (131251625ee8 for bug 1180556, 5bdcc058e6d6 for bug 1180555, f7e0cd74c082 for bug 1180554) for B"G ICS Emulator opt M8 and debug M19 failures. r=backout
https://hg.mozilla.org/integration/b2g-inbound/rev/00b847293e70
checkin-needed to land 2.2r patch in comment 15.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/3c598d855e33
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → FxOS-S6 (04Sep)
You need to log in before you can comment on or make changes to this bug.