fix(db): Revert removal of retry_count#4527
Conversation
This reverts commit 207c485, re-adding the `retry_count` column to the `packet` and `reactions` tables. A new database migration from version 34 to 35 has been created to undo the previous deletion of these columns. Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Reverts a prior schema change by attempting to re-introduce the retry_count column to the Room database, using a new DB version bump and auto-migration.
Changes:
- Bumps
MeshtasticDatabaseversion from 34 → 35 and adds an auto-migration entry for 34 → 35. - Adds a new
AutoMigration34to35spec. - Modifies the exported Room schema JSON for version 34 to include
retry_countinpacketandreactions.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
core/database/src/main/kotlin/org/meshtastic/core/database/MeshtasticDatabase.kt |
Adds DB version 35 and a 34→35 auto-migration spec intended to restore retry_count. |
core/database/schemas/org.meshtastic.core.database.MeshtasticDatabase/34.json |
Updates exported schema content/identity hash and adds retry_count to table definitions (but for v34). |
Comments suppressed due to low confidence (1)
core/database/schemas/org.meshtastic.core.database.MeshtasticDatabase/34.json:306
- The
createSqlforpacket/reactionsin the version-34 schema now includesretry_count, but the current Room entities incore/database/.../entity/Packet.ktdo not define aretry_countcolumn. This mismatch will cause Room schema validation/auto-migration generation to fail (exported schema must match the entities for that version). Update the entities to includeretry_count(with a default) and ensure the exported schema version that contains it is the current one (v35), not v34.
{
"tableName": "packet",
"createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`uuid` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, `myNodeNum` INTEGER NOT NULL DEFAULT 0, `port_num` INTEGER NOT NULL, `contact_key` TEXT NOT NULL, `received_time` INTEGER NOT NULL, `read` INTEGER NOT NULL DEFAULT 1, `data` TEXT NOT NULL, `packet_id` INTEGER NOT NULL DEFAULT 0, `routing_error` INTEGER NOT NULL DEFAULT -1, `retry_count` INTEGER NOT NULL DEFAULT 0, `snr` REAL NOT NULL DEFAULT 0, `rssi` INTEGER NOT NULL DEFAULT 0, `hopsAway` INTEGER NOT NULL DEFAULT -1, `sfpp_hash` BLOB, `filtered` INTEGER NOT NULL DEFAULT 0)",
"fields": [
{
"fieldPath": "uuid",
"columnName": "uuid",
| @DeleteColumn(tableName = "packet", columnName = "retry_count") | ||
| @DeleteColumn(tableName = "reactions", columnName = "retry_count") |
There was a problem hiding this comment.
AutoMigration34to35 is annotated with @DeleteColumn for retry_count, which does the opposite of the PR goal (re-adding the column). This will also likely make Room’s auto-migration generation/validation fail because the 34→35 schema diff should be an add column change, not a delete. Remove the @DeleteColumn spec for 34→35 (or remove the spec entirely if Room can auto-add the columns), and ensure the entities define retry_count with a default so Room can add a NOT NULL column safely.
| @DeleteColumn(tableName = "packet", columnName = "retry_count") | |
| @DeleteColumn(tableName = "reactions", columnName = "retry_count") |
| AutoMigration(from = 31, to = 32), | ||
| AutoMigration(from = 32, to = 33), | ||
| AutoMigration(from = 33, to = 34, spec = AutoMigration33to34::class), | ||
| AutoMigration(from = 34, to = 35, spec = AutoMigration34to35::class), | ||
| ], | ||
| version = 34, | ||
| version = 35, | ||
| exportSchema = true, | ||
| ) |
There was a problem hiding this comment.
This change bumps the DB version to 35, but the only migration-specific logic added is a second delete-column spec. If the intention is to restore retry_count, the schema/entities need to reintroduce the column (with a default) so Room can generate an ADD COLUMN migration; otherwise version 35 will still match the current entities (no retry_count) and the migration won’t actually restore anything.
| "database": { | ||
| "version": 34, | ||
| "identityHash": "25bf8e7feb6d0e7f9eab4dfccf546e45", | ||
| "identityHash": "34352663e54f76b7b9c13de31d9ac8e7", | ||
| "entities": [ |
There was a problem hiding this comment.
34.json should describe the schema for database version 34, but this diff changes it to include the retry_count columns and updates the identityHash. That makes the historical schema inconsistent with the declared migration path (33→34 deletes retry_count, and the PR description says 34→35 re-adds it). Revert the edits to 34.json and instead update/add the exported schema for version 35 to reflect the restored columns.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4527 +/- ##
==========================================
+ Coverage 14.42% 14.46% +0.04%
==========================================
Files 424 424
Lines 14521 14522 +1
Branches 2410 2410
==========================================
+ Hits 2094 2101 +7
+ Misses 12124 12117 -7
- Partials 303 304 +1 ☔ View full report in Codecov by Sentry. |
This reverts commit 207c485, re-adding the
retry_countcolumn to thepacketandreactionstables.A new database migration from version 34 to 35 has been created to undo the previous deletion of these columns.