[SB][Sat] Add carrier text logs, make device-based sat VM a singleton.
1) Adds more logs around how we determine the satellite carrier text to
help us debug b/341841138.
2) Makes the DeviceBasedSatelliteViewModel into a singleton, which may
just fix b/341841138? It looks like there were previously two
instances of the VM, one for lockscreen and one for shade (because
CarrierTextManager is *not* a singleton and was injecting the view
model). So, it's possible the 10s delay in showing satellite info was
being triggered at different times in the two VM instances, causing
differences between lockscreen and shade.
Bug: 341841138
Test: Use demo satellite mode, dump logs for CarrierTextManagerLog,
ShadeCarrierGroupControllerLog, and DeviceBasedSatelliteInputLog
Flag: NONE Adding logs only
Change-Id: I96bf9de1248e25a8d32e8440c161db2508a91eff
Merged-In: I96bf9de1248e25a8d32e8440c161db2508a91eff
(cherry picked from commit 996bf18e2c3647814d86aebf02d2c25cb81aed81)
diff --git a/packages/SystemUI/src/com/android/keyguard/CarrierTextManager.java b/packages/SystemUI/src/com/android/keyguard/CarrierTextManager.java
index 5647b0b..c5f1c17 100644
--- a/packages/SystemUI/src/com/android/keyguard/CarrierTextManager.java
+++ b/packages/SystemUI/src/com/android/keyguard/CarrierTextManager.java
@@ -300,6 +300,7 @@
});
mTelephonyListenerManager.addActiveDataSubscriptionIdListener(mPhoneStateListener);
cancelSatelliteCollectionJob(/* reason= */ "Starting new job");
+ mLogger.logStartListeningForSatelliteCarrierText();
mSatelliteConnectionJob =
mJavaAdapter.alwaysCollectFlow(
mDeviceBasedSatelliteViewModel.getCarrierText(),
@@ -316,7 +317,7 @@
mWakefulnessLifecycle.removeObserver(mWakefulnessObserver);
});
mTelephonyListenerManager.removeActiveDataSubscriptionIdListener(mPhoneStateListener);
- cancelSatelliteCollectionJob(/* reason= */ "Stopping listening");
+ cancelSatelliteCollectionJob(/* reason= */ "#handleSetListening has null callback");
}
}
@@ -336,6 +337,7 @@
private void onSatelliteCarrierTextChanged(@Nullable String text) {
mLogger.logUpdateCarrierTextForReason(REASON_SATELLITE_CHANGED);
+ mLogger.logNewSatelliteCarrierText(text);
mSatelliteCarrierText = text;
updateCarrierText();
}
@@ -654,6 +656,7 @@
private void cancelSatelliteCollectionJob(String reason) {
Job job = mSatelliteConnectionJob;
if (job != null) {
+ mLogger.logStopListeningForSatelliteCarrierText(reason);
job.cancel(new CancellationException(reason));
}
}
diff --git a/packages/SystemUI/src/com/android/keyguard/logging/CarrierTextManagerLogger.kt b/packages/SystemUI/src/com/android/keyguard/logging/CarrierTextManagerLogger.kt
index 48fea55..7d0c491 100644
--- a/packages/SystemUI/src/com/android/keyguard/logging/CarrierTextManagerLogger.kt
+++ b/packages/SystemUI/src/com/android/keyguard/logging/CarrierTextManagerLogger.kt
@@ -38,8 +38,11 @@
buffer.log(
TAG,
LogLevel.VERBOSE,
- { int1 = numSubs },
- { "updateCarrierText: location=${location ?: "(unknown)"} numSubs=$int1" },
+ {
+ int1 = numSubs
+ str1 = location
+ },
+ { "updateCarrierText: location=${str1 ?: "(unknown)"} numSubs=$int1" },
)
}
@@ -77,6 +80,15 @@
)
}
+ fun logNewSatelliteCarrierText(newSatelliteText: String?) {
+ buffer.log(
+ TAG,
+ LogLevel.VERBOSE,
+ { str1 = newSatelliteText },
+ { "New satellite text = $str1" },
+ )
+ }
+
fun logUsingSatelliteText(satelliteText: String) {
buffer.log(
TAG,
@@ -125,10 +137,37 @@
buffer.log(
TAG,
LogLevel.DEBUG,
- { int1 = reason },
+ {
+ int1 = reason
+ str1 = location
+ },
{
"refreshing carrier info for reason: ${reason.reasonMessage()}" +
- " location=${location ?: "(unknown)"}"
+ " location=${str1 ?: "(unknown)"}"
+ }
+ )
+ }
+
+ fun logStartListeningForSatelliteCarrierText() {
+ buffer.log(
+ TAG,
+ LogLevel.DEBUG,
+ { str1 = location },
+ { "Start listening for satellite carrier text. Location=${str1 ?: "(unknown)"}" }
+ )
+ }
+
+ fun logStopListeningForSatelliteCarrierText(reason: String) {
+ buffer.log(
+ TAG,
+ LogLevel.DEBUG,
+ {
+ str1 = location
+ str2 = reason
+ },
+ {
+ "Stop listening for satellite carrier text. " +
+ "Location=${str1 ?: "(unknown)"} Reason=$str2"
}
)
}
diff --git a/packages/SystemUI/src/com/android/systemui/log/dagger/LogModule.java b/packages/SystemUI/src/com/android/systemui/log/dagger/LogModule.java
index f2013be..6a39596 100644
--- a/packages/SystemUI/src/com/android/systemui/log/dagger/LogModule.java
+++ b/packages/SystemUI/src/com/android/systemui/log/dagger/LogModule.java
@@ -458,7 +458,7 @@
@SysUISingleton
@CarrierTextManagerLog
public static LogBuffer provideCarrierTextManagerLog(LogBufferFactory factory) {
- return factory.create("CarrierTextManagerLog", 100);
+ return factory.create("CarrierTextManagerLog", 400);
}
/**
diff --git a/packages/SystemUI/src/com/android/systemui/shade/ShadeModule.kt b/packages/SystemUI/src/com/android/systemui/shade/ShadeModule.kt
index 5632766..d4a726e 100644
--- a/packages/SystemUI/src/com/android/systemui/shade/ShadeModule.kt
+++ b/packages/SystemUI/src/com/android/systemui/shade/ShadeModule.kt
@@ -18,6 +18,9 @@
import com.android.systemui.dagger.SysUISingleton
import com.android.systemui.scene.shared.flag.SceneContainerFlags
+import com.android.systemui.log.LogBuffer
+import com.android.systemui.log.LogBufferFactory
+import com.android.systemui.shade.carrier.ShadeCarrierGroupControllerLog
import com.android.systemui.shade.data.repository.PrivacyChipRepository
import com.android.systemui.shade.data.repository.PrivacyChipRepositoryImpl
import com.android.systemui.shade.data.repository.ShadeRepository
@@ -111,6 +114,13 @@
sceneContainerOff.get()
}
}
+
+ @Provides
+ @SysUISingleton
+ @ShadeCarrierGroupControllerLog
+ fun provideShadeCarrierLog(factory: LogBufferFactory): LogBuffer {
+ return factory.create("ShadeCarrierGroupControllerLog", 400)
+ }
}
@Binds
diff --git a/packages/SystemUI/src/com/android/systemui/shade/carrier/ShadeCarrierGroupController.java b/packages/SystemUI/src/com/android/systemui/shade/carrier/ShadeCarrierGroupController.java
index 2b68add..8cd7644 100644
--- a/packages/SystemUI/src/com/android/systemui/shade/carrier/ShadeCarrierGroupController.java
+++ b/packages/SystemUI/src/com/android/systemui/shade/carrier/ShadeCarrierGroupController.java
@@ -97,6 +97,8 @@
private final SlotIndexResolver mSlotIndexResolver;
+ private final ShadeCarrierGroupControllerLogger mLogger;
+
private final SignalCallback mSignalCallback = new SignalCallback() {
@Override
public void setMobileDataIndicators(@NonNull MobileDataIndicators indicators) {
@@ -148,6 +150,7 @@
ActivityStarter activityStarter,
@Background Handler bgHandler,
@Main Looper mainLooper,
+ ShadeCarrierGroupControllerLogger logger,
NetworkController networkController,
CarrierTextManager.Builder carrierTextManagerBuilder,
Context context,
@@ -160,6 +163,7 @@
mContext = context;
mActivityStarter = activityStarter;
mBgHandler = bgHandler;
+ mLogger = logger;
mNetworkController = networkController;
mStatusBarPipelineFlags = statusBarPipelineFlags;
mCarrierTextManager = carrierTextManagerBuilder
@@ -376,10 +380,13 @@
return;
}
+ mLogger.logHandleUpdateCarrierInfo(info);
+
mNoSimTextView.setVisibility(View.GONE);
if (!info.airplaneMode && info.anySimReady) {
boolean[] slotSeen = new boolean[SIM_SLOTS];
if (info.listOfCarriers.length == info.subscriptionIds.length) {
+ mLogger.logUsingSimViews();
for (int i = 0; i < SIM_SLOTS && i < info.listOfCarriers.length; i++) {
int slot = getSlotIndex(info.subscriptionIds[i]);
if (slot >= SIM_SLOTS) {
@@ -407,9 +414,11 @@
}
}
} else {
- Log.e(TAG, "Carrier information arrays not of same length");
+ mLogger.logInvalidArrayLengths(
+ info.listOfCarriers.length, info.subscriptionIds.length);
}
} else {
+ mLogger.logUsingNoSimView(info.carrierText);
// No sims or airplane mode (but not WFC). Do not show ShadeCarrierGroup,
// instead just show info.carrierText in a different view.
for (int i = 0; i < SIM_SLOTS; i++) {
@@ -460,6 +469,7 @@
private final ActivityStarter mActivityStarter;
private final Handler mHandler;
private final Looper mLooper;
+ private final ShadeCarrierGroupControllerLogger mLogger;
private final NetworkController mNetworkController;
private final CarrierTextManager.Builder mCarrierTextControllerBuilder;
private final Context mContext;
@@ -474,6 +484,7 @@
ActivityStarter activityStarter,
@Background Handler handler,
@Main Looper looper,
+ ShadeCarrierGroupControllerLogger logger,
NetworkController networkController,
CarrierTextManager.Builder carrierTextControllerBuilder,
Context context,
@@ -486,6 +497,7 @@
mActivityStarter = activityStarter;
mHandler = handler;
mLooper = looper;
+ mLogger = logger;
mNetworkController = networkController;
mCarrierTextControllerBuilder = carrierTextControllerBuilder;
mContext = context;
@@ -507,6 +519,7 @@
mActivityStarter,
mHandler,
mLooper,
+ mLogger,
mNetworkController,
mCarrierTextControllerBuilder,
mContext,
diff --git a/packages/SystemUI/src/com/android/systemui/shade/carrier/ShadeCarrierGroupControllerLog.kt b/packages/SystemUI/src/com/android/systemui/shade/carrier/ShadeCarrierGroupControllerLog.kt
new file mode 100644
index 0000000..36aa7a6
--- /dev/null
+++ b/packages/SystemUI/src/com/android/systemui/shade/carrier/ShadeCarrierGroupControllerLog.kt
@@ -0,0 +1,25 @@
+/*
+ * Copyright (C) 2024 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.systemui.shade.carrier
+
+import javax.inject.Qualifier
+
+/** A [LogBuffer] for detailed carrier text logs for [ShadeCarrierGroupController]. */
+@Qualifier
+@MustBeDocumented
+@Retention(AnnotationRetention.RUNTIME)
+annotation class ShadeCarrierGroupControllerLog
diff --git a/packages/SystemUI/src/com/android/systemui/shade/carrier/ShadeCarrierGroupControllerLogger.kt b/packages/SystemUI/src/com/android/systemui/shade/carrier/ShadeCarrierGroupControllerLogger.kt
new file mode 100644
index 0000000..af06a35
--- /dev/null
+++ b/packages/SystemUI/src/com/android/systemui/shade/carrier/ShadeCarrierGroupControllerLogger.kt
@@ -0,0 +1,75 @@
+/*
+ * Copyright (C) 2024 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.systemui.shade.carrier
+
+import com.android.keyguard.CarrierTextManager
+import com.android.systemui.dagger.SysUISingleton
+import com.android.systemui.log.LogBuffer
+import com.android.systemui.log.core.LogLevel
+import javax.inject.Inject
+
+/** Logger for [ShadeCarrierGroupController], mostly to try and solve b/341841138. */
+@SysUISingleton
+class ShadeCarrierGroupControllerLogger
+@Inject
+constructor(@ShadeCarrierGroupControllerLog val buffer: LogBuffer) {
+ /** De-structures the info object so that we don't have to generate new strings */
+ fun logHandleUpdateCarrierInfo(info: CarrierTextManager.CarrierTextCallbackInfo) {
+ buffer.log(
+ TAG,
+ LogLevel.VERBOSE,
+ {
+ str1 = "${info.carrierText}"
+ bool1 = info.anySimReady
+ bool2 = info.airplaneMode
+ },
+ {
+ "handleUpdateCarrierInfo: " +
+ "result=(carrierText=$str1, anySimReady=$bool1, airplaneMode=$bool2)"
+ },
+ )
+ }
+
+ fun logInvalidArrayLengths(numCarriers: Int, numSubs: Int) {
+ buffer.log(
+ TAG,
+ LogLevel.ERROR,
+ {
+ int1 = numCarriers
+ int2 = numSubs
+ },
+ { "┗ carriers.length != subIds.length. carriers.length=$int1 subs.length=$int2" },
+ )
+ }
+
+ fun logUsingNoSimView(text: CharSequence) {
+ buffer.log(
+ TAG,
+ LogLevel.VERBOSE,
+ { str1 = "$text" },
+ { "┗ updating No SIM view with text=$str1" },
+ )
+ }
+
+ fun logUsingSimViews() {
+ buffer.log(TAG, LogLevel.VERBOSE, {}, { "┗ updating SIM views" })
+ }
+
+ private companion object {
+ const val TAG = "SCGC"
+ }
+}
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/satellite/ui/viewmodel/DeviceBasedSatelliteViewModel.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/satellite/ui/viewmodel/DeviceBasedSatelliteViewModel.kt
index 332c121..d76fd40 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/satellite/ui/viewmodel/DeviceBasedSatelliteViewModel.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/satellite/ui/viewmodel/DeviceBasedSatelliteViewModel.kt
@@ -18,6 +18,7 @@
import android.content.Context
import com.android.systemui.common.shared.model.Icon
+import com.android.systemui.dagger.SysUISingleton
import com.android.systemui.dagger.qualifiers.Application
import com.android.systemui.log.LogBuffer
import com.android.systemui.log.core.LogLevel
@@ -39,6 +40,7 @@
import kotlinx.coroutines.flow.distinctUntilChanged
import kotlinx.coroutines.flow.flatMapLatest
import kotlinx.coroutines.flow.flowOf
+import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.flow.stateIn
/**
@@ -60,6 +62,7 @@
}
@OptIn(ExperimentalCoroutinesApi::class)
+@SysUISingleton
class DeviceBasedSatelliteViewModelImpl
@Inject
constructor(
@@ -124,18 +127,37 @@
shouldActuallyShowIcon,
interactor.connectionState,
) { shouldShow, connectionState ->
+ logBuffer.log(
+ TAG,
+ LogLevel.INFO,
+ {
+ bool1 = shouldShow
+ str1 = connectionState.name
+ },
+ { "Updating carrier text. shouldActuallyShow=$bool1 connectionState=$str1" }
+ )
if (shouldShow) {
when (connectionState) {
SatelliteConnectionState.On,
SatelliteConnectionState.Connected ->
context.getString(R.string.satellite_connected_carrier_text)
SatelliteConnectionState.Off,
- SatelliteConnectionState.Unknown -> null
+ SatelliteConnectionState.Unknown -> {
+ null
+ }
}
} else {
null
}
}
+ .onEach {
+ logBuffer.log(
+ TAG,
+ LogLevel.INFO,
+ { str1 = it },
+ { "Resulting carrier text = $str1" }
+ )
+ }
.stateIn(scope, SharingStarted.WhileSubscribed(), null)
companion object {
diff --git a/packages/SystemUI/tests/src/com/android/systemui/shade/carrier/ShadeCarrierGroupControllerTest.java b/packages/SystemUI/tests/src/com/android/systemui/shade/carrier/ShadeCarrierGroupControllerTest.java
index e7056c7..19803a6 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/shade/carrier/ShadeCarrierGroupControllerTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/shade/carrier/ShadeCarrierGroupControllerTest.java
@@ -46,6 +46,7 @@
import androidx.test.filters.SmallTest;
import com.android.keyguard.CarrierTextManager;
+import com.android.systemui.log.core.FakeLogBuffer;
import com.android.systemui.plugins.ActivityStarter;
import com.android.systemui.statusbar.connectivity.IconState;
import com.android.systemui.statusbar.connectivity.MobileDataIndicators;
@@ -169,6 +170,7 @@
mActivityStarter,
handler,
TestableLooper.get(this).getLooper(),
+ new ShadeCarrierGroupControllerLogger(FakeLogBuffer.Factory.Companion.create()),
mNetworkController,
mCarrierTextControllerBuilder,
mContext,