1179141 - [cleanup] Remove macro |BT_API2_LOGR|

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Bluetooth, defect)

Description

[Ben Tian (inactive)]

Remove macro |BT_API2_LOGR| as we already switched to bluetooth2 on m-c.

Comment 1

[Ben Tian (inactive)]

Attachment: Patch 1 (v1): Remove macro |BT_API2_LOGR|

Also remove redundant logs.

Comment 2

[Jocelyn Liu [:jocelyn] [:joliu]]

Comment on attachment 8628135 [details] [diff] [review]
Patch 1 (v1): Remove macro |BT_API2_LOGR|

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

r=me after comments addressed, thanks for the cleanup.

::: dom/bluetooth/bluedroid/BluetoothGattManager.cpp
@@ +1354,5 @@
>  BluetoothGattManager::RegisterClientNotification(BluetoothGattStatus aStatus,
>                                                   int aClientIf,
>                                                   const BluetoothUuid& aAppUuid)
>  {
> +  BT_LOGR("Client Registered, clientIf = %d", aClientIf);

I think it can be removed, since we can see the similar log by enabling stack log.

@@ +1370,5 @@
>    NS_ENSURE_TRUE_VOID(bs);
>  
>    if (aStatus != GATT_STATUS_SUCCESS) {
> +    BT_LOGR("RegisterClient failed, clientIf = %d, status = %d, appUuid = %s",
> +            aClientIf, aStatus, NS_ConvertUTF16toUTF8(uuid).get());

nit: How about using BT_LOGD for these failed operations?

@@ +1448,5 @@
>                                            BluetoothGattStatus aStatus,
>                                            int aClientIf,
>                                            const nsAString& aDeviceAddr)
>  {
> +  BT_LOGR();

I think it's redundant, what do you think?

@@ +1462,5 @@
>    nsRefPtr<BluetoothGattClient> client = sClients->ElementAt(index);
>  
>    if (aStatus != GATT_STATUS_SUCCESS) {
> +    BT_LOGR("Connect failed, clientIf = %d, connId = %d, status = %d",
> +            aClientIf, aConnId, aStatus);

DITTO.

@@ +1501,5 @@
>                                               BluetoothGattStatus aStatus,
>                                               int aClientIf,
>                                               const nsAString& aDeviceAddr)
>  {
> +  BT_LOGR();

DITTO.

@@ +2028,5 @@
>                                                   const nsAString& aBdAddr,
>                                                   int aRssi,
>                                                   BluetoothGattStatus aStatus)
>  {
> +  BT_LOGR();

DITTO.

@@ +2043,5 @@
>  
>    if (aStatus != GATT_STATUS_SUCCESS) { // operation failed
> +    BT_LOGR("ReadRemoteRssi failed, clientIf = %d, bdAddr = %s, rssi = %d, " \
> +            "status = %d", aClientIf, NS_ConvertUTF16toUTF8(aBdAddr).get(),
> +            aRssi, (int)aStatus);

DITTO.

::: dom/bluetooth/bluetooth2/BluetoothReplyRunnable.cpp
@@ +24,5 @@
>    , mErrorStatus(STATUS_FAIL)
>    , mName(aName)
>  {
>    if (aPromise) {
> +    BT_LOGR("<%s>", NS_ConvertUTF16toUTF8(mName).get());

Do we need to have these logs here and below?
I would suggest to remove them since they looked a bit of redundant to me.

Comment 3

[Ben Tian (inactive)]

Attachment: [final] Patch 1: Remove macro |BT_API2_LOGR|, r=joliu

Revise based on reviewer's comment.

Comment 4

[Ben Tian (inactive)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=906243614360

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/b2g-inbound/rev/2c12938d0840

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/2c12938d0840