B2G RIL: fix and suppress some unnecessary ril logs

RESOLVED FIXED in 1.3 C3/1.4 S3(31jan)

Status

Firefox OS
RIL
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: jessica, Assigned: jessica)

Tracking

unspecified
1.3 C3/1.4 S3(31jan)
x86_64
Linux
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 4 obsolete attachments)

(Assignee)

Description

5 years ago
Add back function names in |RIL[FOO] = function FOO() {...}|, or the following log will not show properly: 'Handling parcel as FOO'.
(Assignee)

Comment 1

5 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

5 years ago
Created attachment 8361597 [details] [diff] [review]
Part 1: add back function names in ril_worker.js
(Assignee)

Comment 3

5 years ago
Created attachment 8361598 [details] [diff] [review]
Part 2: removes logs that shouldn't appear in logcat.

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 5

5 years ago
Created attachment 8362408 [details] [diff] [review]
Part 2: removes logs that shouldn't appear in logcat, v2.

Modify dom/mobileconnection/src/MobileConnection.cpp instead of dom/network/src/MobileConnection.cpp.
Attachment #8361598 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Attachment #8361597 - Flags: review?(vyang)
(Assignee)

Updated

5 years ago
Attachment #8362408 - Flags: review?(vyang)
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. :)
Depends on: 934125
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 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

5 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

5 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

5 years ago
Created attachment 8362821 [details] [diff] [review]
Part 1: add back function names in ril_worker.js (final)

Add r=vicamo.
Attachment #8361597 - Attachment is obsolete: true
Attachment #8362821 - Flags: review+
(Assignee)

Comment 12

5 years ago
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.
Attachment #8362408 - Attachment is obsolete: true
Attachment #8362823 - Flags: review?(vyang)
(Assignee)

Comment 13

5 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

5 years ago
Created attachment 8362827 [details] [diff] [review]
Part 3: add logs for setup/deactivate data call (final)

- Add logs as requested in comment 6.
- Also add the same logs in 'deactivateDataCallByType()'.
- Add r=vicamo.
Attachment #8362827 - Flags: review?(vyang)
(Assignee)

Updated

5 years ago
Assignee: nobody → jjong
(Assignee)

Comment 15

5 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.
Attachment #8362827 - Flags: review?(vyang) → review+
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

5 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

5 years ago
Created attachment 8363546 [details] [diff] [review]
Part 2: remove ril logs or wrap them with DEBUG flag (final)

Address review comment 16.
Attachment #8362823 - Attachment is obsolete: true
Attachment #8363546 - Flags: review+
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
Last Resolved: 5 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.