Repository navigation
P0 Studio: protect SMV/PTP baseline, send live phase table, improve operator layout - #245
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: 763f3c9098
ℹ️ 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".
| ok = root.sendSignal(group, row) && ok | ||
| if (ok) root.pendingApply = false |
There was a problem hiding this comment.
When the device command queue is full or locked by an exclusive transaction, each sendSignal() still returns true because DeviceController::sendCommandBatch() reports only whether the queued Qt invocation was scheduled; the actual admission failure occurs later in DeviceIoWorker::enqueueCommandsInternal(). Consequently, toggling 3-phase link can reject some or all eight SETs while this line clears pendingApply, falsely presenting the visible table as applied. Send these SETs as one admission-checked batch or retain the pending state until worker acceptance is reported.
AGENTS.md reference: AGENTS.md:L116-L116
Useful? React with 👍 / 👎.
| require("Layout.preferredWidth: bar.compact ? 232 : 286" in ribbon | ||
| and "font.pixelSize: bar.compact ? 21 : 25" in ribbon, | ||
| "operator brand/logo must stay legible") |
There was a problem hiding this comment.
The Qt CI workflow runs this script, but the newly required layout and branding strings contradict assertions that remain later in the same function. Running python3 apps/arstack_studio/tests/check_operator_workspace.py on this commit already exits at the old Layout.minimumHeight: root.compactLayout ? 180 : 210 requirement; after updating that, lines 120–122 still require the old 220 : 270, 20 : 22, and width: 44 branding that this new assertion and ModernRibbon.qml explicitly replace. The gate therefore cannot pass until the obsolete expectations are removed or updated.
Useful? React with 👍 / 👎.
| Layout.fillHeight: false | ||
| Layout.preferredHeight: root.compactLayout ? 378 : 410 | ||
| Layout.maximumHeight: 420 | ||
| Layout.minimumHeight: root.compactLayout ? 340 : 370 |
There was a problem hiding this comment.
At the declared 960×620 minimum window, the footer, outer margins, 78-pixel workflow bar, and split handle leave roughly 490 pixels for the workspace. Inside the injection panel, its margins plus the header, 48-pixel frequency row, 57-pixel timing tray, quality row, separators, and spacing leave substantially less than the new 340-pixel compact table minimum, so the bottom timing/quality controls are clipped or forced through layout constraints. The previous 180-pixel minimum explicitly allowed this viewport to shrink; retain a shrinkable minimum and use preferred/maximum heights only to prevent stretching on tall windows.
Useful? React with 👍 / 👎.
| Timer { | ||
| interval: 350 | ||
| repeat: true | ||
| running: root.ptpDefaultPending |
There was a problem hiding this comment.
This timer starts PTP without setting the pre-existing ptpDefaultStartAttempted flag, while the original 500 ms timer remains armed on that flag. If this first guided start is accepted but subsequently fails asynchronously—such as a command rejection, ACK timeout, or readback mismatch—ptpStarting eventually becomes false and the original timer automatically launches a second attempt, violating the documented one-shot behavior and producing an implicit retry. Use the existing attempted state for the first request or replace the old timer rather than maintaining two independent default-start state machines.
AGENTS.md reference: AGENTS.md:L76-L83
Useful? React with 👍 / 👎.
…nd desktop viewports
0a3e916
into
feature/studio-matched-ptp-sv-auto-20261017
1 participant
Preserve baseline
No changes to ESP32-P4 SV/PTP engine,
smpCnt, AUTO phase servo or wire encoder. Existing 120s capture proves 480,294 canonical frames with no discontinuities, localsmpSynch=1, aligned SV/PTP epochs (host-relative). Do not regress M3C.5/M3C.6.Operator fixes
CalmButtonicon + label now use centered content root rather than invalid managed Item anchors, maintaining dark text on light hover.3-phase linklive toggle now sends every active model setpoint to device; do not assume changing a checkbox transmits phases. START still flushes pending 90ms editors, serializes ALL eight model phases as integer millidegree SET commands before START, preserving device path SSOT.Verification
Qt CI, Windows Lifecycle, native installer Preview with exact matching firmware. Distinguish GUI model from on-wire proof: physical readback per-channel is absent; no claim the displayed angle was transmitted until verified with independent PCAP. Final acceptance: preview install, all 8 angles and phase link RUNNING, PTP AUTO=1, loss/dup=0.