Skip to content

Comments

refactor(ui): compose resources, domain layer#4628

Merged
jamesarich merged 18 commits intomainfrom
chore/cleanup
Feb 23, 2026
Merged

refactor(ui): compose resources, domain layer#4628
jamesarich merged 18 commits intomainfrom
chore/cleanup

Conversation

@jamesarich
Copy link
Collaborator

This pull request primarily migrates all string and resource management from the core:strings module to a new centralized core:resources module, and updates all relevant references and documentation throughout the codebase. Additionally, it introduces a new use case for device discovery and improves robustness for service initialization during testing.

Resource module migration and documentation updates:

  • All code and documentation references to core:strings have been updated to core:resources, including import paths, Gradle dependencies, and module diagrams. This centralizes string, drawable, and other resource management in :core:resources and clarifies best practices for resource placement and usage. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12]

New functionality:

  • Added GetDiscoveredDevicesUseCase in app/src/main/java/com/geeksville/mesh/domain/usecase/, which aggregates BLE, USB, and TCP devices, as well as recent TCP connections, into a unified flow for device discovery in the app.

Codebase robustness and minor improvements:

  • Improved MeshService startup logic to gracefully handle Hilt initialization errors during tests, preventing test failures due to premature service creation.
  • Minor code formatting improvement in app/build.gradle.kts for variant application ID assignment.

Summary of most important changes:

Resource Management Migration

  • All references to strings and resources have been moved from core:strings to core:resources, including imports, Gradle dependencies, and documentation. This centralizes global resources and clarifies modular boundaries for feature-specific assets. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12]

New Device Discovery Use Case

  • Introduced GetDiscoveredDevicesUseCase, which combines BLE, USB, TCP, and recent device discovery logic into a single flow for use in the app's device list.

Service Initialization Robustness

  • Enhanced MeshService to catch and handle Hilt initialization errors during tests, improving reliability in CI and test environments.

Documentation and Diagram Updates

  • Updated documentation (AGENTS.md, CONTRIBUTING.md, and module diagrams in app/README.md) to reflect the new resource module structure and best practices for resource management. [1] [2] [3] [4] [5]

Minor Code Quality Improvements

  • Improved code formatting in app/build.gradle.kts for clarity and consistency.

…divider bug

This commit optimizes the performance of the message list and resolves a UI issue where the "Unread Messages" divider could overlap with a message item.

Key changes include:
- The logic for finding the unread divider index and navigating to a replied-to message now uses `itemSnapshotList`. This avoids performing searches on the lazy-loaded `PagingData` during composition, leading to better performance and stability.
- The unread divider and its corresponding message item are now wrapped in a `Column`. This ensures they are rendered correctly in a single animated block, preventing the visual overlap bug.
- Item animation modifiers have been streamlined to reduce redundant code.
- `MessageStatusDialog` is now marked as private as it's only used within its own file.

Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com>
This commit optimizes the message screen's performance and stability by leveraging `itemSnapshotList` from the Paging 3 library. This avoids triggering unnecessary page loads when performing operations on the currently loaded items.

Key optimizations include:
- Finding the first unread message without causing new pages to load.
- Looking up the original message for a reply from the snapshot.
- Copying and selecting messages now operate on the `itemSnapshotList`, preventing accidental fetches of the entire message history.

Additionally, `LazyPagingItems` now includes a `contentType` to improve `LazyColumn` performance and efficiency. A minor function rename from `renderPagedChatMessageRow` to `RenderPagedChatMessageRow` has been made to align with composable function naming conventions.

Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com>
This commit introduces several performance optimizations to the messaging feature to reduce jank and unnecessary recompositions.

- The `MessageListHandlers` and `MessageListPagedState` data classes are now marked as `@Stable` to help the Compose compiler avoid unnecessary recompositions.
- The use of `derivedStateOf` has been removed for simple calculations that are dependent on list size changes, such as finding the unread divider index and counting remote messages.
- `LazyPagingItems.peek()` is now used to access adjacent items in the list. This prevents the premature fetching of pages and reduces potential performance overhead.

