Closed
Bug 960961
Opened 10 years ago
Closed 10 years ago
B2G RIL: fix and suppress some unnecessary ril logs
Categories
(Firefox OS Graveyard :: RIL, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
1.3 C3/1.4 S3(31jan)
People
(Reporter: jessica, Assigned: jessica)
References
Details
Attachments
(3 files, 4 obsolete files)
42.71 KB,
patch
|
jessica
:
review+
|
Details | Diff | Splinter Review |
1.92 KB,
patch
|
vicamo
:
review+
|
Details | Diff | Splinter Review |
7.69 KB,
patch
|
jessica
:
review+
|
Details | Diff | Splinter Review |
Add back function names in |RIL[FOO] = function FOO() {...}|, or the following log will not show properly: 'Handling parcel as FOO'.
Assignee | ||
Comment 1•10 years ago
|
||
I will also remove some logs and shouldn't appear in logcat.
Summary: B2G RIL: adding back some function names in ril_worker.js → B2G RIL: fix and suppress some unnecessary ril logs
Assignee | ||
Comment 2•10 years ago
|
||
Assignee | ||
Comment 3•10 years ago
|
||
Removing lastKnown(Home)Network log as they contain sensitive information and MobileConnection log as it appears no matter ril debugging flag is enabled or not.
Assignee | ||
Comment 4•10 years ago
|
||
Try: https://tbpl.mozilla.org/?tree=Try&rev=26e3d9a73f39
Assignee | ||
Comment 5•10 years ago
|
||
Modify dom/mobileconnection/src/MobileConnection.cpp instead of dom/network/src/MobileConnection.cpp.
Attachment #8361598 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Attachment #8361597 -
Flags: review?(vyang)
Assignee | ||
Updated•10 years ago
|
Attachment #8362408 -
Flags: review?(vyang)
Comment 6•10 years ago
|
||
Sorry for jumping in. Could we also take the chance to add some more ril logs as https://bug960537.bugzilla.mozilla.org/attachment.cgi?id=8362404? I feel like these two benefit to debugging data connection setup. :)
Comment 7•10 years ago
|
||
Comment on attachment 8361597 [details] [diff] [review] Part 1: add back function names in ril_worker.js Review of attachment 8361597 [details] [diff] [review]: ----------------------------------------------------------------- Thank you :)
Attachment #8361597 -
Flags: review?(vyang) → review+
Comment 8•10 years ago
|
||
Comment on attachment 8362408 [details] [diff] [review] Part 2: removes logs that shouldn't appear in logcat, v2. Review of attachment 8362408 [details] [diff] [review]: ----------------------------------------------------------------- I think there are still a few unwrapped ones in ril_worker. Could you also help wrapping them with DEBUG? Thank you :)
Attachment #8362408 -
Flags: review?(vyang)
Assignee | ||
Comment 9•10 years ago
|
||
(In reply to Hsin-Yi Tsai [:hsinyi] from comment #6) > Sorry for jumping in. > > Could we also take the chance to add some more ril logs as > https://bug960537.bugzilla.mozilla.org/attachment.cgi?id=8362404? I feel > like these two benefit to debugging data connection setup. :) Sure. I will upload another patch for this.
Assignee | ||
Comment 10•10 years ago
|
||
(In reply to Vicamo Yang [:vicamo][:vyang] from comment #8) > Comment on attachment 8362408 [details] [diff] [review] > Part 2: removes logs that shouldn't appear in logcat, v2. > > Review of attachment 8362408 [details] [diff] [review]: > ----------------------------------------------------------------- > > I think there are still a few unwrapped ones in ril_worker. Could you also > help wrapping them with DEBUG? Thank you :) Sure. I will check RadioInterfaceLayer.js and RILContentHelper.js as well.
Assignee | ||
Comment 11•10 years ago
|
||
Add r=vicamo.
Attachment #8361597 -
Attachment is obsolete: true
Attachment #8362821 -
Flags: review+
Assignee | ||
Comment 12•10 years ago
|
||
- From comment 8, check also ril_worker.js, RadioInterfaceLayer.js and RILContentHelper.js. - Add r=vicamo.
Attachment #8362408 -
Attachment is obsolete: true
Attachment #8362823 -
Flags: review?(vyang)
Assignee | ||
Comment 13•10 years ago
|
||
(In reply to Jessica Jong [:jjong] [:jessica] from comment #12) > Created attachment 8362823 [details] [diff] [review] > Part 2: remove ril logs or wrap them with DEBUG flag, v3. > > - From comment 8, check also ril_worker.js, RadioInterfaceLayer.js and > RILContentHelper.js. > - Add r=vicamo. I am not sure if the debug message in onerror() in ril_worker.js is left without the DEBUG on purpose. Please let me know if it is the case.
Assignee | ||
Comment 14•10 years ago
|
||
- Add logs as requested in comment 6. - Also add the same logs in 'deactivateDataCallByType()'. - Add r=vicamo.
Attachment #8362827 -
Flags: review?(vyang)
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → jjong
Assignee | ||
Comment 15•10 years ago
|
||
Try: https://tbpl.mozilla.org/?tree=Try&rev=bde07f7f6b5c Note: the failure in B2G ICS Emulator Debug (5) is being tracked in Bug 961107.
Updated•10 years ago
|
Attachment #8362827 -
Flags: review?(vyang) → review+
Comment 16•10 years ago
|
||
Comment on attachment 8362823 [details] [diff] [review] Part 2: remove ril logs or wrap them with DEBUG flag, v3. Review of attachment 8362823 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/system/gonk/ril_worker.js @@ +6238,5 @@ > // Always print the NITZ info so we can collection what different providers > // send down the pipe (see bug XXX). > // TODO once data is collected, add in |if (DEBUG)| > > + if (DEBUG) debug("DateTimeZone string " + dateString); nit: please also remove "// Always ..." and add "// See also bug 714352 - Listen for NITZ updates from rild." @@ +11481,5 @@ > if (options.sw1 && options.sw2) { > errorMsg += "(" + options.sw1.toString(16) + > "/" + options.sw2.toString(16) + ")"; > } > + if (DEBUG) debug(errorMsg); nit: this has been wrapped by DEBUG at line 11475.
Attachment #8362823 -
Flags: review?(vyang) → review+
Assignee | ||
Updated•10 years ago
|
Attachment #8362827 -
Attachment description: Part 3: add logs for setup/deactivate data call, v1. → Part 3: add logs for setup/deactivate data call (final)
Assignee | ||
Comment 17•10 years ago
|
||
Address review comment 16.
Attachment #8362823 -
Attachment is obsolete: true
Attachment #8363546 -
Flags: review+
Assignee | ||
Comment 18•10 years ago
|
||
Try: https://tbpl.mozilla.org/?tree=Try&rev=8aaf1a17af0b
Keywords: checkin-needed
Comment 19•10 years ago
|
||
https://hg.mozilla.org/integration/b2g-inbound/rev/98f0077e1820 https://hg.mozilla.org/integration/b2g-inbound/rev/356ce1bba953 https://hg.mozilla.org/integration/b2g-inbound/rev/d32b34edd752
Keywords: checkin-needed
Comment 20•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/98f0077e1820 https://hg.mozilla.org/mozilla-central/rev/356ce1bba953 https://hg.mozilla.org/mozilla-central/rev/d32b34edd752
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.3 C3/1.4 S3(31jan)
You need to log in
before you can comment on or make changes to this bug.
Description
•