Update properties parsing for app manifest and device manifest files#544
Conversation
Parse as properties instead of ini file
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThis PR switches device and manifest property handling from INI-style sections to flat dotted key=value entries. Build scripts, CMake logic, and Python tooling are updated to read the new format, and all device.properties files are rewritten accordingly. The app manifest parser now accepts a file path, auto-detects V1 or V2 manifest format, and delegates to dedicated parsing implementations. Callers and tests were updated to use the new manifest flow. Sequence Diagram(s)Included above in the manifest parsing layer. Suggested labels: build, refactor, device-properties, manifest-parsing 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
Tactility/Source/app/AppManifestParsing.cpp (1)
59-81: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueAvoid the extra manifest pass
detectIsV1Format()reads to EOF just to capture the first line, thenparseManifest()opens the same file again. Fold the format check into the properties parse so the manifest is only read once.device.py (1)
42-53: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winPython parser doesn't strip surrounding quotes like the CMake parser does.
READ_PROPERTIES_TO_MAPinBuildscripts/properties.cmakestrips a leading/trailing quote pair from values, but thisread_properties_file(and its duplicate inBuildscripts/CDN/generate-firmware-files.py) does not. If a device.properties value is ever quoted, CMake-based and Python-based consumers would silently disagree on the resulting value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 04d08dbd-b27c-4eda-a4f1-75c3391dd8a5
📒 Files selected for processing (64)
Buildscripts/CDN/generate-firmware-files.pyBuildscripts/properties.cmakeDevices/btt-panda-touch/device.propertiesDevices/cyd-2432s024c/device.propertiesDevices/cyd-2432s024r/device.propertiesDevices/cyd-2432s028r/device.propertiesDevices/cyd-2432s028rv3/device.propertiesDevices/cyd-2432s032c/device.propertiesDevices/cyd-3248s035c/device.propertiesDevices/cyd-4848s040c/device.propertiesDevices/cyd-8048s043c/device.propertiesDevices/cyd-e32r28t/device.propertiesDevices/cyd-e32r32p/device.propertiesDevices/elecrow-crowpanel-advance-28/device.propertiesDevices/elecrow-crowpanel-advance-35/device.propertiesDevices/elecrow-crowpanel-advance-50/device.propertiesDevices/elecrow-crowpanel-basic-28/device.propertiesDevices/elecrow-crowpanel-basic-35/device.propertiesDevices/elecrow-crowpanel-basic-50/device.propertiesDevices/generic-esp32/device.propertiesDevices/generic-esp32c6/device.propertiesDevices/generic-esp32p4/device.propertiesDevices/generic-esp32s3/device.propertiesDevices/guition-jc1060p470ciwy/device.propertiesDevices/guition-jc2432w328c/device.propertiesDevices/guition-jc3248w535c/device.propertiesDevices/guition-jc8048w550c/device.propertiesDevices/heltec-wifi-lora-32-v3/device.propertiesDevices/lilygo-tdeck/device.propertiesDevices/lilygo-tdisplay-s3/device.propertiesDevices/lilygo-tdisplay/device.propertiesDevices/lilygo-tdongle-s3/device.propertiesDevices/lilygo-thmi/device.propertiesDevices/lilygo-tlora-pager/device.propertiesDevices/m5stack-cardputer-adv/device.propertiesDevices/m5stack-cardputer/device.propertiesDevices/m5stack-core2/device.propertiesDevices/m5stack-cores3/device.propertiesDevices/m5stack-papers3/device.propertiesDevices/m5stack-stackchan/device.propertiesDevices/m5stack-stickc-plus/device.propertiesDevices/m5stack-stickc-plus2/device.propertiesDevices/m5stack-sticks3/device.propertiesDevices/m5stack-tab5/device.propertiesDevices/simulator/device.propertiesDevices/unphone/device.propertiesDevices/waveshare-esp32-s3-geek/device.propertiesDevices/waveshare-s3-lcd-13/device.propertiesDevices/waveshare-s3-touch-lcd-128/device.propertiesDevices/waveshare-s3-touch-lcd-147/device.propertiesDevices/waveshare-s3-touch-lcd-43/device.propertiesDevices/wireless-tag-wt32-sc01-plus/device.propertiesFirmware/CMakeLists.txtModules/lvgl-module/CMakeLists.txtTactility/Private/Tactility/app/AppManifestParsing.hTactility/Private/Tactility/app/AppManifestParsingInternal.hTactility/Source/Tactility.cppTactility/Source/app/AppInstall.cppTactility/Source/app/AppManifestParsing.cppTactility/Source/app/AppManifestParsingV1.cppTactility/Source/app/AppManifestParsingV2.cppTests/SdkIntegration/tactility.pyTests/Tactility/Source/AppManifestParsingTest.cppdevice.py
We used to parse the properties files as if they were ini files. That's corrected now.
Device properties are all updated.
App manifest supports old and new format for now.
Summary by CodeRabbit
New Features
key=valueformat with dotted namespaces.Bug Fixes
Tests