Last Comment Bug 811605 - B2G RIL: enable ril debugging output in run-time
: B2G RIL: enable ril debugging output in run-time
Status: RESOLVED FIXED
: dev-doc-needed
Product: Core
Classification: Components
Component: DOM: Device Interfaces (show other bugs)
: unspecified
: ARM Gonk (Firefox OS)
: -- normal (vote)
: mozilla19
Assigned To: Edgar Chen [:edgar][:echen] (OOO until 9/29)
:
Mentors:
Depends on:
Blocks: 960926
  Show dependency treegraph
 
Reported: 2012-11-13 19:15 PST by Edgar Chen [:edgar][:echen] (OOO until 9/29)
Modified: 2014-04-16 03:10 PDT (History)
11 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
fixed


Attachments
Bug_811605_patch_V1 (4.68 KB, patch)
2012-11-13 22:46 PST, Edgar Chen [:edgar][:echen] (OOO until 9/29)
vicamo: review+
Details | Diff | Splinter Review
Bug_811605_patch_V2 (3.99 KB, patch)
2012-11-14 02:39 PST, Edgar Chen [:edgar][:echen] (OOO until 9/29)
echen: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description Edgar Chen [:edgar][:echen] (OOO until 9/29) 2012-11-13 19:15:44 PST
Currently, to enable debug output only can be achieved in compile time. To make debug easily, we may need to add some mechanism to enable debug output in run-time.
Comment 1 Vicamo Yang [:vicamo][:vyang] 2012-11-13 22:14:53 PST
We had similar feature in WiFi debugging in bug 806611. Ability to turn on debug messages in runtime can be handy for testing, but we still don't want to allow this through Gaia/Settings. We should instead have a default disabled preference like "ril.debugging.enabled" here.
Comment 2 Edgar Chen [:edgar][:echen] (OOO until 9/29) 2012-11-13 22:46:26 PST
Created attachment 681369 [details] [diff] [review]
Bug_811605_patch_V1
Comment 3 Vicamo Yang [:vicamo][:vyang] 2012-11-14 01:34:07 PST
Comment on attachment 681369 [details] [diff] [review]
Bug_811605_patch_V1

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

Great! Thank you :)

::: dom/system/gonk/RILContentHelper.js
@@ +1063,5 @@
>  };
>  
>  this.NSGetFactory = XPCOMUtils.generateNSGetFactory([RILContentHelper]);
>  
> +// Read debug setting from pref

Please move this segment right below "var DEBUG = ...".

@@ +1070,5 @@
> +  debugPref = Services.prefs.getBoolPref("ril.debugging.enabled");
> +} catch (e) {
> +  debugPref = false;
> +}
> +DEBUG = RIL.DEBUG_CONTENT_HELPER || debugPref;

You can have:

  try {
    let debugPref = Services.prefs.getBoolPref("ril.debugging.enabled");
    DEBUG = RIL.DEBUG_CONTENT_HELPER || debugPref;
  } catch (e) {};

instead, so that it won't insert a useless variable 'debugPref' into global ns after all these.

