Repository navigation
P0 Studio operator UX: table-as-wire SSOT, visible injector title, stacked 4I+4V, centered controls - #239
Conversation
…tage/current vertically
…d current table; fit stacked matrices
… operator layout without touching firmware
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: c5a71702a1
ℹ️ 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".
| and 'property bool phaseLink: true' in main_qml, | ||
| "PTP SOURCE/AUTO and three-phase link start enabled, while OFF remains authoritative") | ||
| require('var draft = scenarioSnapshot()' in main_qml | ||
| and 'const accepted = smartSession.requestStartWithScenario(' in workflow |
There was a problem hiding this comment.
The new assertions reference workflow here and ribbon on line 72, but both variables are assigned later in main(). Consequently, every invocation—including .github/workflows/arstack-studio-qt.yml:175—terminates with UnboundLocalError at this line rather than running the structural gates. Move both source(...) assignments above these assertions.
Useful? React with 👍 / 👎.
| if (device.deviceVerified && root.ptpDesiredForSession && | ||
| !device.ptpRunning) { | ||
| root.ptpRestoreChecks = 0 | ||
| root.ptpRestoreAfterFirmware = true |
There was a problem hiding this comment.
After a fresh connection defaults ptpDesiredForSession to true, an operator can explicitly stop PTP through PtpExpertPanel.qml's header switch or Stop button, both of which call session.requestStopPtp() directly and never clear this property. On the next unplug/replug or firmware reconnect, this handler still sees true and automatically restarts the PTP source, so OFF is not authoritative for the session as promised. Advanced start/stop actions need to update the same controller-owned intent used by the main tray.
AGENTS.md reference: AGENTS.md:L46-L50
Useful? React with 👍 / 👎.
| Layout.fillWidth: true | ||
| Layout.fillHeight: true | ||
| Layout.minimumHeight: 175 | ||
| Layout.preferredHeight: root.compactLayout ? 180 : 205 | ||
| Layout.maximumHeight: 242 |
There was a problem hiding this comment.
At the declared supported window minimum of 620 px, the injection surface has only about 455 px of interior height after the 78 px workflow bar, collapsed telemetry split, handles, and margins. These two 175 px matrix minima plus their 10 px gap consume 360 px, while the title, 48 px frequency row, 57 px timing tray, quality row, and layout spacing require well over the remaining 95 px. Because this surface has no scrolling, resizing to the supported minimum causes the lower controls to overflow or be clipped; reduce the matrix minima, add scrolling, or raise the window minimum height.
Useful? React with 👍 / 👎.
…ead of obsolete ternary
… without clipping PTP tray
…ls at all Windows DPIs
…sible at min viewport
1 participant
Operator acceptance baseline (DO NOT REGRESS)
User's 120s canonical SV/PTP PCAP: 480,294 SV, 360 PTP, no counter gaps, 100%
smpSynch=1LOCAL AUTO, ~49µs host-relative epoch alignment. Preserve exact M3C.6 embedded PTP/SV firmware, servo, Pdelay/Announce settings, timing ISR, rate/counter, and verified Preview packaging; this PR changes only Studio UI and structural UI tests.Actual bug behind GUI vs on-wire angles
WorkflowBar.requestStart()skipped the complete 4I+4V batch wheneverpendingApply=falseand invoked barerequestStart(). Separately, enabling3-phase linkwhile RUNNING adjusted its checkbox/model without transmitting the newly linked phase angles. Fix both: START always snapshots all 8 current visible model rows through the bounded existing worker FIFO, and enabling three-phase link derives A/B/C values for current+voltage groups and submits live SET commands on RUNNING. Neither pathway sets the SV clock or PTP status. Default current phase -30/-150/+90 and 3-phase link ON. Persistent operator scenario still takes precedence on launch.Direct UI improvements
CalmButtonhover fills light with dark caption; Advanced tab light-hover title dark.StudioDarkCheckBoxsingle stable 20×20 indicator at all states and hover, no tiny second checkbox.Gate
Studio Qt contract/build-smoke + Windows Lifecycle + native Preview exact-head; no physical claims about actual relay acceptance beyond previous PCAP. Preview artifact requires current M3C.6 SOURCE servo/MAC evidence flags. Do not merge to public main until staged firmware is physical-board verified.