Closed Bug 1000755 Opened 5 years ago Closed 5 years ago

(about service) Refine the format of output reports

Categories

(Firefox OS Graveyard :: General, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: alchen, Assigned: alchen)

References

Details

(Whiteboard: [p=1])

Attachments

(3 files, 1 obsolete file)

It is a follow-up bug for bug 964636.
In this bug, we would like to output a report with formal json form.

value :  string | number | object | array | true | false | null
object:  { string:value (,string:value)^n }
array :  [ value (,value)^n ]
Assignee: nobody → alchen
Take these two online parser as a reference.
http://json.parser.online.fr/
http://www.jsoneditoronline.org/

(check '\n' and '\t' case)
original json :  "\tBluetooth ON/OFF: ON\n"
string parse  :  "\tBluetooth ON/OFF: ON\n"
JS eval       :  " Bluetooth ON/OFF: ON "
Hi Dave,
in this patch, I modify the output format to regular json.
We can generate a json which can be parsed by normal json parser.

There is one need to be addressed.
When consumers write their description, they should not write '\n' in it.
Instead, they can use '|' to separate two sentences.
(EX: "Bluetooth ON/OFF: ON | Local device address: E4:2D:02:0C:5B:34")
I will upload a test sample for this.
Attachment #8412464 - Flags: review?(dhylands)
In this patch, we add a reporter for BT service. (bluez version)
The reporter will report some BT parameters such as BT state, address, name.
This is the output based attachment 8412464 [details] [diff] [review] and attachment 8412467 [details] [diff] [review].
We can get the following parsed result by normal json parser.

(parsed result)
{

    "subject":"about:service reports",
    "reporters":[
        {
            "Process":"PID 4906",
            "Reporter name":"StatusReporter State",
            "Status Description":[
                "Running: There are 2 reporters"
            ]
        },
        {
            "Process":"PID 4906",
            "Reporter name":"Bluetooth",
            "Status Description":[
                "Bluetooth ON/OFF: ON",
                "Local device address: E4:2D:02:0C:5B:34",
                "Local device name: hamachi",
                "Visibility: invisible",
                "Discovering: false"
            ]
        }
    ]

}
Trigger way:
adb shell "echo 'status reporter' > debug_info_trigger"

the output file:
(status-reports-$pid-$num.json) will exist in /data/local/tmp/status-reports/
Comment on attachment 8412464 [details] [diff] [review]
(0425) [about:service] Refine the format of output reports

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

::: xpcom/base/nsStatusReporterManager.cpp
@@ +84,4 @@
>  
>  static nsresult
>  DumpReport(nsIFileOutputStream* aOStream, const nsCString& aProcess,
> +           const nsCString& aName, nsCString& aDescription)

Why did you remove the const for aDescription?

I think it would be better to leave aDescription as const and allocate a new string to do the substitution on.

I wouldn't expect a Dump function to be modifying its parameters.

@@ +171,4 @@
>  
>    //Write the reports to the file
>  
> +  DUMP(ostream, "{\n\"subject\":\"about:service reports\",\n");

json supports single quotes for strings right?

It would look cleaner here if you used single quotes rather than \" for writing the quotes.
Attachment #8412464 - Flags: review?(dhylands) → review+
(In reply to Dave Hylands [:dhylands] from comment #6)
> Comment on attachment 8412464 [details] [diff] [review]
> (0425) [about:service] Refine the format of output reports
> 
> Review of attachment 8412464 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: xpcom/base/nsStatusReporterManager.cpp
> @@ +84,4 @@
> >  
> >  static nsresult
> >  DumpReport(nsIFileOutputStream* aOStream, const nsCString& aProcess,
> > +           const nsCString& aName, nsCString& aDescription)
> 
> Why did you remove the const for aDescription?
> 
> I think it would be better to leave aDescription as const and allocate a new
> string to do the substitution on.
> 
> I wouldn't expect a Dump function to be modifying its parameters.
> 
> @@ +171,4 @@
> >  
> >    //Write the reports to the file
> >  
> > +  DUMP(ostream, "{\n\"subject\":\"about:service reports\",\n");
> 
> json supports single quotes for strings right?

I try it on the following website.
Single quote cannot be recognized as a string.
So I keep the original way for string.

Both websites show the same result.
http://www.jsoneditoronline.org/
http://json.parser.online.fr/
> 
> It would look cleaner here if you used single quotes rather than \" for
> writing the quotes.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/ba3c6ac9dfda
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Whiteboard: [p=1]
Blocks: 1131927
You need to log in before you can comment on or make changes to this bug.