Skip to content

P0 Studio UX: exact 4I+4V START, vertical matrices, default PTP and centered controls - #236

Closed
masarray wants to merge 21 commits into
feature/studio-matched-ptp-sv-auto-20261017from
fix/studio-p0-waveform-layout-controls-20261018
Closed

masarray wants to merge 21 commits into
feature/studio-matched-ptp-sv-auto-20261017from
fix/studio-p0-waveform-layout-controls-20261018

Conversation

@masarray

Copy link
Copy Markdown
Owner

Capture-proven baseline is frozen

Do NOT modify M3C.6 PTP, SV codec, servo, local AUTO or capture policy. See docs/SMV_PTP_OPERATOR_BASELINE_NO_REGRESSION.md: canonical 480,294 frames in 120s, 0 smpCnt gaps, local AUTO=1, PTP 120 Sync/FUP/Announce, ~49us host-relative epoch indication (not hardware calibration).

Functional P0 defect: GUI -30° vs on-wire 0°

WorkflowBar.requestStart() chose bare START whenever controller.pendingApply was false, so after STOP the ESP32-P4 could retain earlier 0/-120/+120 setpoints even while the GUI showed -30/-150/+90. Fixed: every START submits current visible eight-channel snapshot (FREQ, 8x SET, 8x ENABLE, CT shape, START) through the pre-existing exclusive all-or-none serial queue. Any failed live SET now marks draft pending and presents error. Added native C++ deterministic encoder regression that requires exact SET IA 1000 -30000 0, SET IB 1000 -150000 0, SET IC 1000 90000 0 and Python structural guard forbidding the bare START shortcut.

Operator desktop UX

  • Stack Voltage over Current, distribute the editor height between the two matrices, eliminate empty bottom filler.
  • Enlarge ribbon SMV Injector emblem from 28 to 40px, image 15 to 28px, caption 11 to 15px, wider branding area. Ribbon buttons center icon/caption vertically and horizontally.
  • Verified ESP32-P4 defaults to one PTP SOURCE auto-start. smpSynch=AUTO retained. Explicit PTP OFF suppresses auto-rearm within the current Studio process; Advanced and Main share the same intent. Never issue offline commands. Phasor/Wave remain visible by default.
  • Center Advanced button captions and tabs; hover white/light with dark text for shared CalmButton, combo and tabs. Dark checkbox hover preserves one 18x18 icon, no duplicate native Windows glyph.
  • Add release/readback acceptance documentation and executable UI contracts. No new operator dialog.

Acceptance required before merge

Studio Qt / operator contract / Windows Lifecycle / Native Preview exact-head installer and core firmware CI. Test Windows 100%,150%,175% DPI screenshots and on-wire phasors; CI cannot substitute hardware confirmation.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 17, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-17T22:36:33.085867Z 5d7e3da PR opened
ℹ️ 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" or "@codex security review".

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 chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5d7e3dace9

ℹ️ 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".