Additionally, a minor formatting change was made in `app/build.gradle.kts`.

Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com>
…access

This commit refactors the messaging feature to remove the usage of `PagingData`'s `itemSnapshotList`. Direct access to the `LazyPagingItems` collection is now used, which simplifies the code and resolves an issue where the snapshot could become stale, leading to incorrect behavior.

Key changes include:
-   Finding the first unread message, the message to reply to, and selected messages for clipboard operations now iterate the `LazyPagingItems` directly.
-   The "Select All" functionality is now aligned with the currently loaded items in the `LazyPagingItems`.
-   Several `derivedStateOf` usages have been refined to trigger recalculations only when necessary.
-   A minor KDoc fix in `MessageTopAppBar` clarifies the `title` parameter.

In the `FirmwareReleaseRepository`, logic has been added to explicitly emit `null` if no cached firmware release is found, ensuring the UI receives a clear signal.

Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com>
This commit introduces a `GetNodeDetailsUseCase` to encapsulate and centralize the business logic for fetching and assembling all data required for the node detail screen. This significantly cleans up the `NodeDetailViewModel` and `MetricsViewModel`, removing redundant data-fetching code and improving maintainability.

The new use case is responsible for combining flows from multiple repositories (`NodeRepository`, `MeshLogRepository`, `RadioConfigRepository`, etc.) to build a comprehensive `NodeDetailUiState`. This state includes node identity, metrics, configuration, hardware details, and available log types.

Key changes:
- **`GetNodeDetailsUseCase`**: A new use case that serves as a single source of truth for node detail data.
- **`NodeDetailViewModel`**: Refactored to use `GetNodeDetailsUseCase`, drastically simplifying its implementation. State management is now driven directly by the use case output.
- **`MetricsViewModel`**: Also refactored to consume `GetNodeDetailsUseCase`, eliminating its own complex data-fetching and state-building logic. It now focuses solely on view-specific concerns like time frame filtering and UI interactions.
- **`NodeDetailScreen.kt`**: The `viewModel.start(nodeId)` call is now wrapped in a `LaunchedEffect` to ensure it's executed correctly based on the `nodeId`.

Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com>
This commit refactors the application by extracting business logic from ViewModels into dedicated use case classes. This improves separation of concerns, making the code more modular, testable, and maintainable.

The following use cases have been created:

-   `GetFilteredNodesUseCase`: Encapsulates the logic for filtering and sorting the node list, which was previously in `NodeListViewModel`.
-   `SendMessageUseCase`: Centralizes the process of sending messages, including handling direct message logic (favoriting nodes or sending contacts) and applying homoglyph transformations. This logic was moved from `MessageViewModel`.
-   `GetDiscoveredDevicesUseCase`: Consolidates the logic for finding, processing, and matching BLE, USB, and TCP devices with nodes in the database. This simplifies the `ScannerViewModel` by abstracting away the complexities of device discovery and enrichment.

The respective ViewModels (`NodeListViewModel`, `MessageViewModel`, and `ScannerViewModel`) have been updated to inject and utilize these new use cases, resulting in cleaner and more focused ViewModel implementations.

Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com>
This commit introduces unit tests for several domain use cases across different features, improving code coverage and ensuring the correctness of business logic.

- **`SendMessageUseCaseTest`**: Verifies the logic for sending messages. It includes tests for:
    - Sending broadcast messages.
    - Automatically favoriting a node when sending a direct message to a device with older firmware.
    - Sending a shared contact when messaging a device with newer firmware.
    - Correctly applying homoglyph character transformations when the feature is enabled.

- **`GetFilteredNodesUseCaseTest`**: Covers the node filtering logic. Tests ensure that:
    - Nodes are correctly filtered based on various criteria passed to the repository.
    - Ignored nodes are properly excluded from the results when requested.
    - Infrastructure nodes (like `ROUTER` and `REPEATER`) are filtered out when the `excludeInfrastructure` flag is set.

