feat: move native to a separate process, again #964
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements a significant architectural refactoring to move native functionality into a separate process. The main application now communicates with the native process via gRPC over Unix sockets, improving isolation and reliability through automatic restart capabilities.
Key changes include:
- Introduction of gRPC-based IPC between main app and native process
- Implementation of a native proxy with automatic restart logic
- Addition of process monitoring and lifecycle management
- New scripts for proto generation, deployment, and debugging support
Reviewed Changes
Copilot reviewed 33 out of 35 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/release.sh | New release automation script for system updates |
| scripts/generate_proto.sh | Script to generate gRPC code from proto definitions |
| scripts/dev_deploy.sh | Enhanced deployment script with native binary debugging support |
| scripts/configure_vscode.py | VSCode C/C++ IntelliSense configuration helper |
| scripts/build_cgo.sh | Updated to support Debug/Release build types |
| internal/native/proto/*.proto | gRPC service definitions for native IPC |
| internal/native/proto/*.pb.go | Generated protobuf/gRPC code |
| internal/native/proxy.go | Proxy implementation managing native process lifecycle |
| internal/native/server.go | gRPC server running in native process |
| internal/native/interface.go | Common interface for both Native and NativeProxy |
| internal/native/native.go | Updated Native implementation to match interface |
| internal/utils/env.go | Environment variable marshaling utility |
| internal/utils/env_test.go | Tests for env marshaling |
| internal/supervisor/consts.go | Supervisor-related constants |
| internal/native/cgo/main.h | Header file for native binary entry point |
| internal/native/cgo/CMakeLists.txt | Updated build to produce both library and binary |
| native.go | Integration with new proxy architecture |
| main.go | Process title updates and initialization reordering |
| display.go | Display reconfiguration on native restart |
| go.mod | New dependencies for gRPC and env parsing |
| Makefile | Build type configuration support |
| DEVELOPMENT.md | Documentation for native debugging |
💡 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 39 out of 41 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| get_file_by_name "update_ota.tar" | ||
| get_file_by_name "update.img" | ||
|
|
||
| strings -d bin/jetkvm_app | grep -x '0.4.8' |
There was a problem hiding this comment.
Hardcoded version string '0.4.8' should be parameterized or replaced with a variable. This appears to be checking for a specific version in the binary, but the version is not passed as a parameter to the script.
| mkdir -p ${TEMP_DIR}/extracted-update | ||
| ${UNPACK_BIN} -i update.img -o ${TEMP_DIR}/extracted-update | ||
|
|
||
| exit 0 |
There was a problem hiding this comment.
Early exit at line 54 makes the code after it unreachable. All the release logic from lines 55-134 will never execute. This appears to be debug/development code that should be removed or moved.
|
Why are we splitting this stuff back out to a distinct process? (not that I object, just wondering about the motivation) |
|
cgo panics will crash the entire process. While we’ve added a failsafe mode to avoid disrupting the user’s device, in many cases a simple restart is enough to recover. By splitting this into a separate process, we can restart it independently and provide a better user experience, with failsafe mode only triggered after repeated failures. |
No description provided.