app-module and TactilitySDK improvements - #665
Conversation
P4 devices are reaching the 4MB app partition limit. This will give them more space.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe SDK release process now packages target-specific application configuration and version metadata. The build tool uses metadata and configuration associated with the selected SDK version. The app module adds readiness polling and platform-specific POSIX I/O wrappers, with integration tests for stdin metadata, polling, and app exit. The shell can run eligible installed headless app binaries as commands. The terminal emulator now handles CSI J erase modes 0, 1, and 2. Priority: ➖ Normal Merge Risk: 🟡 Moderate · up to This change adds a way for an app to call exit() and end only itself, plus new shell and SDK tooling behavior. Several problems should be fixed before merging. An app's exit status may not reach its parent correctly. Exiting while C++ objects are still alive is undefined behavior. POSIX SDK builds can fail because a configuration file they do not need is required. Calling fstat with a null buffer on an app descriptor can crash. Two installed apps with the same binary name produce the same shell command, so one of them cannot be run by name. Security Architecture ReviewSecurity architecture risk: 🟡 Moderate · up to Installed apps gain a new command-name launch path, and app termination now uses a jump that may not preserve C++ cleanup or reliably report exit status. Built-in shell commands retain priority, and normal app exit still reaches scheduler cleanup, but these changes merit design review. Retained concerns
Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 3 | ❌ 1 | ❓ 1❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 10.96% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 146 functions across 30 files. (1 skipped: 1 unsupported.) ✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: 0fae3099-a03e-4e80-8deb-d7c65a4d660e
⛔ Files ignored due to path filters (5)
partitions-16mb-no-sd-dev.csvis excluded by!**/*.csvpartitions-16mb-no-sd.csvis excluded by!**/*.csvpartitions-16mb-with-sd.csvis excluded by!**/*.csvpartitions-32mb-no-sd-dev.csvis excluded by!**/*.csvpartitions-32mb-no-sd.csvis excluded by!**/*.csv
📒 Files selected for processing (31)
.github/actions/build-sdk-posix/action.yml.github/actions/build-sdk/action.ymlBuildscripts/CDN/upload-sdk-files.pyBuildscripts/TactilitySDK/sdkconfig.app.esp32Buildscripts/TactilitySDK/sdkconfig.app.esp32c6Buildscripts/TactilitySDK/sdkconfig.app.esp32p4Buildscripts/TactilitySDK/sdkconfig.app.esp32s3Buildscripts/release-sdk-esp32.pyCMakeLists.txtDocumentation/ideas.mdModules/app-module/CMakeLists.txtModules/app-module/include/app/io.hModules/app-module/private/app/private/stdio_wrap.hModules/app-module/private/app/private/stdio_wrap_posix.hModules/app-module/source/io.cppModules/app-module/source/module.cppModules/app-module/source/stdio_wrap.cppModules/app-module/source/stdio_wrap_apple.cppModules/app-module/source/stdio_wrap_elf.cppModules/app-module/source/stdio_wrap_esp32.cppModules/app-module/source/stdio_wrap_posix.cppModules/app-module/tests/CMakeLists.txtModules/app-module/tests/source/io_test.cppModules/app-posix-module/tests/CMakeLists.txtModules/c-symbols-module/source/module.cppModules/posix-symbols-module/source/module.cppTactility/Source/app/shell/LineEditor.cppTactility/Source/app/terminal/vterm/vterm.cdevice.pytactility.pytactility.py.json
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: 1110ab61-7604-471d-84cd-256817cfc9e0
📒 Files selected for processing (12)
Buildscripts/CDN/upload-sdk-files.pyCMakeLists.txtModules/c-symbols-module/source/module.cppModules/posix-symbols-module/source/module.cppPlatforms/platform-esp32/source/mkdir.cppPlatforms/platform-esp32/source/root_dir.cppPlatforms/platform-esp32/source/unistd.cppPlatforms/platform-esp32/source/vfs_null_path.cppTactility/Private/Tactility/app/shell/Shell.hTactility/Source/app/shell/Shell.cppTactility/Tests/Source/ShellCompletionTest.cpptactility.py
🚧 Files skipped from review as they are similar to previous changes (1)
- Buildscripts/CDN/upload-sdk-files.py
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: 222b3521-094f-4697-8977-d92443454316
📒 Files selected for processing (13)
Modules/app-module/include/app/io.hModules/app-module/private/app/private/stdio_wrap.hModules/app-module/source/io.cppModules/app-module/source/stdio_wrap.cppModules/app-module/source/stdio_wrap_esp32.cppModules/app-module/source/stdio_wrap_posix.cppModules/app-module/tests/source/io_test.cppModules/c-symbols-module/source/module.cppTactility/Private/Tactility/app/shell/Run.hTactility/Private/Tactility/app/shell/Shell.hTactility/Source/app/shell/Run.cppTactility/Source/app/shell/Shell.cpptactility.py
🚧 Files skipped from review as they are similar to previous changes (3)
- Modules/c-symbols-module/source/module.cpp
- Tactility/Private/Tactility/app/shell/Shell.h
- Modules/app-module/include/app/io.h
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: 865d3473-79d9-4727-9301-102522d3a84f
📒 Files selected for processing (9)
CMakeLists.txtModules/app-module/include/app/scheduler.hModules/app-module/private/app/private/stdio_wrap_posix.hModules/app-module/source/scheduler.cppModules/app-module/source/stdio_wrap_apple.cppModules/app-module/source/stdio_wrap_elf.cppModules/app-module/source/stdio_wrap_esp32.cppModules/app-module/source/stdio_wrap_posix.cppModules/app-module/tests/source/io_test.cpp
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 1 remain after this review.
New features:
Fixes: