Repository navigation
Introduce PM3 core API and Desktop CLI backend; remove Android CI and upstream PM3 sync - #26
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4c4fab6eec
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (_config == null || state != Pm3ConnectionState.connected) { | ||
| throw StateError('PM3 is not connected'); |
There was a problem hiding this comment.
When no PM3 is connected, the terminal's always-enabled submit handler still calls AppState.sendCommand without awaiting or catching its future. This new guard therefore produces an unhandled asynchronous StateError whenever a user submits a terminal command while disconnected, whereas the previous process adapter emitted [未连接] and returned normally. Preserve that non-throwing behavior or guard the terminal submission path.
Useful? React with 👍 / 👎.
| } on ProcessException { | ||
| // The settings page lets the user select an explicit client path. | ||
| } | ||
| return Platform.isWindows ? 'proxmark3.exe' : 'proxmark3'; |
There was a problem hiding this comment.
When the client is available only as the common ./pm3 launcher in the working directory and proxmark3 is not installed system-wide, this new fallback initializes the connection page with proxmark3, so connecting fails until the user manually corrects the path. The previous default was ./pm3, and the README still advertises both pm3 and proxmark3; include the local launcher among the detected candidates before falling back.
Useful? React with 👍 / 👎.
| Future<Pm3Result> execute(Pm3Command command, {Duration? timeout}) => | ||
| _backend.execute(command, timeout: timeout); |
There was a problem hiding this comment.
When a caller invokes execute or send again before the previous future resolves—which the terminal permits because submissions are neither disabled nor awaited—the calls are forwarded concurrently to Pm3Process.sendCommandAndWait. That method clears and polls one shared _responseBuffer, so the later invocation erases the earlier command's captured output and both structured results can contain truncated or interleaved data. Queue executions or otherwise enforce one in-flight desktop command.
Useful? React with 👍 / 👎.
| final output = await _process.sendCommandAndWait( | ||
| command.executable, | ||
| timeout: timeout ?? const Duration(seconds: 10), | ||
| ); |
There was a problem hiding this comment.
For hardware commands that emit an initial line and then pause for more than 200 ms while doing work, sendCommandAndWait interprets one unchanged polling interval as completion, so this new execute implementation returns a Pm3Result before the command has finished and omits its later output despite the supplied timeout. This is common for reads, attacks, and scripts; use the PM3 prompt or a reliable command terminator rather than a single short silence interval.
Useful? React with 👍 / 👎.
| ConnectionState({Pm3Controller? controller}) | ||
| : controller = controller ?? Pm3Controller(DesktopCliBackend()) { | ||
| pm3Path = DesktopCliBackend.detectExecutable(); |
There was a problem hiding this comment.
Although this constructor makes ConnectionState injectable, AppState still unconditionally constructs it as ConnectionState(), and every widget and feature reaches PM3 through that hard-coded AppState. As a result, the newly added MockPm3Backend cannot be used for the advertised hardware-free feature or widget development without replacing application internals; expose this injection point through AppState as well.
Useful? React with 👍 / 👎.
Motivation
MockPm3Backendfor feature development and unit tests without hardware.sync-pm3workflow to reduce CI resource usage and avoid producing upstream binaries in this repo.Description
lib/core/pm3/withPm3Backend,Pm3Controller,Pm3Command,Pm3ConnectionConfig,Pm3Event, andPm3Resultto standardize backend interactions.DesktopCliBackend(lib/backend/desktop_cli/desktop_cli_backend.dart) backed by a refactoredPm3Processmoved tolib/backend/desktop_cli/pm3_process.dart, including process launching, state mapping and command/response handling.MockPm3Backend(lib/backend/mock/mock_pm3_backend.dart) for tests and development without hardware.pm3Outputstream instead of directPm3Processaccess, and change connection handling to usePm3ConnectionConfig.README.mdto reflect desktop-only focus and the new architecture, and update many UI pages to consume the new output stream abstraction..github/workflows/build.yml, remove Android release/upload fromrelease.yml, and delete the largesync-pm3.ymlworkflow that built upstream proxmark3 artifacts.test/backend/pm3_controller_test.dartto validate connection/execution behavior and error handling.Testing
flutter testwhich executed the newpm3_controller_test.dartsuite (2 tests) and they passed.Codex Task