Closed Bug 834551 Opened 7 years ago Closed 7 years ago

[Bluetooth] Add a debug flag for runtime switching on/off logging

Categories

(Firefox OS Graveyard :: General, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(firefox21 fixed, b2g18+ fixed, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 wontfix)

RESOLVED FIXED
Tracking Status
firefox21 --- fixed
b2g18 + fixed
b2g18-v1.0.0 --- wontfix
b2g18-v1.0.1 --- wontfix

People

(Reporter: echou, Assigned: echou)

Details

Attachments

(3 files, 3 obsolete files)

Recently we've got many Bluetooth bugs reported by users who use release build. They usually would try to provide information for us as much as possible. However, we can hardly figure out what the root cause is from adb logcat because we didn't leave enough debugging information in our code. On the other hand, if not necessary, we prefer not printing out logs too often. Otherwise Bluetooth logs may occupy the whole logcat.

So I added a debug flag which could be switched by modifying a key of mozSettings "bluetooth.debugging.enabled" and added a BT_LOG() macro which would check the debug flag to decide if the log should be printed out. In this case, with adding a checkbox in Gaia (settings/Device Information/More information/Developer), we will be able to switch the runtime logging on/off easily.
Assignee: nobody → echou
Attachment #706206 - Flags: review?(kyle)
* fix a typo
Attachment #706206 - Attachment is obsolete: true
Attachment #706206 - Flags: review?(kyle)
Attachment #706216 - Flags: review?(kyle)
* Replace 'LOG()' with 'BT_WARNING()' defined in BluetoothCommon.h for consistency
Attachment #706223 - Flags: review?(kyle)
Attachment #706216 - Flags: review?(kyle) → review+
Attachment #706223 - Flags: review?(kyle) → review+
* removed code for test
Attachment #706993 - Attachment is obsolete: true
Attachment #706993 - Flags: review?(gyeh)
Attachment #706994 - Flags: review?(gyeh)
* added more check points according to Gina's suggestion
Attachment #706994 - Attachment is obsolete: true
Attachment #706994 - Flags: review?(gyeh)
Attachment #706997 - Flags: review?(gyeh)
Comment on attachment 706997 [details] [diff] [review]
patch 3: v3: Inserted logs at some important check points

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

Looks good, r=me.
Attachment #706997 - Flags: review?(gyeh) → review+
oops, sorry, my bad.
We usually got issues which are reported from partners without useful log. That's because we didn't print any log for bluetooth in release mode. This bug resolved the problem of lack of debugging info. Since it should not be a blocker of any projects, nominate as tracking-b2g18+ to see if we could have this landed on b2g18.
tracking-b2g18: --- → ?
Comment on attachment 706216 [details] [diff] [review]
patch 1: v1: add Bluetooth debug flag and BT_LOG() macro

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): no
User impact if declined: Lack of debugging info of Bluetooth
Testing completed: mozilla-central
Risk to taking this patch (and alternatives if risky): no
String or UUID changes made by this patch: no

Please notice that it may not be able to merged into b2g18 directly, so I'll provide b2g18-specific patch once we get the approval.
Attachment #706216 - Flags: approval-mozilla-b2g18?
Comment on attachment 706223 [details] [diff] [review]
patch 2: v1: Replace 'LOG()' with 'BT_WARNING()'

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): no
User impact if declined: Lack of debugging info of Bluetooth
Testing completed: mozilla-central
Risk to taking this patch (and alternatives if risky): no
String or UUID changes made by this patch: no

Please notice that it may not be able to merged into b2g18 directly, so I'll provide b2g18-specific patch once we get the approval.
Attachment #706223 - Flags: approval-mozilla-b2g18?
Comment on attachment 706997 [details] [diff] [review]
patch 3: v3: Inserted logs at some important check points

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): no
User impact if declined: Lack of debugging info of Bluetooth
Testing completed: mozilla-central
Risk to taking this patch (and alternatives if risky): no
String or UUID changes made by this patch: no

Please notice that it may not be able to merged into b2g18 directly, so I'll provide b2g18-specific patch once we get the approval.
Attachment #706997 - Flags: approval-mozilla-b2g18?
So is the plan here to ask partners to enable this flag in their builds?  It's not going to be on by default for any release builds, right? Will someone be doing outreach with partners to let them know about this and to get them to enable it?
Flags: needinfo?(echou)
(In reply to lsblakk@mozilla.com from comment #19)
> So is the plan here to ask partners to enable this flag in their builds? 
> It's not going to be on by default for any release builds, right? 
> Will someone be doing outreach with partners to let them know about this
> and to get them to enable it?

The original idea came after bug 806611, we want to have the same strategy as WiFi. I don't think we should enable this flag by default for release builds. As for partners' builds, I'll talk to PM team, making sure our partners will test Bluetooth with this flag enabled.
Flags: needinfo?(echou)
ni? Joe Cheng, PM of Firefox OS

Hi Joe,

Could you help with telling our partners about the change after this lands on b2g18? Thanks.
Flags: needinfo?(jcheng)
sure, added other OEM facing PMs to this bug to be aware of this.
Flags: needinfo?(jcheng)
Comment on attachment 706216 [details] [diff] [review]
patch 1: v1: add Bluetooth debug flag and BT_LOG() macro

Approving since this has been on m-c for a long time and will be useful to partners' builds.
Attachment #706216 - Flags: approval-mozilla-b2g18? → approval-mozilla-b2g18+
Attachment #706223 - Flags: approval-mozilla-b2g18? → approval-mozilla-b2g18+
Attachment #706997 - Flags: approval-mozilla-b2g18? → approval-mozilla-b2g18+
You need to log in before you can comment on or make changes to this bug.