Skip to content

Fix race condition widget refactor 6966862986207730827#116

Closed
harrydbarnes wants to merge 16 commits intomainfrom
fix-race-condition-widget-refactor-6966862986207730827
Closed

Fix race condition widget refactor 6966862986207730827#116
harrydbarnes wants to merge 16 commits intomainfrom
fix-race-condition-widget-refactor-6966862986207730827

Conversation

@harrydbarnes
Copy link
Owner

No description provided.

google-labs-jules bot and others added 16 commits January 20, 2026 07:02
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
- Implemented permission result handling in MainActivity to refresh data or show feedback.
- Replaced hardcoded SimpleDateFormat with DateUtils in CalendarCard to respect user time format preference.
- Added TODO in MainViewModel regarding hardcoded news category.
- Replaced e.printStackTrace() with Log.e() in repositories for better error tracking.
- Added TAG constants to repositories.
- Implemented permission result handling in MainActivity.
- Replaced hardcoded date format with DateUtils in CalendarCard.
- Re-implemented dynamic news category selection in MainViewModel.
- Added isLoading state to MainViewModel.
- Replaced e.printStackTrace() with Log.e() in repositories.
- Updated repositories to return Boolean success/failure status.
- Implemented transactional replaceAll in ArticleDao/NewsRepository to prevent data loss.
- Implemented retry logic in WidgetUpdateWorker based on refresh success.
- Created missing colors.xml to fix build failure.
- Implemented settings navigation in HomeScreen.
- Implemented permission result handling in MainActivity.
- Replaced hardcoded date format with DateUtils in CalendarCard.
- Re-implemented dynamic news category selection in MainViewModel.
- Added isLoading state to MainViewModel.
- Fixed combine function signature in MainViewModel to handle 8 flows.
- Replaced e.printStackTrace() with Log.e() in repositories.
- Updated repositories to return Boolean success/failure status.
- Implemented transactional replaceAll in ArticleDao/NewsRepository to prevent data loss.
- Implemented retry logic in WidgetUpdateWorker based on refresh success.
- Created missing colors.xml to fix build failure.
- Implemented settings navigation in HomeScreen.
…Repository.kt

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
- Implemented permission result handling in MainActivity.
- Replaced hardcoded date format with DateUtils in CalendarCard.
- Re-implemented dynamic news category selection in MainViewModel.
- Added isLoading state to MainViewModel.
- Fixed combine function signature in MainViewModel to handle 8 flows.
- Replaced e.printStackTrace() with Log.e() in repositories.
- Updated repositories to return Boolean success/failure status.
- Implemented transactional replaceAll in ArticleDao/NewsRepository to prevent data loss.
- Implemented retry logic in WidgetUpdateWorker based on refresh success.
- Created missing colors.xml to fix build failure.
- Implemented settings navigation in HomeScreen.
- Fixed syntax error in CalendarRepository.
- Implemented permission result handling in MainActivity.
- Replaced hardcoded date format with DateUtils in CalendarCard.
- Re-implemented dynamic news category selection in MainViewModel.
- Added isLoading state to MainViewModel.
- Refactored MainViewModel combine logic to use chained calls for type safety.
- Replaced e.printStackTrace() with Log.e() in repositories and DayAheadWidget.
- Updated repositories to return Boolean success/failure status.
- Implemented transactional replaceAll in ArticleDao/NewsRepository.
- Implemented retry logic in WidgetUpdateWorker.
- Created missing colors.xml to fix build failure.
- Implemented settings navigation in HomeScreen.
- Fixed CalendarRepository syntax error and extracted magic number.
- Added @JvmStatic to AppUtils for Java interop.
- Extracted Hilt versions in build.gradle.
- Added null/blank title filtering in NewsRepository.
- Implemented permission result handling in MainActivity.
- Replaced hardcoded date format with DateUtils in CalendarCard.
- Re-implemented dynamic news category selection in MainViewModel.
- Added isLoading state to MainViewModel.
- Refactored MainViewModel combine logic to use chained calls for type safety.
- Replaced e.printStackTrace() with Log.e() in repositories and DayAheadWidget.
- Updated repositories to return Boolean success/failure status.
- Implemented transactional replaceAll in ArticleDao/NewsRepository.
- Implemented retry logic in WidgetUpdateWorker.
- Created missing colors.xml to fix build failure.
- Implemented settings navigation in HomeScreen.
- Fixed CalendarRepository syntax error and extracted magic number.
- Added @JvmStatic to AppUtils for Java interop.
- Extracted Hilt versions in build.gradle.
- Added null/blank title filtering in NewsRepository.
- Implemented location fetching in MainActivity and updated MainViewModel.
- Updated Geocoder usage for API 33+.
- Updated `fetchLocationName` in `MainViewModel` to use `suspendCancellableCoroutine` for the Tiramisu `Geocoder` API, preventing a race condition where the coroutine scope would close before the callback fired.
- Refactored `DayAheadWidget` to inject `WeatherRepository` instead of `WeatherDao` and `Gson`.
- Simplified `DayAheadWidget.onUpdate` to fetch parsed weather data directly from the repository flow, removing data access and parsing logic from the UI component.
@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @harrydbarnes, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request undertakes a comprehensive modernization of the application's architecture and user interface. It transitions the UI from legacy Android Views to the declarative Jetpack Compose framework, establishes a robust and reactive data layer using Room for persistence and Kotlin Flow for data streams, and integrates Dagger Hilt for efficient dependency management. A key aspect of this refactor is addressing a race condition in the DayAheadWidget by rewriting it in Kotlin with proper asynchronous handling, leading to a more stable and maintainable codebase.

