[cleanup] Remove macro |BT_API2_LOGR|

RESOLVED FIXED in Firefox 42

Status

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: ben.tian, Assigned: ben.tian)

Tracking

unspecified
FxOS-S2 (10Jul)
ARM
Gonk (Firefox OS)

Firefox Tracking Flags

(firefox42 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

3 years ago
Remove macro |BT_API2_LOGR| as we already switched to bluetooth2 on m-c.
(Assignee)

Comment 1

3 years ago
Created attachment 8628135 [details] [diff] [review]
Patch 1 (v1): Remove macro |BT_API2_LOGR|

Also remove redundant logs.
Assignee: nobody → btian
Attachment #8628135 - Flags: review?(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.
Attachment #8628135 - Flags: review?(joliu) → review+
(Assignee)

Comment 3

3 years ago
Created attachment 8628683 [details] [diff] [review]
[final] Patch 1: Remove macro |BT_API2_LOGR|, r=joliu

Revise based on reviewer's comment.
Attachment #8628135 - Attachment is obsolete: true
(Assignee)

Updated

3 years ago
Blocks: 1179682
https://hg.mozilla.org/mozilla-central/rev/2c12938d0840
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox42: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → FxOS-S2 (10Jul)
You need to log in before you can comment on or make changes to this bug.