diff --git a/app/src/main/java/ink/trmnl/android/data/TrmnlDisplayRepository.kt b/app/src/main/java/ink/trmnl/android/data/TrmnlDisplayRepository.kt index 8850c53..c82d5bf 100644 --- a/app/src/main/java/ink/trmnl/android/data/TrmnlDisplayRepository.kt +++ b/app/src/main/java/ink/trmnl/android/data/TrmnlDisplayRepository.kt @@ -1,7 +1,5 @@ package ink.trmnl.android.data -import android.content.Context -import android.os.BatteryManager import com.slack.eithernet.ApiResult import com.slack.eithernet.exceptionOrNull import com.squareup.anvil.annotations.optional.SingleIn @@ -9,7 +7,6 @@ import ink.trmnl.android.BuildConfig.USE_FAKE_API import ink.trmnl.android.data.fake.generateFakeDeviceSetupInfo import ink.trmnl.android.data.fake.generateFakeTrmnlDisplayInfo import ink.trmnl.android.di.AppScope -import ink.trmnl.android.di.ApplicationContext import ink.trmnl.android.model.SupportedDeviceModel import ink.trmnl.android.model.TrmnlDeviceConfig import ink.trmnl.android.model.TrmnlDeviceType @@ -28,11 +25,9 @@ import ink.trmnl.android.network.model.TrmnlUser import ink.trmnl.android.network.util.constructApiUrl import ink.trmnl.android.network.util.extractHttpResponseMetadata import ink.trmnl.android.network.util.extractHttpResponseMetadataFromFailure +import ink.trmnl.android.util.AndroidDeviceInfoProvider import ink.trmnl.android.util.HTTP_500 import ink.trmnl.android.util.isHttpOk -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.launch import timber.log.Timber import javax.inject.Inject @@ -49,29 +44,12 @@ import javax.inject.Inject class TrmnlDisplayRepository @Inject constructor( - @ApplicationContext private val context: Context, private val apiService: TrmnlApiService, private val userApiService: TrmnlUserApiService, private val imageMetadataStore: ImageMetadataStore, private val repositoryConfigProvider: RepositoryConfigProvider, + private val androidDeviceInfoProvider: AndroidDeviceInfoProvider, ) { - /** - * Gets the current battery level of the Android device. - * - * @return Battery percentage (0-100), or null if unable to retrieve - */ - private fun getBatteryLevel(): Int? = - try { - val batteryManager = context.getSystemService(Context.BATTERY_SERVICE) as? BatteryManager - val batteryLevel = - batteryManager?.getIntProperty(BatteryManager.BATTERY_PROPERTY_CAPACITY) - Timber.i("Current battery level: $batteryLevel%") - batteryLevel - } catch (e: Exception) { - Timber.e(e, "Failed to get battery level") - null - } - /** * Fetches display data for next plugin from the server using the provided access token. * If the app is in debug mode, it uses mock data instead. @@ -128,19 +106,6 @@ class TrmnlDisplayRepository displayInfo.imageUrl, displayInfo.refreshIntervalSeconds, ) - - // Report battery status for BYOD devices (non-blocking) - if (trmnlDeviceConfig.type == TrmnlDeviceType.BYOD && - trmnlDeviceConfig.deviceId != null && - trmnlDeviceConfig.userApiToken != null - ) { - val batteryLevel = getBatteryLevel() - if (batteryLevel != null) { - CoroutineScope(Dispatchers.IO).launch { - reportBatteryStatus(trmnlDeviceConfig, batteryLevel) - } - } - } } return displayInfo @@ -201,19 +166,6 @@ class TrmnlDisplayRepository displayInfo.imageUrl, displayInfo.refreshIntervalSeconds, ) - - // Report battery status for BYOD devices (non-blocking) - if (trmnlDeviceConfig.type == TrmnlDeviceType.BYOD && - trmnlDeviceConfig.deviceId != null && - trmnlDeviceConfig.userApiToken != null - ) { - val batteryLevel = getBatteryLevel() - if (batteryLevel != null) { - CoroutineScope(Dispatchers.IO).launch { - reportBatteryStatus(trmnlDeviceConfig, batteryLevel) - } - } - } } return displayInfo @@ -481,6 +433,45 @@ class TrmnlDisplayRepository */ } + /** + * Reports the device's battery status to the TRMNL API for BYOD devices. + * + * This is a convenience method that checks if the device is a BYOD device with the necessary + * configuration (deviceId and userApiToken), retrieves the current battery level, + * and reports it to the server. + * + * This method should be called after successful image refresh operations. + * + * @param config Device configuration containing device type, device ID, and user API token + */ + suspend fun reportDeviceBatteryStatus(config: TrmnlDeviceConfig) { + // Only report battery for BYOD devices with required configuration + if (config.type != TrmnlDeviceType.BYOD) { + Timber.d("Battery reporting skipped: not a BYOD device (type: ${config.type})") + return + } + + if (config.deviceId == null) { + Timber.w("Battery reporting skipped: device ID is null") + return + } + + if (config.userApiToken == null) { + Timber.w("Battery reporting skipped: user API token is null") + return + } + + // Get current battery level + val batteryLevel = androidDeviceInfoProvider.getBatteryLevel() + if (batteryLevel == null) { + Timber.w("Battery reporting skipped: unable to get battery level") + return + } + + // Report battery status + reportBatteryStatus(config, batteryLevel) + } + /** * Reports the device's battery status to the TRMNL API. * @@ -493,7 +484,7 @@ class TrmnlDisplayRepository * @param batteryPercent The current battery percentage (0-100) * @return A Result containing Unit on success or an exception on failure */ - suspend fun reportBatteryStatus( + private suspend fun reportBatteryStatus( config: TrmnlDeviceConfig, batteryPercent: Int, ): Result { diff --git a/app/src/main/java/ink/trmnl/android/util/AndroidDeviceInfoProvider.kt b/app/src/main/java/ink/trmnl/android/util/AndroidDeviceInfoProvider.kt new file mode 100644 index 0000000..933fb34 --- /dev/null +++ b/app/src/main/java/ink/trmnl/android/util/AndroidDeviceInfoProvider.kt @@ -0,0 +1,39 @@ +package ink.trmnl.android.util + +import android.content.Context +import android.os.BatteryManager +import com.squareup.anvil.annotations.optional.SingleIn +import ink.trmnl.android.di.AppScope +import ink.trmnl.android.di.ApplicationContext +import timber.log.Timber +import javax.inject.Inject + +/** + * Provider class for accessing Android device information. + * + * This class provides utility methods to retrieve device-specific information + * such as battery level, which can be used for reporting to the TRMNL API. + */ +@SingleIn(AppScope::class) +class AndroidDeviceInfoProvider + @Inject + constructor( + @ApplicationContext private val context: Context, + ) { + /** + * Gets the current battery level of the Android device. + * + * @return Battery percentage (0-100), or null if unable to retrieve + */ + fun getBatteryLevel(): Int? = + try { + val batteryManager = context.getSystemService(Context.BATTERY_SERVICE) as? BatteryManager + val batteryLevel = + batteryManager?.getIntProperty(BatteryManager.BATTERY_PROPERTY_CAPACITY) + Timber.i("Current battery level: $batteryLevel%") + batteryLevel + } catch (e: Exception) { + Timber.e(e, "Failed to get battery level") + null + } + } diff --git a/app/src/main/java/ink/trmnl/android/work/TrmnlImageRefreshWorker.kt b/app/src/main/java/ink/trmnl/android/work/TrmnlImageRefreshWorker.kt index bc92059..df1d4a2 100644 --- a/app/src/main/java/ink/trmnl/android/work/TrmnlImageRefreshWorker.kt +++ b/app/src/main/java/ink/trmnl/android/work/TrmnlImageRefreshWorker.kt @@ -170,6 +170,9 @@ class TrmnlImageRefreshWorker( httpResponseMetadata = trmnlDisplayInfo.httpResponseMetadata, ) + // Report battery status for BYOD devices after successful image refresh + displayRepository.reportDeviceBatteryStatus(deviceConfig) + // NOTE: Image metadata caching is handled automatically by `TrmnlDisplayRepository` // when the API call succeeds, so we don't need to save it again here. // See https://github.com/usetrmnl/trmnl-android/issues/195 diff --git a/app/src/test/java/ink/trmnl/android/data/TrmnlDisplayRepositoryTest.kt b/app/src/test/java/ink/trmnl/android/data/TrmnlDisplayRepositoryTest.kt index ab8c706..2fa7245 100644 --- a/app/src/test/java/ink/trmnl/android/data/TrmnlDisplayRepositoryTest.kt +++ b/app/src/test/java/ink/trmnl/android/data/TrmnlDisplayRepositoryTest.kt @@ -1,6 +1,5 @@ package ink.trmnl.android.data -import android.content.Context import com.google.common.truth.Truth.assertThat import com.slack.eithernet.ApiResult import ink.trmnl.android.model.TrmnlDeviceConfig @@ -10,6 +9,7 @@ import ink.trmnl.android.network.TrmnlUserApiService import ink.trmnl.android.network.model.TrmnlCurrentImageResponse import ink.trmnl.android.network.model.TrmnlDisplayResponse import ink.trmnl.android.network.util.constructApiUrl +import ink.trmnl.android.util.AndroidDeviceInfoProvider import io.mockk.coEvery import io.mockk.coVerify import io.mockk.every @@ -29,12 +29,12 @@ import org.junit.Test @OptIn(com.slack.eithernet.InternalEitherNetApi::class) class TrmnlDisplayRepositoryTest { private lateinit var repository: TrmnlDisplayRepository - private lateinit var context: Context private lateinit var apiService: TrmnlApiService private lateinit var userApiService: TrmnlUserApiService private lateinit var imageMetadataStore: ImageMetadataStore private lateinit var repositoryConfigProvider: RepositoryConfigProvider private lateinit var deviceConfigDataStore: TrmnlDeviceConfigDataStore + private lateinit var androidDeviceInfoProvider: AndroidDeviceInfoProvider private val testDeviceConfig = TrmnlDeviceConfig( @@ -62,22 +62,22 @@ class TrmnlDisplayRepositoryTest { @Before fun setup() { - context = mockk(relaxed = true) apiService = mockk() userApiService = mockk() repositoryConfigProvider = mockk() deviceConfigDataStore = mockk() imageMetadataStore = mockk(relaxed = true) + androidDeviceInfoProvider = mockk(relaxed = true) every { repositoryConfigProvider.shouldUseFakeData } returns false repository = TrmnlDisplayRepository( - context = context, apiService = apiService, userApiService = userApiService, imageMetadataStore = imageMetadataStore, repositoryConfigProvider = repositoryConfigProvider, + androidDeviceInfoProvider = androidDeviceInfoProvider, ) } @@ -587,7 +587,7 @@ class TrmnlDisplayRepositoryTest { } @Test - fun `reportBatteryStatus should succeed with valid BYOD config`() = + fun `reportDeviceBatteryStatus should report battery for valid BYOD config`() = runTest { // Arrange val byodConfigWithDeviceId = @@ -596,6 +596,8 @@ class TrmnlDisplayRepositoryTest { userApiToken = "user_test_token", ) + every { androidDeviceInfoProvider.getBatteryLevel() } returns 75 + val expectedApiUrl = "https://server.example.com/api/devices/123" coEvery { @@ -607,22 +609,38 @@ class TrmnlDisplayRepositoryTest { } returns ApiResult.success(mockk(relaxed = true)) // Act - val result = repository.reportBatteryStatus(byodConfigWithDeviceId, 85) - - // Assert - assertThat(result.isSuccess).isTrue() + repository.reportDeviceBatteryStatus(byodConfigWithDeviceId) + // Assert - Verify battery status was reported coVerify { userApiService.updateDevice( fullApiUrl = expectedApiUrl, accessToken = "Bearer user_test_token", - updateRequest = match { it.percentCharged == 85.0 }, + updateRequest = match { it.percentCharged == 75.0 }, ) } } @Test - fun `reportBatteryStatus should fail when deviceId is null`() = + fun `reportDeviceBatteryStatus should skip for non-BYOD device`() = + runTest { + // Arrange - TRMNL device (not BYOD) + val trmnlConfig = + testDeviceConfig.copy( + deviceId = 123, + userApiToken = "user_test_token", + ) + + // Act + repository.reportDeviceBatteryStatus(trmnlConfig) + + // Assert - Verify API was NOT called + coVerify(exactly = 0) { userApiService.updateDevice(any(), any(), any()) } + coVerify(exactly = 0) { androidDeviceInfoProvider.getBatteryLevel() } + } + + @Test + fun `reportDeviceBatteryStatus should skip when deviceId is null`() = runTest { // Arrange val configWithoutDeviceId = @@ -632,19 +650,15 @@ class TrmnlDisplayRepositoryTest { ) // Act - val result = repository.reportBatteryStatus(configWithoutDeviceId, 85) + repository.reportDeviceBatteryStatus(configWithoutDeviceId) - // Assert - assertThat(result.isFailure).isTrue() - assertThat(result.exceptionOrNull()).isInstanceOf(IllegalStateException::class.java) - assertThat(result.exceptionOrNull()?.message).contains("Device ID is required") - - // Verify API was NOT called + // Assert - Verify API was NOT called coVerify(exactly = 0) { userApiService.updateDevice(any(), any(), any()) } + coVerify(exactly = 0) { androidDeviceInfoProvider.getBatteryLevel() } } @Test - fun `reportBatteryStatus should fail when userApiToken is null`() = + fun `reportDeviceBatteryStatus should skip when userApiToken is null`() = runTest { // Arrange val configWithoutUserToken = @@ -654,19 +668,15 @@ class TrmnlDisplayRepositoryTest { ) // Act - val result = repository.reportBatteryStatus(configWithoutUserToken, 85) + repository.reportDeviceBatteryStatus(configWithoutUserToken) - // Assert - assertThat(result.isFailure).isTrue() - assertThat(result.exceptionOrNull()).isInstanceOf(IllegalStateException::class.java) - assertThat(result.exceptionOrNull()?.message).contains("User API token is required") - - // Verify API was NOT called + // Assert - Verify API was NOT called coVerify(exactly = 0) { userApiService.updateDevice(any(), any(), any()) } + coVerify(exactly = 0) { androidDeviceInfoProvider.getBatteryLevel() } } @Test - fun `reportBatteryStatus should handle API failure`() = + fun `reportDeviceBatteryStatus should skip when battery level unavailable`() = runTest { // Arrange val byodConfigWithDeviceId = @@ -675,46 +685,13 @@ class TrmnlDisplayRepositoryTest { userApiToken = "user_test_token", ) - val expectedApiUrl = "https://server.example.com/api/devices/123" - val apiException = java.io.IOException("Network error") - val httpFailure: ApiResult.Failure = - ApiResult.networkFailure(apiException) - - coEvery { - userApiService.updateDevice( - fullApiUrl = expectedApiUrl, - accessToken = "Bearer user_test_token", - updateRequest = any(), - ) - } returns httpFailure + every { androidDeviceInfoProvider.getBatteryLevel() } returns null // Act - val result = repository.reportBatteryStatus(byodConfigWithDeviceId, 85) + repository.reportDeviceBatteryStatus(byodConfigWithDeviceId) - // Assert - assertThat(result.isFailure).isTrue() - assertThat(result.exceptionOrNull()).isEqualTo(apiException) - } - - @Test - fun `reportBatteryStatus should skip API call in fake data mode`() = - runTest { - // Arrange - every { repositoryConfigProvider.shouldUseFakeData } returns true - - val byodConfigWithDeviceId = - byodDeviceConfig.copy( - deviceId = 123, - userApiToken = "user_test_token", - ) - - // Act - val result = repository.reportBatteryStatus(byodConfigWithDeviceId, 85) - - // Assert - assertThat(result.isSuccess).isTrue() - - // Verify API was NOT called + // Assert - Verify battery level was requested but API was NOT called + coVerify(exactly = 1) { androidDeviceInfoProvider.getBatteryLevel() } coVerify(exactly = 0) { userApiService.updateDevice(any(), any(), any()) } } }