::: dom/system/gonk/RadioInterfaceLayer.js
@@ +282,5 @@
>    this.portAddressedSmsApps = {};
>    this.portAddressedSmsApps[WAP.WDP_PORT_PUSH] = this.handleSmsWdpPortPush.bind(this);
> +
> +  // pass debug pref to ril_worker
> +  this.worker.postMessage({rilMessageType: "updateDebug",

"setDebugEnabled"

@@ +2622,5 @@
>  };
>  
>  this.NSGetFactory = XPCOMUtils.generateNSGetFactory([RadioInterfaceLayer]);
>  
> +// Read debug setting from pref

Please also move this segment up.
Comment 4 Edgar Chen [:edgar][:echen] (OOO until 9/29) 2012-11-14 02:39:08 PST
Created attachment 681421 [details] [diff] [review]
Bug_811605_patch_V2
Comment 5 Edgar Chen [:edgar][:echen] (OOO until 9/29) 2012-11-14 17:58:35 PST
To enable debug log, you need to add one line, 'pref("ril.debugging.enabled", true);', into /system/b2g/defaults/pref/user.js in device. Then reboot device or restart b2g service.

detailed step:
adb remount
adb pull /system/b2g/defaults/pref/user.js user.js
[update user.js with adding 'pref("ril.debugging.enabled", true);']
adb push user.js /system/b2g/defaults/pref/user.js
adb shell reboot
Comment 6 Ryan VanderMeulen [:RyanVM] 2012-11-14 18:03:22 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/208c1637560b
Comment 7 Shian-Yow Wu [:swu] 2012-11-15 00:41:48 PST
Comment on attachment 681421 [details] [diff] [review]
Bug_811605_patch_V2

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

This is very useful for RIL debugging, asking approval to check into mozilla-aurora.

A similar runtime debugging support for Wifi also checked in to mozilla-aurora(bug 806611).
Comment 8 Ed Morley [:emorley] 2012-11-15 08:24:12 PST
https://hg.mozilla.org/mozilla-central/rev/208c1637560b
Comment 9 Alex Keybl [:akeybl] 2012-11-16 14:30:55 PST
(In reply to Shian-Yow Wu from comment #7)
> Comment on attachment 681421 [details] [diff] [review]
> Bug_811605_patch_V2
> 
> Review of attachment 681421 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> This is very useful for RIL debugging, asking approval to check into
> mozilla-aurora.
> 
> A similar runtime debugging support for Wifi also checked in to
> mozilla-aurora(bug 806611).

Approving for Aurora assuming this will not have any major perf hits for non-debug users.
Comment 10 Ryan VanderMeulen [:RyanVM] 2012-11-16 18:07:02 PST
https://hg.mozilla.org/releases/mozilla-aurora/rev/0d9385d1de1a
Comment 11 Shian-Yow Wu [:swu] 2012-11-19 00:08:57 PST
(In reply to Edgar Chen from comment #5)
> To enable debug log, you need to add one line,
> 'pref("ril.debugging.enabled", true);', into
> /system/b2g/defaults/pref/user.js in device. Then reboot device or restart
> b2g service.
> 
> detailed step:
> adb remount
> adb pull /system/b2g/defaults/pref/user.js user.js
> [update user.js with adding 'pref("ril.debugging.enabled", true);']
> adb push user.js /system/b2g/defaults/pref/user.js
> adb shell reboot

Thanks Edgar!  Would you also send a Gaia pull request to add "ril.debugging.enabled" in user.js with false value by default?  This will be convenient for users to know what prefs values they can change.
Comment 12 Edgar Chen [:edgar][:echen] (OOO until 9/29) 2012-11-20 08:12:11 PST
(In reply to Shian-Yow Wu from comment #11)
> (In reply to Edgar Chen from comment #5)
> > To enable debug log, you need to add one line,
> > 'pref("ril.debugging.enabled", true);', into
> > /system/b2g/defaults/pref/user.js in device. Then reboot device or restart
> > b2g service.
> > 
> > detailed step:
> > adb remount
> > adb pull /system/b2g/defaults/pref/user.js user.js
> > [update user.js with adding 'pref("ril.debugging.enabled", true);']
> > adb push user.js /system/b2g/defaults/pref/user.js
> > adb shell reboot
> 
> Thanks Edgar!  Would you also send a Gaia pull request to add
> "ril.debugging.enabled" in user.js with false value by default?  This will
> be convenient for users to know what prefs values they can change.

Add "ril.debugging.enabled" in user.js with false value by default.
This will be convenient for users to know what prefs values they can change. 

Send a pull request for this.
https://github.com/mozilla-b2g/gaia/pull/6532
Comment 13 Edgar Chen [:edgar][:echen] (OOO until 9/29) 2012-11-21 00:30:23 PST
Merged on Github:
https://github.com/mozilla-b2g/gaia/commit/da8dad4

Note You need to log in before you can comment on or make changes to this bug.