diff options
6 files changed, 223 insertions, 65 deletions
diff --git a/packages/SystemUI/src/com/android/systemui/log/table/Diffable.kt b/packages/SystemUI/src/com/android/systemui/log/table/Diffable.kt index c27bfa338df3..38af003ffaa5 100644 --- a/packages/SystemUI/src/com/android/systemui/log/table/Diffable.kt +++ b/packages/SystemUI/src/com/android/systemui/log/table/Diffable.kt @@ -30,10 +30,11 @@ import kotlinx.coroutines.flow.Flow */ interface Diffable<T> { /** - * Finds the differences between [prevVal] and [this] and logs those diffs to [row]. + * Finds the differences between [prevVal] and this object and logs those diffs to [row]. * * Each implementer should determine which individual fields have changed between [prevVal] and - * [this], and only log the fields that have actually changed. This helps save buffer space. + * this object, and only log the fields that have actually changed. This helps save buffer + * space. * * For example, if: * - prevVal = Object(val1=100, val2=200, val3=300) @@ -42,6 +43,16 @@ interface Diffable<T> { * Then only the val3 change should be logged. */ fun logDiffs(prevVal: T, row: TableRowLogger) + + /** + * Logs all the relevant fields of this object to [row]. + * + * As opposed to [logDiffs], this method should log *all* fields. + * + * Implementation is optional. This method will only be used with [logDiffsForTable] in order to + * fully log the initial value of the flow. + */ + fun logFull(row: TableRowLogger) {} } /** @@ -57,7 +68,12 @@ fun <T : Diffable<T>> Flow<T>.logDiffsForTable( columnPrefix: String, initialValue: T, ): Flow<T> { - return this.pairwiseBy(initialValue) { prevVal, newVal -> + // Fully log the initial value to the table. + val getInitialValue = { + tableLogBuffer.logChange(columnPrefix) { row -> initialValue.logFull(row) } + initialValue + } + return this.pairwiseBy(getInitialValue) { prevVal: T, newVal: T -> tableLogBuffer.logDiffs(columnPrefix, prevVal, newVal) newVal } diff --git a/packages/SystemUI/src/com/android/systemui/log/table/TableLogBuffer.kt b/packages/SystemUI/src/com/android/systemui/log/table/TableLogBuffer.kt index 02aac01521bc..0bf530496b28 100644 --- a/packages/SystemUI/src/com/android/systemui/log/table/TableLogBuffer.kt +++ b/packages/SystemUI/src/com/android/systemui/log/table/TableLogBuffer.kt @@ -113,6 +113,20 @@ class TableLogBuffer( newVal.logDiffs(prevVal, row) } + /** + * Logs change(s) to the buffer using [rowInitializer]. + * + * @param rowInitializer a function that will be called immediately to store relevant data on + * the row. + */ + @Synchronized + fun logChange(columnPrefix: String, rowInitializer: (TableRowLogger) -> Unit) { + val row = tempRow + row.timestamp = systemClock.currentTimeMillis() + row.columnPrefix = columnPrefix + rowInitializer(row) + } + // Keep these individual [logChange] methods private (don't let clients give us their own // timestamps.) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/data/model/WifiNetworkModel.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/data/model/WifiNetworkModel.kt index d663fd6e17b2..a682a5711a6f 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/data/model/WifiNetworkModel.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/data/model/WifiNetworkModel.kt @@ -31,15 +31,19 @@ sealed class WifiNetworkModel : Diffable<WifiNetworkModel> { if (prevVal is Inactive) { return } - row.logChange(COL_NETWORK_TYPE, TYPE_INACTIVE) if (prevVal is CarrierMerged) { // The only difference between CarrierMerged and Inactive is the type + row.logChange(COL_NETWORK_TYPE, TYPE_INACTIVE) return } // When changing from Active to Inactive, we need to log diffs to all the fields. - logDiffsFromActiveToNotActive(prevVal as Active, row) + logFullNonActiveNetwork(TYPE_INACTIVE, row) + } + + override fun logFull(row: TableRowLogger) { + logFullNonActiveNetwork(TYPE_INACTIVE, row) } } @@ -56,15 +60,15 @@ sealed class WifiNetworkModel : Diffable<WifiNetworkModel> { if (prevVal is CarrierMerged) { return } - row.logChange(COL_NETWORK_TYPE, TYPE_CARRIER_MERGED) if (prevVal is Inactive) { // The only difference between CarrierMerged and Inactive is the type. + row.logChange(COL_NETWORK_TYPE, TYPE_CARRIER_MERGED) return } // When changing from Active to CarrierMerged, we need to log diffs to all the fields. - logDiffsFromActiveToNotActive(prevVal as Active, row) + logFullNonActiveNetwork(TYPE_CARRIER_MERGED, row) } } @@ -147,7 +151,6 @@ sealed class WifiNetworkModel : Diffable<WifiNetworkModel> { } } - override fun toString(): String { // Only include the passpoint-related values in the string if we have them. (Most // networks won't have them so they'll be mostly clutter.) @@ -174,21 +177,15 @@ sealed class WifiNetworkModel : Diffable<WifiNetworkModel> { } } - internal fun logDiffsFromActiveToNotActive(prevActive: Active, row: TableRowLogger) { + internal fun logFullNonActiveNetwork(type: String, row: TableRowLogger) { + row.logChange(COL_NETWORK_TYPE, type) row.logChange(COL_NETWORK_ID, NETWORK_ID_DEFAULT) row.logChange(COL_VALIDATED, false) row.logChange(COL_LEVEL, LEVEL_DEFAULT) row.logChange(COL_SSID, null) - - if (prevActive.isPasspointAccessPoint) { - row.logChange(COL_PASSPOINT_ACCESS_POINT, false) - } - if (prevActive.isOnlineSignUpForPasspointAccessPoint) { - row.logChange(COL_ONLINE_SIGN_UP, false) - } - if (prevActive.passpointProviderFriendlyName != null) { - row.logChange(COL_PASSPOINT_NAME, null) - } + row.logChange(COL_PASSPOINT_ACCESS_POINT, false) + row.logChange(COL_ONLINE_SIGN_UP, false) + row.logChange(COL_PASSPOINT_NAME, null) } } diff --git a/packages/SystemUI/src/com/android/systemui/util/kotlin/Flow.kt b/packages/SystemUI/src/com/android/systemui/util/kotlin/Flow.kt index f71d596ff835..b61b2e66fb22 100644 --- a/packages/SystemUI/src/com/android/systemui/util/kotlin/Flow.kt +++ b/packages/SystemUI/src/com/android/systemui/util/kotlin/Flow.kt @@ -20,7 +20,6 @@ import java.util.concurrent.atomic.AtomicReference import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.coroutineScope import kotlinx.coroutines.flow.Flow -import kotlinx.coroutines.flow.collect import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.flow import kotlinx.coroutines.flow.onStart @@ -58,6 +57,22 @@ fun <T, R> Flow<T>.pairwiseBy( onStart { emit(initialValue) }.pairwiseBy(transform) /** + * Returns a new [Flow] that combines the two most recent emissions from [this] using [transform]. + * + * + * The output of [getInitialValue] will be used as the "old" value for the first emission. As + * opposed to the initial value in the above [pairwiseBy], [getInitialValue] can do some work before + * returning the initial value. + * + * Useful for code that needs to compare the current value to the previous value. + */ +fun <T, R> Flow<T>.pairwiseBy( + getInitialValue: suspend () -> T, + transform: suspend (previousValue: T, newValue: T) -> R, +): Flow<R> = + onStart { emit(getInitialValue()) }.pairwiseBy(transform) + +/** * Returns a new [Flow] that produces the two most recent emissions from [this]. Note that the new * Flow will not start emitting until it has received two emissions from the upstream Flow. * diff --git a/packages/SystemUI/tests/src/com/android/systemui/log/table/TableLogBufferTest.kt b/packages/SystemUI/tests/src/com/android/systemui/log/table/TableLogBufferTest.kt index ba21f1260b32..37fb6af3886a 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/log/table/TableLogBufferTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/log/table/TableLogBufferTest.kt @@ -337,6 +337,68 @@ class TableLogBufferTest : SysuiTestCase() { } @Test + fun logChange_rowInitializer_dumpsCorrectly() { + systemClock.setCurrentTimeMillis(100L) + + underTest.logChange("") { row -> + row.logChange("column1", "val1") + row.logChange("column2", 2) + row.logChange("column3", true) + } + + val dumpedString = dumpChanges() + + val timestamp = TABLE_LOG_DATE_FORMAT.format(100L) + val expected1 = timestamp + SEPARATOR + "column1" + SEPARATOR + "val1" + val expected2 = timestamp + SEPARATOR + "column2" + SEPARATOR + "2" + val expected3 = timestamp + SEPARATOR + "column3" + SEPARATOR + "true" + assertThat(dumpedString).contains(expected1) + assertThat(dumpedString).contains(expected2) + assertThat(dumpedString).contains(expected3) + } + + @Test + fun logChangeAndLogDiffs_bothLogged() { + systemClock.setCurrentTimeMillis(100L) + + underTest.logChange("") { row -> + row.logChange("column1", "val1") + row.logChange("column2", 2) + row.logChange("column3", true) + } + + systemClock.setCurrentTimeMillis(200L) + val prevDiffable = object : TestDiffable() {} + val nextDiffable = + object : TestDiffable() { + override fun logDiffs(prevVal: TestDiffable, row: TableRowLogger) { + row.logChange("column1", "newVal1") + row.logChange("column2", 222) + row.logChange("column3", false) + } + } + + underTest.logDiffs(columnPrefix = "", prevDiffable, nextDiffable) + + val dumpedString = dumpChanges() + + val timestamp1 = TABLE_LOG_DATE_FORMAT.format(100L) + val expected1 = timestamp1 + SEPARATOR + "column1" + SEPARATOR + "val1" + val expected2 = timestamp1 + SEPARATOR + "column2" + SEPARATOR + "2" + val expected3 = timestamp1 + SEPARATOR + "column3" + SEPARATOR + "true" + val timestamp2 = TABLE_LOG_DATE_FORMAT.format(200L) + val expected4 = timestamp2 + SEPARATOR + "column1" + SEPARATOR + "newVal1" + val expected5 = timestamp2 + SEPARATOR + "column2" + SEPARATOR + "222" + val expected6 = timestamp2 + SEPARATOR + "column3" + SEPARATOR + "false" + assertThat(dumpedString).contains(expected1) + assertThat(dumpedString).contains(expected2) + assertThat(dumpedString).contains(expected3) + assertThat(dumpedString).contains(expected4) + assertThat(dumpedString).contains(expected5) + assertThat(dumpedString).contains(expected6) + } + + @Test fun dumpChanges_rotatesIfBufferIsFull() { lateinit var valToDump: String diff --git a/packages/SystemUI/tests/src/com/android/systemui/util/kotlin/FlowUtilTests.kt b/packages/SystemUI/tests/src/com/android/systemui/util/kotlin/FlowUtilTests.kt index 7df707789290..6bfc2f1a8b8b 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/util/kotlin/FlowUtilTests.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/util/kotlin/FlowUtilTests.kt @@ -51,15 +51,11 @@ class PairwiseFlowTest : SysuiTestCase() { ) } - @Test - fun notEnough() = runBlocking { - assertThatFlow(flowOf(1).pairwise()).emitsNothing() - } + @Test fun notEnough() = runBlocking { assertThatFlow(flowOf(1).pairwise()).emitsNothing() } @Test fun withInit() = runBlocking { - assertThatFlow(flowOf(2).pairwise(initialValue = 1)) - .emitsExactly(WithPrev(1, 2)) + assertThatFlow(flowOf(2).pairwise(initialValue = 1)).emitsExactly(WithPrev(1, 2)) } @Test @@ -68,25 +64,78 @@ class PairwiseFlowTest : SysuiTestCase() { } @Test - fun withStateFlow() = runBlocking(Dispatchers.Main.immediate) { - val state = MutableStateFlow(1) - val stop = MutableSharedFlow<Unit>() - - val stoppable = merge(state, stop) - .takeWhile { it is Int } - .filterIsInstance<Int>() + fun withTransform() = runBlocking { + assertThatFlow( + flowOf("val1", "val2", "val3").pairwiseBy { prev: String, next: String -> + "$prev|$next" + } + ) + .emitsExactly("val1|val2", "val2|val3") + } - val job1 = launch { - assertThatFlow(stoppable.pairwise()).emitsExactly(WithPrev(1, 2)) - } - state.value = 2 - val job2 = launch { assertThatFlow(stoppable.pairwise()).emitsNothing() } + @Test + fun withGetInit() = runBlocking { + var initRun = false + assertThatFlow( + flowOf("val1", "val2").pairwiseBy( + getInitialValue = { + initRun = true + "initial" + } + ) { prev: String, next: String -> "$prev|$next" } + ) + .emitsExactly("initial|val1", "val1|val2") + assertThat(initRun).isTrue() + } - stop.emit(Unit) + @Test + fun notEnoughWithGetInit() = runBlocking { + var initRun = false + assertThatFlow( + emptyFlow<String>().pairwiseBy( + getInitialValue = { + initRun = true + "initial" + } + ) { prev: String, next: String -> "$prev|$next" } + ) + .emitsNothing() + // Even though the flow will not emit anything, the initial value function should still get + // run. + assertThat(initRun).isTrue() + } - assertThatJob(job1).isCompleted() - assertThatJob(job2).isCompleted() + @Test + fun getInitNotRunWhenFlowNotCollected() = runBlocking { + var initRun = false + flowOf("val1", "val2").pairwiseBy( + getInitialValue = { + initRun = true + "initial" + } + ) { prev: String, next: String -> "$prev|$next" } + + // Since the flow isn't collected, ensure [initialValueFun] isn't run. + assertThat(initRun).isFalse() } + + @Test + fun withStateFlow() = + runBlocking(Dispatchers.Main.immediate) { + val state = MutableStateFlow(1) + val stop = MutableSharedFlow<Unit>() + + val stoppable = merge(state, stop).takeWhile { it is Int }.filterIsInstance<Int>() + + val job1 = launch { assertThatFlow(stoppable.pairwise()).emitsExactly(WithPrev(1, 2)) } + state.value = 2 + val job2 = launch { assertThatFlow(stoppable.pairwise()).emitsNothing() } + + stop.emit(Unit) + + assertThatJob(job1).isCompleted() + assertThatJob(job2).isCompleted() + } } @SmallTest @@ -94,18 +143,17 @@ class PairwiseFlowTest : SysuiTestCase() { class SetChangesFlowTest : SysuiTestCase() { @Test fun simple() = runBlocking { - assertThatFlow( - flowOf(setOf(1, 2, 3), setOf(2, 3, 4)).setChanges() - ).emitsExactly( - SetChanges( - added = setOf(1, 2, 3), - removed = emptySet(), - ), - SetChanges( - added = setOf(4), - removed = setOf(1), - ), - ) + assertThatFlow(flowOf(setOf(1, 2, 3), setOf(2, 3, 4)).setChanges()) + .emitsExactly( + SetChanges( + added = setOf(1, 2, 3), + removed = emptySet(), + ), + SetChanges( + added = setOf(4), + removed = setOf(1), + ), + ) } @Test @@ -147,14 +195,19 @@ class SetChangesFlowTest : SysuiTestCase() { class SampleFlowTest : SysuiTestCase() { @Test fun simple() = runBlocking { - assertThatFlow(flow { yield(); emit(1) }.sample(flowOf(2)) { a, b -> a to b }) + assertThatFlow( + flow { + yield() + emit(1) + } + .sample(flowOf(2)) { a, b -> a to b } + ) .emitsExactly(1 to 2) } @Test fun otherFlowNoValueYet() = runBlocking { - assertThatFlow(flowOf(1).sample(emptyFlow<Unit>())) - .emitsNothing() + assertThatFlow(flowOf(1).sample(emptyFlow<Unit>())).emitsNothing() } @Test @@ -178,13 +231,14 @@ class SampleFlowTest : SysuiTestCase() { } } -private fun <T> assertThatFlow(flow: Flow<T>) = object { - suspend fun emitsExactly(vararg emissions: T) = - assertThat(flow.toList()).containsExactly(*emissions).inOrder() - suspend fun emitsNothing() = - assertThat(flow.toList()).isEmpty() -} +private fun <T> assertThatFlow(flow: Flow<T>) = + object { + suspend fun emitsExactly(vararg emissions: T) = + assertThat(flow.toList()).containsExactly(*emissions).inOrder() + suspend fun emitsNothing() = assertThat(flow.toList()).isEmpty() + } -private fun assertThatJob(job: Job) = object { - fun isCompleted() = assertThat(job.isCompleted).isTrue() -} +private fun assertThatJob(job: Job) = + object { + fun isCompleted() = assertThat(job.isCompleted).isTrue() + } |