1111976 - Enable/Disable Bluetooth Snoop log from Settings Developer Menu debugging options

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: Bluetooth, defect)

Description

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Enable/Disable Bluetooth Snoop log from Settings

Comment 2

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

This bug is public and general bug, not only 2.0m+ bug.

Comment 3

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Attachment: 0001-Bug-1111976-Config-Bluetooth-snoop-log-from-Develope.patch

Comment 4

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Attachment: 0001-Bug-1111976-Config-Bluetooth-snoop-log-from-Develope.patch

Comment 5

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Attachment: [2.0m] Bug 1111976 - Config Bluetooth snoop log from Developer menu

Comment 6

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Attachment: [2.0m] Bug 1111976 - Config Bluetooth snoop log from Developer menu

Comment 7

[Ben Tian (inactive)]

Comment on attachment 8540002 [details] [diff] [review]
[2.0m] Bug 1111976 - Config Bluetooth snoop log from Developer menu

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

r=me with comment addressed.

::: dom/bluetooth/bluedroid/BluetoothServiceBluedroid.cpp
@@ +1422,5 @@
>    return sBtInterface;
>  }
>  
>  void
> +BluetoothServiceBluedroid::ConfigHciDumpLog(bool aEnabled) {

nit: newline for {

@@ +1423,5 @@
>  }
>  
>  void
> +BluetoothServiceBluedroid::ConfigHciDumpLog(bool aEnabled) {
> +  if (sBtInterface) {

Rewrite with guardian clause to reduce nested if:

  if (!sBtInterface) {
    // print log/warning msg
  }

  int ret = sBtInterface->config_hci_snoop_log(aEnabled);
  if (ret != BT_STATUS_SUCCESS) {
    BT_LOGR("Fail to ConfigHciDumpLog");
  }

@@ +1426,5 @@
> +BluetoothServiceBluedroid::ConfigHciDumpLog(bool aEnabled) {
> +  if (sBtInterface) {
> +    int ret = sBtInterface->config_hci_snoop_log(aEnabled);
> +    if (ret != BT_STATUS_SUCCESS) {
> +      BT_LOGR("Fail to ConfigHciDumpLog");

Print |aEnabled|.

::: dom/bluetooth/bluez/BluetoothDBusService.cpp
@@ +2163,5 @@
>  
> +void
> +BluetoothDBusService::ConfigHciDumpLog(bool aEnabled)
> +{
> +  // Only avaiable for BT HAL interface

typo: avai'l'able

::: dom/bluetooth/bluez/BluetoothDBusService.h
@@ +179,5 @@
>    virtual nsresult
>    SendInputMessage(const nsAString& aDeviceAddresses,
>                     const nsAString& aMessage) MOZ_OVERRIDE;
> +
> +  virtual void ConfigHciDumpLog(bool aEnabled) MOZ_OVERRIDE;

nit: newline after this line.

::: dom/bluetooth/ipc/BluetoothServiceChildProcess.cpp
@@ +389,5 @@
>    MOZ_CRASH("This should never be called!");
>  }
>  
> +void
> +BluetoothServiceChildProcess::ConfigHciDumpLog(bool aEnabled) {

nit: newline for {

Comment 8

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Attachment: [2.0m] Bug 1111976 - Config Bluetooth snoop log from Developer menu, r=btian

Comment 9

[Kai-Zhen Li [:seinlin][:kli]]

https://hg.mozilla.org/releases/mozilla-b2g32_v2_0m/rev/4a96589bf6b6

Comment 10

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

m-c now is going to change a lot due to Bluetooth Daemon will be introduced in these two weeks, so i preferred to wait for that big patch get landed.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Sounds like it can land on v2.1 in the mean time? If so, please request approval for that at least.

Comment 12

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

v2.1 also uses the new design, so I guess I will handle it for m-c only.

Comment 13

[Josh Cheng [:josh]]

Leave 2.1 decision to Steven.

Comment 14

[Steven Yang [:styang]]

Need it for 2.1S

Comment 15

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Comment on attachment 8540028 [details] [diff] [review]
[2.0m] Bug 1111976 - Config Bluetooth snoop log from Developer menu, r=btian

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

::: dom/bluetooth/BluetoothService.h
@@ +359,5 @@
>    /**
> +   * Enable/disable snoop log
> +   */
> +  virtual void
> +  ConfigHciDumpLog(bool aEnabled) = 0;

I'd like to suggest to call this method |ConfigHciSnoopLog|; here and everywhere else. That would be consistent with Bluedroid terminology.

Comment 16

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

(In reply to Thomas Zimmermann [:tzimmermann] [:tdz] from comment #15)
> Comment on attachment 8540028 [details] [diff] [review]
> [2.0m] Bug 1111976 - Config Bluetooth snoop log from Developer menu, r=btian
> 
> Review of attachment 8540028 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/bluetooth/BluetoothService.h
> @@ +359,5 @@
> >    /**
> > +   * Enable/disable snoop log
> > +   */
> > +  virtual void
> > +  ConfigHciDumpLog(bool aEnabled) = 0;
> 
> I'd like to suggest to call this method |ConfigHciSnoopLog|; here and
> everywhere else. That would be consistent with Bluedroid terminology.

Thanks, i will change it for m-c.
We also need to check settings value for 'bluetooth.snoop.enabled' instead of 'bluetooth.debugging.enabled'

Comment 17

[Steven Yang [:styang]]

Shawn, do you still need this for 2.1S?

Comment 18

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

(In reply to Steven Yang [:styang] from comment #17)
> Shawn, do you still need this for 2.1S?

Not really hard requirement now for v2.1+.