Closed Bug 960961 Opened 6 years ago Closed 6 years ago

B2G RIL: fix and suppress some unnecessary ril logs

Categories

(Firefox OS Graveyard :: RIL, defect)

x86_64
Linux
defect
Not set

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)

Add back function names in |RIL[FOO] = function FOO() {...}|, or the following log will not show properly: 'Handling parcel as FOO'.
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
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.
Modify dom/mobileconnection/src/MobileConnection.cpp instead of dom/network/src/MobileConnection.cpp.
Attachment #8361598 - Attachment is obsolete: true
Attachment #8361597 - Flags: review?(vyang)
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)
(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.
(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.
Add r=vicamo.
Attachment #8361597 - Attachment is obsolete: true
Attachment #8362821 - Flags: review+
- 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)
(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.
- Add logs as requested in comment 6.
- Also add the same logs in 'deactivateDataCallByType()'.
- Add r=vicamo.
Attachment #8362827 - Flags: review?(vyang)
Assignee: nobody → jjong
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+
Attachment #8362827 - Attachment description: Part 3: add logs for setup/deactivate data call, v1. → Part 3: add logs for setup/deactivate data call (final)
Address review comment 16.
Attachment #8362823 - Attachment is obsolete: true
Attachment #8363546 - Flags: review+
You need to log in before you can comment on or make changes to this bug.