diff options
8 files changed, 166 insertions, 45 deletions
diff --git a/service/java/com/android/server/wifi/FrameworkFacade.java b/service/java/com/android/server/wifi/FrameworkFacade.java index f494a8573..bf731a293 100644 --- a/service/java/com/android/server/wifi/FrameworkFacade.java +++ b/service/java/com/android/server/wifi/FrameworkFacade.java @@ -41,16 +41,6 @@ import android.telephony.CarrierConfigManager; public class FrameworkFacade { public static final String TAG = "FrameworkFacade"; - public BaseWifiDiagnostics makeBaseDiagnostics() { - return new BaseWifiDiagnostics(); - } - - public BaseWifiDiagnostics makeRealDiagnostics( - Context context, WifiStateMachine stateMachine, WifiNative wifiNative, - BuildProperties buildProperties) { - return new WifiDiagnostics(context, stateMachine, wifiNative, buildProperties); - } - public boolean setIntegerSetting(Context context, String name, int def) { return Settings.Global.putInt(context.getContentResolver(), name, def); } diff --git a/service/java/com/android/server/wifi/LogcatLog.java b/service/java/com/android/server/wifi/LogcatLog.java new file mode 100644 index 000000000..1aea03dc1 --- /dev/null +++ b/service/java/com/android/server/wifi/LogcatLog.java @@ -0,0 +1,52 @@ +/* + * Copyright (C) 2016 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server.wifi; + +import android.util.Log; + +import com.android.internal.annotations.Immutable; + +// @ThreadSafe +// This class is trivially thread-safe, as instances are immutable. +@Immutable +class LogcatLog implements WifiLog { + private final String mTag; + + LogcatLog(String tag) { + mTag = tag; + } + + public void e(String msg) { + Log.e(mTag, msg); + } + + public void w(String msg) { + Log.w(mTag, msg); + } + + public void i(String msg) { + Log.i(mTag, msg); + } + + public void d(String msg) { + Log.d(mTag, msg); + } + + public void v(String msg) { + Log.v(mTag, msg); + } +} diff --git a/service/java/com/android/server/wifi/WifiDiagnostics.java b/service/java/com/android/server/wifi/WifiDiagnostics.java index 70e2475e4..cde8fdf8d 100644 --- a/service/java/com/android/server/wifi/WifiDiagnostics.java +++ b/service/java/com/android/server/wifi/WifiDiagnostics.java @@ -106,9 +106,10 @@ class WifiDiagnostics extends BaseWifiDiagnostics { private final WifiNative mWifiNative; private final BuildProperties mBuildProperties; private int mMaxRingBufferSizeBytes; + private WifiLog mLog; - public WifiDiagnostics(Context context, WifiStateMachine wifiStateMachine, WifiNative wifiNative, - BuildProperties buildProperties) { + public WifiDiagnostics(Context context, WifiInjector wifiInjector, WifiStateMachine wifiStateMachine, + WifiNative wifiNative, BuildProperties buildProperties) { RING_BUFFER_BYTE_LIMIT_SMALL = context.getResources().getInteger( R.integer.config_wifi_logger_ring_buffer_default_size_limit_kb) * 1024; RING_BUFFER_BYTE_LIMIT_LARGE = context.getResources().getInteger( @@ -119,6 +120,7 @@ class WifiDiagnostics extends BaseWifiDiagnostics { mBuildProperties = buildProperties; mIsLoggingEventHandlerRegistered = false; mMaxRingBufferSizeBytes = RING_BUFFER_BYTE_LIMIT_SMALL; + mLog = wifiInjector.makeLog(TAG); } @Override @@ -153,7 +155,7 @@ class WifiDiagnostics extends BaseWifiDiagnostics { } if (!mWifiNative.startPktFateMonitoring()) { - Log.e(TAG, "Failed to start packet fate monitoring"); + mLog.e("Failed to start packet fate monitoring"); } } @@ -162,7 +164,7 @@ class WifiDiagnostics extends BaseWifiDiagnostics { if (mPerPacketRingBuffer != null) { startLoggingRingBuffer(mPerPacketRingBuffer); } else { - if (DBG) Log.d(TAG, "There is no per packet ring buffer"); + if (DBG) mLog.d("There is no per packet ring buffer"); } } @@ -171,7 +173,7 @@ class WifiDiagnostics extends BaseWifiDiagnostics { if (mPerPacketRingBuffer != null) { stopLoggingRingBuffer(mPerPacketRingBuffer); } else { - if (DBG) Log.d(TAG, "There is no per packet ring buffer"); + if (DBG) mLog.d("There is no per packet ring buffer"); } } @@ -179,9 +181,9 @@ class WifiDiagnostics extends BaseWifiDiagnostics { public synchronized void stopLogging() { if (mIsLoggingEventHandlerRegistered) { if (!mWifiNative.resetLogHandler()) { - Log.e(TAG, "Fail to reset log handler"); + mLog.e("Fail to reset log handler"); } else { - if (DBG) Log.d(TAG, "Reset log handler"); + if (DBG) mLog.d("Reset log handler"); } // Clear mIsLoggingEventHandlerRegistered even if resetLogHandler() failed, because // the log handler is in an indeterminate state. @@ -413,7 +415,7 @@ class WifiDiagnostics extends BaseWifiDiagnostics { mRingBuffers = mWifiNative.getRingBufferStatus(); if (mRingBuffers != null) { for (WifiNative.RingBufferStatus buffer : mRingBuffers) { - if (DBG) Log.d(TAG, "RingBufferStatus is: \n" + buffer.name); + if (DBG) mLog.d("RingBufferStatus is: \n" + buffer.name); if (mRingBufferData.containsKey(buffer.name) == false) { mRingBufferData.put(buffer.name, new ByteArrayRingBuffer(mMaxRingBufferSizeBytes)); @@ -423,7 +425,7 @@ class WifiDiagnostics extends BaseWifiDiagnostics { } } } else { - Log.e(TAG, "no ring buffers found"); + mLog.e("no ring buffers found"); } return mRingBuffers != null; @@ -438,7 +440,7 @@ class WifiDiagnostics extends BaseWifiDiagnostics { private boolean startLoggingAllExceptPerPacketBuffers() { if (mRingBuffers == null) { - if (DBG) Log.d(TAG, "No ring buffers to log anything!"); + if (DBG) mLog.d("No ring buffers to log anything!"); return false; } @@ -446,7 +448,7 @@ class WifiDiagnostics extends BaseWifiDiagnostics { if ((buffer.flag & RING_BUFFER_FLAG_HAS_PER_PACKET_ENTRIES) != 0) { /* skip per-packet-buffer */ - if (DBG) Log.d(TAG, "skipped per packet logging ring " + buffer.name); + if (DBG) mLog.d("skipped per packet logging ring " + buffer.name); continue; } @@ -463,7 +465,7 @@ class WifiDiagnostics extends BaseWifiDiagnostics { if (mWifiNative.startLoggingRingBuffer( mLogLevel, 0, minInterval, minDataSize, buffer.name) == false) { - if (DBG) Log.e(TAG, "Could not start logging ring " + buffer.name); + if (DBG) mLog.e("Could not start logging ring " + buffer.name); return false; } @@ -472,7 +474,7 @@ class WifiDiagnostics extends BaseWifiDiagnostics { private boolean stopLoggingRingBuffer(WifiNative.RingBufferStatus buffer) { if (mWifiNative.startLoggingRingBuffer(0, 0, 0, 0, buffer.name) == false) { - if (DBG) Log.e(TAG, "Could not stop logging ring " + buffer.name); + if (DBG) mLog.e("Could not stop logging ring " + buffer.name); } return true; } @@ -488,19 +490,19 @@ class WifiDiagnostics extends BaseWifiDiagnostics { private boolean getAllRingBufferData() { if (mRingBuffers == null) { - Log.e(TAG, "Not ring buffers available to collect data!"); + mLog.e("Not ring buffers available to collect data!"); return false; } for (WifiNative.RingBufferStatus element : mRingBuffers){ boolean result = mWifiNative.getRingBufferData(element.name); if (!result) { - Log.e(TAG, "Fail to get ring buffer data of: " + element.name); + mLog.e("Fail to get ring buffer data of: " + element.name); return false; } } - Log.d(TAG, "getAllRingBufferData Successfully!"); + mLog.d("getAllRingBufferData Successfully!"); return true; } @@ -542,7 +544,7 @@ class WifiDiagnostics extends BaseWifiDiagnostics { return mLastBugReports; } - private static String compressToBase64(byte[] input) { + private String compressToBase64(byte[] input) { String result; //compress Deflater compressor = new Deflater(); @@ -561,14 +563,14 @@ class WifiDiagnostics extends BaseWifiDiagnostics { compressor.end(); bos.close(); } catch (IOException e) { - Log.e(TAG, "ByteArrayOutputStream close error"); + mLog.e("ByteArrayOutputStream close error"); result = android.util.Base64.encodeToString(input, Base64.DEFAULT); return result; } byte[] compressed = bos.toByteArray(); if (DBG) { - Log.d(TAG," length is:" + (compressed == null? "0" : compressed.length)); + mLog.d(" length is:" + (compressed == null? "0" : compressed.length)); } //encode @@ -576,7 +578,7 @@ class WifiDiagnostics extends BaseWifiDiagnostics { compressed.length < input.length ? compressed : input , Base64.DEFAULT); if (DBG) { - Log.d(TAG, "FwMemoryDump length is :" + result.length()); + mLog.d("FwMemoryDump length is :" + result.length()); } return result; @@ -599,20 +601,20 @@ class WifiDiagnostics extends BaseWifiDiagnostics { } process.waitFor(); } catch (InterruptedException|IOException e) { - Log.e(TAG, "Exception while capturing logcat" + e); + mLog.e("Exception while capturing logcat" + e); } return lines; } private LimitedCircularArray<String> getKernelLog(int maxLines) { - if (DBG) Log.d(TAG, "Reading kernel log ..."); + if (DBG) mLog.d("Reading kernel log ..."); LimitedCircularArray<String> lines = new LimitedCircularArray<String>(maxLines); String log = mWifiNative.readKernelLog(); String logLines[] = log.split("\n"); for (int i = 0; i < logLines.length; i++) { lines.addLast(logLines[i]); } - if (DBG) Log.d(TAG, "Added " + logLines.length + " lines"); + if (DBG) mLog.d("Added " + logLines.length + " lines"); return lines; } diff --git a/service/java/com/android/server/wifi/WifiInjector.java b/service/java/com/android/server/wifi/WifiInjector.java index c35440714..63cc35c19 100644 --- a/service/java/com/android/server/wifi/WifiInjector.java +++ b/service/java/com/android/server/wifi/WifiInjector.java @@ -69,6 +69,7 @@ public class WifiInjector { private final KeyStore mKeyStore = KeyStore.getInstance(); private final WifiBackupRestore mWifiBackupRestore = new WifiBackupRestore(); private final WifiMulticastLockManager mWifiMulticastLockManager; + private final boolean mUseRealLogger; public WifiInjector(Context context) { if (context == null) { @@ -84,6 +85,9 @@ public class WifiInjector { sWifiInjector = this; mContext = context; + mUseRealLogger = mContext.getResources().getBoolean( + R.bool.config_wifi_enable_wifi_firmware_debugging); + // Now create and start handler threads mWifiServiceHandlerThread = new HandlerThread("WifiService"); mWifiServiceHandlerThread.start(); @@ -235,4 +239,21 @@ public class WifiInjector { wifiNative, countryCode, allowed2GChannels, listener, apInterface); } + + /** + * Create a WifiLog instance. + * @param tag module name to include in all log messages + */ + public WifiLog makeLog(String tag) { + return new LogcatLog(tag); + } + + public BaseWifiDiagnostics makeWifiDiagnostics(WifiNative wifiNative) { + if (mUseRealLogger) { + return new WifiDiagnostics( + mContext, this, mWifiStateMachine, wifiNative, mBuildProperties); + } else { + return new BaseWifiDiagnostics(); + } + } } diff --git a/service/java/com/android/server/wifi/WifiLog.java b/service/java/com/android/server/wifi/WifiLog.java new file mode 100644 index 000000000..f8703ea65 --- /dev/null +++ b/service/java/com/android/server/wifi/WifiLog.java @@ -0,0 +1,49 @@ +/* + * Copyright (C) 2016 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server.wifi; + +public interface WifiLog { + /** + * Log an error using the default tag for this WifiLog instance. + * @param msg the message to be logged + */ + void e(String msg); + + /** + * Log a warning using the default tag for this WifiLog instance. + * @param msg the message to be logged + */ + void w(String msg); + + /** + * Log an informational message using the default tag for this WifiLog instance. + * @param msg the message to be logged + */ + void i(String msg); + + /** + * Log a debug message using the default tag for this WifiLog instance. + * @param msg the message to be logged + */ + void d(String msg); + + /** + * Log a verbose message using the default tag for this WifiLog instance. + * @param msg the message to be logged + */ + void v(String msg); +} diff --git a/service/java/com/android/server/wifi/WifiStateMachine.java b/service/java/com/android/server/wifi/WifiStateMachine.java index dee7d2c00..e4039dd14 100644 --- a/service/java/com/android/server/wifi/WifiStateMachine.java +++ b/service/java/com/android/server/wifi/WifiStateMachine.java @@ -969,16 +969,7 @@ public class WifiStateMachine extends StateMachine implements WifiNative.WifiRss mWifiInjector.getClock(), userManager, mWifiInjector.getKeyStore()); mWifiMonitor = WifiMonitor.getInstance(); - - boolean enableFirmwareLogs = mContext.getResources().getBoolean( - R.bool.config_wifi_enable_wifi_firmware_debugging); - - if (enableFirmwareLogs) { - mWifiDiagnostics = facade.makeRealDiagnostics( - mContext, this, mWifiNative, mBuildProperties); - } else { - mWifiDiagnostics = facade.makeBaseDiagnostics(); - } + mWifiDiagnostics = mWifiInjector.makeWifiDiagnostics(mWifiNative); mWifiInfo = new WifiInfo(); mWifiQualifiedNetworkSelector = new WifiQualifiedNetworkSelector(mWifiConfigManager, diff --git a/tests/wifitests/src/com/android/server/wifi/WifiDiagnosticsTest.java b/tests/wifitests/src/com/android/server/wifi/WifiDiagnosticsTest.java index d954522a6..ce5ba7bf0 100644 --- a/tests/wifitests/src/com/android/server/wifi/WifiDiagnosticsTest.java +++ b/tests/wifitests/src/com/android/server/wifi/WifiDiagnosticsTest.java @@ -26,6 +26,8 @@ import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.mockito.Matchers.anyString; +import static org.mockito.Matchers.contains; import static org.mockito.Mockito.anyInt; import static org.mockito.Mockito.anyObject; import static org.mockito.Mockito.eq; @@ -55,6 +57,8 @@ public class WifiDiagnosticsTest { @Mock WifiNative mWifiNative; @Mock BuildProperties mBuildProperties; @Mock Context mContext; + @Mock WifiInjector mWifiInjector; + @Mock WifiLog mLog; WifiDiagnostics mWifiDiagnostics; private static final String FAKE_RING_BUFFER_NAME = "fake-ring-buffer"; @@ -100,8 +104,10 @@ public class WifiDiagnosticsTest { resources.setInteger(R.integer.config_wifi_logger_ring_buffer_verbose_size_limit_kb, LARGE_RING_BUFFER_SIZE_KB); when(mContext.getResources()).thenReturn(resources); + when(mWifiInjector.makeLog(anyString())).thenReturn(mLog); - mWifiDiagnostics = new WifiDiagnostics(mContext, mWsm, mWifiNative, mBuildProperties); + mWifiDiagnostics = new WifiDiagnostics( + mContext, mWifiInjector, mWsm, mWifiNative, mBuildProperties); mWifiNative.enableVerboseLogging(0); } @@ -263,6 +269,15 @@ public class WifiDiagnosticsTest { verify(mWifiNative).startPktFateMonitoring(); } + // Verifies that startLogging() reports failure of startPktFateMonitoring(). + @Test + public void startLoggingReportsFailureOfStartPktFateMonitoring() { + final boolean verbosityToggle = true; + when(mWifiNative.startPktFateMonitoring()).thenReturn(false); + mWifiDiagnostics.startLogging(verbosityToggle); + verify(mLog).e(contains("Failed")); + } + /** * Verifies that, when verbose mode is not enabled, reportConnectionFailure() still * fetches packet fates. diff --git a/tests/wifitests/src/com/android/server/wifi/WifiStateMachineTest.java b/tests/wifitests/src/com/android/server/wifi/WifiStateMachineTest.java index 491911702..0c258e989 100644 --- a/tests/wifitests/src/com/android/server/wifi/WifiStateMachineTest.java +++ b/tests/wifitests/src/com/android/server/wifi/WifiStateMachineTest.java @@ -158,7 +158,6 @@ public class WifiStateMachineTest { when(facade.makeWifiScanner(any(Context.class), any(Looper.class))) .thenReturn(mWifiScanner); - when(facade.makeBaseDiagnostics()).thenReturn(mock(BaseWifiDiagnostics.class)); when(facade.getService(Context.NETWORKMANAGEMENT_SERVICE)).thenReturn( mockWithInterfaces(IBinder.class, INetworkManagementService.class)); @@ -349,6 +348,8 @@ public class WifiStateMachineTest { when(mWifiInjector.getBuildProperties()).thenReturn(mBuildProperties); when(mWifiInjector.getKeyStore()).thenReturn(mock(KeyStore.class)); when(mWifiInjector.getWifiBackupRestore()).thenReturn(mock(WifiBackupRestore.class)); + when(mWifiInjector.makeWifiDiagnostics(anyObject())).thenReturn( + mock(BaseWifiDiagnostics.class)); FrameworkFacade factory = getFrameworkFacade(); Context context = getContext(); |