Highlights

  • Complete UI Migration to Jetpack Compose: The entire user interface, including MainActivity and various UI components, has been rewritten from traditional Android XML layouts and Java to Jetpack Compose and Kotlin. This significantly modernizes the UI development stack.
  • Introduction of Dagger Hilt for Dependency Injection: Dagger Hilt is now integrated across the application to manage dependencies, improving modularity, testability, and code organization. This includes setting up an Application class for Hilt and injecting dependencies into ViewModels and Workers.
  • Robust Data Layer with Room and Kotlin Flow: A new data layer has been implemented using Room Persistence Library for local caching and Kotlin Flow for reactive data streams. This provides a more structured, efficient, and reactive way to handle data from various sources (weather, news, calendar).
  • Widget Refactor to Kotlin with Coroutines and Hilt: The DayAheadWidget has been fully refactored to Kotlin, leveraging Coroutines for asynchronous operations and Hilt for dependency injection. This refactor specifically addresses a race condition by ensuring data fetching and widget updates are handled safely and asynchronously.
  • Removal of Health Connect Integration: The previous Health Connect integration has been removed as part of this refactoring.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This is an impressive and extensive refactoring of the application from a traditional View-based architecture to a modern one using Jetpack Compose, Hilt, Room, and Kotlin Coroutines/Flows. The new architecture is much cleaner, more maintainable, and follows current Android best practices. The use of the repository pattern, a reactive UI layer with StateFlow, and dependency injection is excellent.

I've identified a potential bug in the geocoder implementation that could cause a coroutine to hang, and I've also provided a couple of suggestions for architectural and UX improvements. Overall, this is a fantastic update that significantly improves the quality of the codebase.

Comment on lines +179 to +190
suspendCancellableCoroutine { cont ->
geocoder.getFromLocation(lat, lon, 1) { addresses ->
if (addresses.isNotEmpty()) {
val address = addresses[0]
val name = address.locality ?: address.subAdminArea ?: "Unknown Location"
_locationName.value = name
}
if (cont.isActive) {
cont.resume(Unit)
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The suspendCancellableCoroutine block for fetching the location name on Android Tiramisu (API 33+) and above does not handle the error case for the geocoder.getFromLocation callback. If the geocoding service returns an error, the onError part of the listener is not implemented, and the coroutine may never resume, causing it to hang indefinitely. You should implement the onError callback to handle failures gracefully by using the full GeocodeListener interface.

                         suspendCancellableCoroutine { cont ->
                             val listener = object : Geocoder.GeocodeListener {
                                 override fun onGeocode(addresses: MutableList<Address>) {
                                     if (addresses.isNotEmpty()) {
                                         val address = addresses[0]
                                         val name = address.locality ?: address.subAdminArea ?: "Unknown Location"
                                         _locationName.value = name
                                     }
                                     if (cont.isActive) cont.resume(Unit)
                                 }

                                 override fun onError(errorMessage: String?) {
                                     if (cont.isActive) cont.resume(Unit) // Resume even on error
                                 }
                             }
                             geocoder.getFromLocation(lat, lon, 1, listener)
                         }

Comment on lines +59 to +61
private val prefs = context.getSharedPreferences(AppConstants.PREFS_NAME, Context.MODE_PRIVATE)
private val _tempUnit = MutableStateFlow(prefs.getString(AppConstants.KEY_TEMP_UNIT, AppConstants.DEFAULT_TEMP_UNIT) ?: "celsius")
private val _userName = MutableStateFlow(prefs.getString(AppConstants.KEY_USER_NAME, "User") ?: "User")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The ViewModel is directly accessing SharedPreferences. To improve separation of concerns, testability, and align with the repository pattern used elsewhere in the app, consider abstracting SharedPreferences access into a dedicated repository (e.g., UserPreferencesRepository). This repository could expose user preferences like temperature unit and user name as a Flow, which can then be combined with other data sources in the ViewModel.

Comment on lines +56 to +61
onClick = {
val intent = Intent(android.provider.Settings.ACTION_APPLICATION_DETAILS_SETTINGS).apply {
data = Uri.fromParts("package", context.packageName, null)
}
context.startActivity(intent)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The onClick handler for the WeatherCard currently opens the application's details page in the system settings. This is an unconventional user experience. Typically, a user might expect this action to either refresh the weather data or navigate to a more detailed forecast screen. The previous version of the app navigated to a dedicated SettingsActivity. Please consider changing this to a more intuitive action, such as navigating to the app's settings screen.

                    onClick = {
                        // For example, navigate to the app's settings screen.
                        // context.startActivity(Intent(context, SettingsActivity::class.java))
                    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant