refactor: migrate :core:database to Room Kotlin Multiplatform#4702
Merged
jamesarich merged 6 commits intomainfrom Mar 4, 2026
Merged
refactor: migrate :core:database to Room Kotlin Multiplatform#4702jamesarich merged 6 commits intomainfrom
jamesarich merged 6 commits intomainfrom
Conversation
- Move Room entities, DAOs, converters, and the `MeshtasticDatabase` definition from `androidMain` to `commonMain`. - Implement `MeshtasticDatabaseConstructor` using the Room KMP pattern. - Update `DatabaseManager` to use `BundledSQLiteDriver` and shared database configuration. - Relocate `DatabaseConstants` and address normalization helpers to `commonMain`, utilizing `okio` for cross-platform SHA-1 hashing. - Move `DatabaseModule` to `:core:data` and refactor `LocalDataSource` classes to access DAOs via `DatabaseManager` instead of direct injection. - Update build configuration to use the KMP library plugin and add required Room/SQLite KMP dependencies. - Update database tests to utilize the new constructor-based initialization. Signed-off-by: James Rich <[email protected]>
Contributor
There was a problem hiding this comment.
Pull request overview
Migrates :core:database to Room Kotlin Multiplatform so the Room schema/DAOs/entities can live in commonMain and the database can be constructed via the KMP @ConstructedBy pattern, with Android using the bundled SQLite driver.
Changes:
- Moved Room entities/DAOs/converters and the
MeshtasticDatabasedefinition tocommonMain, adding the KMP constructor pattern. - Updated Android
DatabaseManagerand DI wiring so callers access DAOs viaDatabaseManagerand use shared DB builder configuration (BundledSQLiteDriver, IO query context). - Updated build setup and tests to use the constructor-based Room builders.
Reviewed changes
Copilot reviewed 19 out of 32 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| gradle/libs.versions.toml | Adds androidx.sqlite:sqlite-bundled version catalog entry for Room KMP driver usage. |
| core/database/src/test/kotlin/org/meshtastic/core/database/dao/MigrationTest.kt | Updates DB initialization to use the KMP constructor-based in-memory builder. |
| core/database/src/main/kotlin/org/meshtastic/core/database/di/DatabaseModule.kt | Removes legacy Hilt module that provided a singleton DB + injected DAOs directly. |
| core/database/src/commonMain/kotlin/org/meshtastic/core/database/entity/TracerouteNodePositionEntity.kt | Adds traceroute node position entity in commonMain. |
| core/database/src/commonMain/kotlin/org/meshtastic/core/database/entity/QuickChatAction.kt | Updates header/formatting as part of commonMain move. |
| core/database/src/commonMain/kotlin/org/meshtastic/core/database/entity/Packet.kt | Adds packet/contact/reaction entities and mapping helpers in commonMain. |
| core/database/src/commonMain/kotlin/org/meshtastic/core/database/entity/NodeEntity.kt | Adds node + metadata entities and relation mapping in commonMain. |
| core/database/src/commonMain/kotlin/org/meshtastic/core/database/entity/MyNodeEntity.kt | Adds “my node” entity in commonMain. |
| core/database/src/commonMain/kotlin/org/meshtastic/core/database/entity/MeshLog.kt | Adds mesh log entity and helpers in commonMain. |
| core/database/src/commonMain/kotlin/org/meshtastic/core/database/entity/FirmwareReleaseEntity.kt | Adds firmware release entity + mapping helpers in commonMain. |
| core/database/src/commonMain/kotlin/org/meshtastic/core/database/entity/DeviceHardwareEntity.kt | Adds device hardware entity + mapping helpers in commonMain. |
| core/database/src/commonMain/kotlin/org/meshtastic/core/database/dao/TracerouteNodePositionDao.kt | Updates header/formatting as part of commonMain move. |
| core/database/src/commonMain/kotlin/org/meshtastic/core/database/dao/QuickChatActionDao.kt | Adds Quick Chat DAO in commonMain. |
| core/database/src/commonMain/kotlin/org/meshtastic/core/database/dao/PacketDao.kt | Adds Packet DAO in commonMain (paging, queries, migrations helpers). |
| core/database/src/commonMain/kotlin/org/meshtastic/core/database/dao/NodeInfoDao.kt | Adds NodeInfo DAO and PK/public-key resolution logic in commonMain. |
| core/database/src/commonMain/kotlin/org/meshtastic/core/database/dao/MeshLogDao.kt | Adds MeshLog DAO in commonMain. |
| core/database/src/commonMain/kotlin/org/meshtastic/core/database/dao/FirmwareReleaseDao.kt | Updates header/formatting as part of commonMain move. |
| core/database/src/commonMain/kotlin/org/meshtastic/core/database/dao/DeviceHardwareDao.kt | Adds DeviceHardware DAO in commonMain. |
| core/database/src/commonMain/kotlin/org/meshtastic/core/database/MeshtasticDatabaseConstructor.kt | Introduces Room KMP expect constructor entrypoint. |
| core/database/src/commonMain/kotlin/org/meshtastic/core/database/MeshtasticDatabase.kt | Adds @ConstructedBy and a shared configureCommon() builder configuration using bundled SQLite. |
| core/database/src/commonMain/kotlin/org/meshtastic/core/database/DatabaseConstants.kt | Moves DB naming/normalization/anonymization helpers to commonMain and uses okio hashing. |
| core/database/src/commonMain/kotlin/org/meshtastic/core/database/Converters.kt | Adds KMP-safe Room type converters in commonMain. |
| core/database/src/androidTest/kotlin/org/meshtastic/core/database/dao/PacketDaoTest.kt | Updates in-memory DB creation to use KMP constructor-based builder. |
| core/database/src/androidTest/kotlin/org/meshtastic/core/database/dao/NodeInfoDaoTest.kt | Updates in-memory DB creation to use KMP constructor-based builder. |
| core/database/src/androidTest/kotlin/org/meshtastic/core/database/MeshtasticDatabaseTest.kt | Updates migration test builder to use KMP constructor + shared configuration. |
| core/database/src/androidMain/kotlin/org/meshtastic/core/database/DatabaseManager.kt | Updates Android DB creation to use constructor-based builder + common configuration; refactors DB name tracking. |
| core/database/build.gradle.kts | Switches module to KMP library plugin and rewires dependencies/KSP for Room KMP. |
| core/database/README.md | Updates module documentation to reflect Room KMP and new component layout. |
| core/data/src/main/kotlin/org/meshtastic/core/data/di/DatabaseModule.kt | Moves Hilt binding of DatabaseManager into :core:data. |
| core/data/src/main/kotlin/org/meshtastic/core/data/datasource/FirmwareReleaseLocalDataSource.kt | Refactors to obtain DAO via DatabaseManager rather than direct injection. |
| core/data/src/main/kotlin/org/meshtastic/core/data/datasource/DeviceHardwareLocalDataSource.kt | Refactors to obtain DAO via DatabaseManager rather than direct injection. |
| core/data/build.gradle.kts | Adds sqlite-bundled dependency alongside Room deps. |
Comments suppressed due to low confidence (2)
core/database/src/androidMain/kotlin/org/meshtastic/core/database/DatabaseManager.kt:132
withDb()derivesactivefrom_currentAddress.valuewhile using the DB instance from_currentDb.value. BecauseswitchActiveDatabase()updates_currentDband_currentAddressseparately (andwithDb()does not take the mutex), there is a race wherewithDb()can observe a new DB with the previous address (or vice versa) and recordlastUsedagainst the wrong DB name. Consider tracking the activedbNamein the same atomic state as the DB (e.g., a singleStateFlow/data class holding both, or an additional_currentDbNameupdated under the same mutex) and use that formarkLastUsed.
core/database/src/androidMain/kotlin/org/meshtastic/core/database/DatabaseManager.kt:203setCacheLimit()computesactiveusing_currentAddress.valuewhen_currentDb.valueis non-null. Since_currentDband_currentAddressare updated independently and this method doesn’t take the mutex, it can record/enforce eviction against the wrong active DB name during a concurrentswitchActiveDatabase(). Using a single source of truth for the activedbName(updated atomically under the mutex) would avoid this inconsistency.
You can also share your feedback on Copilot code review. Take the survey.
core/database/src/androidTest/kotlin/org/meshtastic/core/database/MeshtasticDatabaseTest.kt
Outdated
Show resolved
Hide resolved
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4702 +/- ##
======================================
Coverage ? 7.94%
======================================
Files ? 64
Lines ? 2657
Branches ? 309
======================================
Hits ? 211
Misses ? 2414
Partials ? 32 ☔ View full report in Codecov by Sentry. |
- Move existing instrumented tests from `androidTest` to `androidDeviceTest`. - Move existing unit tests from `test` to `androidHostTest`. - Update `build.gradle.kts` to configure the new test source sets and associated dependencies. - Ensure the database is explicitly opened in `MeshtasticDatabaseTest` to trigger migrations. Signed-off-by: James Rich <[email protected]>
Signed-off-by: James Rich <[email protected]>
- Update `NodeMapViewModel` to use `myNodeInfo` for more direct local node identification. - Enhance `NodeManager` to update `lastHeard` timestamps for both position and telemetry packets. - Improve `handleReceivedPosition` logic to preserve existing coordinates when receiving packets with zeroed latitude/longitude. - Refactor `handleReceivedTelemetry` to support processing multiple metric types (device, environment, power) in a single update. - Update `core:common` and `core:database` build configurations to use `api` for shared core dependencies. - Remove explicit `WRITE_AHEAD_LOGGING` journal mode from `MeshtasticDatabase` configuration. - Add unit tests for device and environment metrics processing in `NodeManagerImplTest`. - Ensure a default zeroed position is provided in `MeshActionHandlerImpl` when requesting positions if no valid location is available. Signed-off-by: James Rich <[email protected]>
- Add test to verify that zeroed coordinates preserve the last known location while still updating satellite counts and timestamps. - Add test ensuring purely empty position packets for the local node are ignored. - Add test confirming that telemetry updates the `lastHeard` timestamp for a node. - Suppress `ComplexCondition` lint warning in `NodeManagerImpl.handleReceivedPosition`. Signed-off-by: James Rich <[email protected]>
Signed-off-by: James Rich <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
follows https://developer.android.com/kotlin/multiplatform/room to update Room to be able to be used Multiplatform
MeshtasticDatabasedefinition fromandroidMaintocommonMain.MeshtasticDatabaseConstructorusing the Room KMP pattern.DatabaseManagerto useBundledSQLiteDriverand shared database configuration.DatabaseConstantsand address normalization helpers tocommonMain, utilizingokiofor cross-platform SHA-1 hashing.DatabaseModuleto:core:dataand refactorLocalDataSourceclasses to access DAOs viaDatabaseManagerinstead of direct injection.