- **`GetDiscoveredDevicesUseCaseTest`**: Adds tests for the device discovery mechanism.
    - Confirms that the "Demo Mode" device is included in the list when the `showMock` flag is true.
    - Verifies that empty lists are returned when no devices are discovered and the mock flag is false.

Additionally, necessary testing dependencies like `mockk` and `kotlinx-coroutines-test` have been added to the `feature:messaging` module's build configuration to support these new tests.

Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com>
The source path in the `crowdin.yml` configuration has been updated to align with the directory structure of Compose Resources.

The path was changed from `/**/src/commonMain/composeResources/values/strings.xml` to `/**/composeResources/values/strings.xml` to correctly locate the source `strings.xml` file for translation.

Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com>
…and `NordicBleInterfaceDrainTest`

This commit removes two test files that are no longer necessary.

`GetDiscoveredDevicesUseCaseTest` has been made obsolete by more comprehensive and robust end-to-end (E2E) tests that cover the device discovery functionality more effectively.

`NordicBleInterfaceDrainTest` is also removed. Its functionality for testing packet draining has been superseded and integrated into the broader E2E testing framework, making this specialized test redundant.

Additionally, a minor Kotlin syntax update was made in `BluetoothRepository.kt`, changing an explicit property getter to a computed property for the `state` variable.

Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com>
Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com>
…ution

This commit centralizes the `UiText` utility by moving it from the `feature/settings` module to the `core/resources` module, making it accessible across the entire application. This refactoring eliminates a duplicate implementation and provides a single, consistent way to handle dynamic and resource-based strings in the UI.

The `UiText.StringResource` class has been renamed to `UiText.Resource` for clarity. It now supports nested `StringResource` arguments, which are resolved recursively. A new `resolve()` suspend function has been added to allow string resolution from non-composable contexts, such as showing a snackbar.

Throughout the `feature/node` module, calls to show snackbar feedback have been updated to use the new `UiText.resolve()` method. This simplifies the presentation logic in various metric and detail screens.

Additionally, node name resolution has been improved. For nodes where the hardware model is not yet known, a fallback name is now generated using the last four characters of the node's ID (e.g., "Meshtastic 1a2b"). The `NodeDetailUiState` now includes a `nodeName` property of type `UiText` to handle this logic, ensuring the UI displays a meaningful name even for partially discovered nodes.

Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com>
Catches an `IllegalStateException` that can be thrown during `MeshService.onCreate()` in a testing environment.

This issue occurs if the service is started by the Android system (for example, after a crash or on boot) before the `HiltAndroidRule` has initialized the necessary Hilt components for the test.

The fix specifically checks if the exception message contains "HiltAndroidRule". If it does, the exception is logged as a warning, and `stopSelf()` is called to gracefully shut down the service, preventing the test from crashing. If the exception is unrelated to Hilt, it is re-thrown.

Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com>
@codecov
Copy link

codecov bot commented Feb 23, 2026

Codecov Report

❌ Patch coverage is 0% with 57 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@96adc70). Learn more about missing BASE report.
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...mesh/domain/usecase/GetDiscoveredDevicesUseCase.kt 0.00% 43 Missing ⚠️
...geeksville/mesh/ui/connections/ScannerViewModel.kt 0.00% 14 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4628   +/-   ##
=======================================
  Coverage        ?   16.10%           
=======================================
  Files           ?       85           
  Lines           ?     4124           
  Branches        ?      724           
=======================================
  Hits            ?      664           
  Misses          ?     3334           
  Partials        ?      126           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jamesarich jamesarich added this pull request to the merge queue Feb 23, 2026
Merged via the queue into main with commit 2676a51 Feb 23, 2026
9 checks passed
@jamesarich jamesarich deleted the chore/cleanup branch February 23, 2026 03:50
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