Current Tint display change to "last known good state" - #13
Conversation
There was a problem hiding this comment.
Pull request overview
This PR migrates panel state tracking from JSON-only storage to a dual SQLite/JSON system as part of addressing unreliable live tint endpoint data. The migration maintains backward compatibility by continuing to write to both storage systems during the transition period, establishing a "last known good state" pattern based on successful tinting API responses.
Key changes:
- Introduces SQLite database table
panel_statefor tracking panel level and timestamp - Updates state persistence to write to both SQLite and JSON for backward compatibility
- Adds
update_panel_state()function called after successful tint operations to maintain accurate state
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| svc/data/audit.db | Binary SQLite database file (should not be committed - see comment) |
| svc/app/state.py | Migrates panel state storage to SQLite while maintaining JSON compatibility; adds new functions for DB initialization, migration, and single-panel updates |
| svc/app/service.py | Integrates update_panel_state() calls after successful panel and group tint operations to persist state based on API response success |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| INSERT OR REPLACE INTO panel_state (panel_id, level, last_change_ts) | ||
| VALUES (?, ?, ?) | ||
| """, | ||
| ( |
There was a problem hiding this comment.
how can we confirm these updates are set in the audit.db?
There was a problem hiding this comment.
If it fails at any point while trying to execute the insert, the commit after the for loop will fail, causing a rollback. So if there is no failure, it is highly assured that the operation succeeded, especially for what we have here which should always be predictable, sterile data. However, I do agree we should probably consider using an ORM instead of raw SQL, as it is easier to use and handles things such as this.
There was a problem hiding this comment.
I do now see an issue with the migration logic though, so that will have to be fixed before merge
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 9 changed files in this pull request and generated 10 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ean time we can use a 'last known good state' approach to have the current window tint numbers as accurate as possible. This can change later if we conclude the live-tint endpoint is reliable. Also, migrated panel state to SQLlite, but kept json functionality too.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
- Remove audit.json file writing - SQLite is now the only source of truth - Implement _db_connection() context manager for cleaner database operations - Refactor all database operations to use context manager (~70% code reduction) - Remove audit.db from git tracking - Update documentation to reflect SQLite-only audit logging
b065198 to
aa671d6
Compare
…ill have verbose output to aid in debugging
…ntain panel configs and window mapping. All writing operations are directed to DB. Now DB has tables for panel_state, audit log, and groups. This will give us perfomance gain and better concurrency acccess compared to writing to JSON. This also helps in restructuring the structure to keep run time and config as seperated as possible for better installation reliability
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 18 changed files in this pull request and generated 7 comments.
Comments suppressed due to low confidence (1)
svc/data/panels_config.json:22
- The config file references groups "G-1" and "G-2" (lines 6, 11, 16, 21) which don't appear to be defined anywhere and are inconsistent with the default groups created in
bootstrap_default_if_empty()("G-facade" and "G-skylights"). This could lead to panels not being associated with valid groups, causing issues when trying to control them via group commands.
"P01": {
"id": "P01",
"name": "Facade 1",
"group_id": "G-1"
},
"P02": {
"id": "P02",
"name": "Facade 2",
"group_id": "G-1"
},
"P03": {
"id": "P03",
"name": "Facade 3",
"group_id": "G-2"
},
"P04": {
"id": "P04",
"name": "Facade 4",
"group_id": "G-2"
},
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 19 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
- Move groups and panel state to SQLite (panels_config.json now read-only) - Remove group_id from Panel model (supports multiple groups per panel) - Move migrations to app startup for better performance - Fix bootstrap to preserve dwell time on restarts - Extract shared utilities for panel timestamp management - Improve exception handling in migration functions All 41 tests pass.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 20 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Load JSON state | ||
| with open(PANELS_STATE_FILE, "r", encoding="utf-8") as f: | ||
| state_data = json.load(f) |
There was a problem hiding this comment.
The migration functions use manual transaction handling but don't wrap the file I/O operations within the transaction. If the JSON file read fails after the count check succeeds, the database connection may be left in an inconsistent state. Consider moving the JSON file read before opening the database connection, or handle file I/O errors before starting the transaction.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Summary
Phase out JSON logging and panel state. Convert cleanly to SQLite implementation. Uses a context manager for cleaner DB operations. Remove audit.db from tracking. Move groups from panel_config.json to a table in the DB. Restructured so all remaining JSON files (config and window mapping) are read-only and all write operations occur in the DB. This completely separates run time from config files.
Type
Testing
Behaves no differently, as it is just a data storage type migration.
Risk and rollout
Medium Risk,k as we have converted to a new data storage system. Will monitor to ensure data reliability.
Checklist