Comment on lines +501 to +504
const sent = phaseLink && row < 3 ? sendLinkedGroup(group) : sendSignal(group, row)
if (!sent) {
pendingApply = true
showMessage("Setpoint not queued. STOP and START to apply all displayed 4I+4V values.", true)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Propagate asynchronous queue rejection to the draft

When the worker queue is full during a live edit, requestSetSignal() still returns true because DeviceController::sendCommandBatch() only confirms that the queued invokeMethod was scheduled; the actual rejection occurs later in DeviceIoWorker::enqueueCommandsInternal(). Since WorkflowBar.onLastErrorChanged() ignores errors unless startQueued is true, this new sent check never marks the draft pending for the queue-exhaustion case it is intended to handle, leaving the GUI and board setpoints inconsistent. Handle the asynchronous commandRejected result for live edits rather than treating dispatcher acceptance as queue admission.

AGENTS.md reference: AGENTS.md:L116-L116

Useful? React with 👍 / 👎.

Comment on lines +288 to +289
const accepted = smartSession.requestStartWithScenario(
controller.startScenarioSnapshot())

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Commit pending editor values before taking the START snapshot

If an operator edits a magnitude or angle and immediately presses F5, SignalMatrix has only stored the visible value in pendingMagnitude/pendingPhase; its 90 ms timer has not yet copied it into the ListModel. This snapshot therefore contains the old value, and the delayed timer runs after START was queued, so the supposedly atomic START can use a different value from the one displayed. Flush or synchronously commit active debounced editors before calling startScenarioSnapshot().

Useful? React with 👍 / 👎.

Comment on lines +238 to +240
root.ptpDefaultStartPending = false
if (!root.startPtpRole("SOURCE"))
root.showMessage("PTP auto-start was not accepted. Use PTP On to retry.", true)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve an explicitly selected PTP role across reconnects

After an operator starts RECEIVER or MONITOR, a serial disconnect/reconnect re-arms ptpDefaultStartPending because ptpExplicitOff is false, but this handler always starts SOURCE instead of using the existing ptpDesiredForSession/ptpDesiredRole intent. The reconnect consequently turns a receiving or monitoring station into a PTP publisher without another operator action; only use SOURCE when no explicit role has been selected, otherwise restore ptpDesiredRole.

Useful? React with 👍 / 👎.

Comment on lines +42 to +43
contentItem: Item {
Row {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve CalmButton's content-based implicit width

The replacement Item has an implicit width of zero and does not inherit the inner Row's implicit size, while the button still computes its width from contentItem.implicitWidth. Consequently every CalmButton without an explicit width now resolves to the 64 px minimum, causing captions such as “Start PTP Receiver” and “Copy diagnostics” to overflow or overlap adjacent controls. Give the content item the Row's implicit dimensions or retain a content item that propagates child sizing.

Useful? React with 👍 / 👎.

Layout.maximumHeight: root.compactLayout ? 214 : 224
spacing: 9
Layout.fillHeight: true
Layout.minimumHeight: 408

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep the stacked matrices within the supported minimum height

At the declared 620 px window minimum, the footer, outer margins, workflow bar, and panel margins leave about 474 px inside the injection surface. This 408 px matrix minimum plus the 57 px timing tray and spacing already consumes essentially all of that space before the title, 48 px frequency row, separator, and quality row are laid out, so the editor must overflow or clip at a supported window size. Either raise the window minimum, make this section scrollable, or allow the matrices to shrink further.

Useful? React with 👍 / 👎.

Comment on lines +193 to +195
require("property bool phasorDockVisible: true" in main_qml
and "property bool waveformDockVisible: true" in main_qml
and "tray.session.requestSmpSynch(" in tray,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Test the persisted dock defaults that actually win at startup

This new assertion does not establish that the docks are visible by default: on a fresh settings profile, WorkflowBar.operatorSettings.phasorVisible and waveformVisible both default to false, and Component.onCompleted immediately assigns those false values over the true Main properties checked here. Thus the test passes while both docks start hidden; update the Settings defaults or initialization behavior and test the effective startup state.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Release cleanup · 2026-10-23 — CLOSED AS SUPERSEDED UI DESIGN (NOT MERGED).

Original branch head: d02bb037790942dbe2215ae7ca10eb0c9f8ee6aa (retained; no branch deletion). This source branch is diverged, not an ancestor of #286. Closure is an explicit feature-level supersession, NOT a claim that its git diff was merged or that all cosmetic patch lines are byte-equivalent.

At canonical #286 head 042df0d40ddd3f061ebeebc81c662c585827e346: WorkflowBar.qml::requestStart() always takes controller.startScenarioSnapshot() and calls smartSession.requestStartWithScenario(scenario), with no bare START path; Main.qml has vertically stacked 4I+4V matrices, one-shot verified-board PTP SOURCE default and explicit OFF cancellation. apps/arstack_studio/tests/check_operator_workspace.py guards snapshot/start, vertical layout and PTP intent. Newer verified transaction path requires firmware commit/readback rather than treating an old serial FIFO as acceptance.

The multiple competing 2026-10-17 UI implementations cannot be merged into current Studio without overwriting the latest operator layout and the other active worker's automatic Live-edit/Enter/focus-out fix. Preserve the unique historical design and regression rationale here; #286 owns the current premium UI and exact-build release gate. Desktop 100/150/175% DPI visual acceptance and physical GUI→SV angles/phase verification remain pending, not marked PASS. In particular #239 stays OPEN because its RUNNING three-phase toggle changes actual channel values contract differs from the current preference-only switch and must be independently dispositioned.

No changes to main, no merge, no deleted branch, no Studio QML edits in this cleanup.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant