Skip to content

P0 Studio operator UX: table-as-wire SSOT, visible injector title, stacked 4I+4V, centered controls - #239

Closed
masarray wants to merge 17 commits into
feature/studio-matched-ptp-sv-auto-20261017from
fix/studio-p0-ssot-ui-20261018
Closed

masarray wants to merge 17 commits into
feature/studio-matched-ptp-sv-auto-20261017from
fix/studio-p0-ssot-ui-20261018

Conversation

@masarray

Copy link
Copy Markdown
Owner

Operator acceptance baseline (DO NOT REGRESS)

User's 120s canonical SV/PTP PCAP: 480,294 SV, 360 PTP, no counter gaps, 100% smpSynch=1 LOCAL 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 whenever pendingApply=false and invoked bare requestStart(). Separately, enabling 3-phase link while 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

  • Resize SMV Injector logo 28→38 px, glyph 15→24, label 11→15, reserve enough brand width.
  • Default PTP SOURCE ON after device verified, using existing bounded PTP restore/reconnect path; explicit OFF cancels, never persist ON between sessions. AUTO remains current sample sync mode; wave/phasor remain shown by default.
  • Replace voltage/current side-by-side matrices with stacked vertical matrices, controlled min/preferred/max heights, responsive at 1240×760.
  • Apply centered icon/caption to ribbon action content, and centered text in common buttons.
  • All Advanced shared CalmButton hover fills light with dark caption; Advanced tab light-hover title dark.
  • Shared StudioDarkCheckBox single stable 20×20 indicator at all states and hover, no tiny second checkbox.
  • Structural guards for layout, default controls, fidelity of START/live edits and colour states.

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.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 18, 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-18T00:33:49.247780Z c5a7170 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: 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

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 Load QML sources before using them

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 👍 / 👎.

Comment on lines +203 to +206
if (device.deviceVerified && root.ptpDesiredForSession &&
!device.ptpRunning) {
root.ptpRestoreChecks = 0
root.ptpRestoreAfterFirmware = 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 Route Advanced PTP controls through the intent owner

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 👍 / 👎.

Comment thread apps/arstack_studio/qml/Main.qml Outdated
Comment on lines +1094 to +1098
Layout.fillWidth: true
Layout.fillHeight: true
Layout.minimumHeight: 175
Layout.preferredHeight: root.compactLayout ? 180 : 205
Layout.maximumHeight: 242

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 stacked matrices within the supported minimum height

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 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Preservation triage 2026-10-23 — KEEP OPEN, not swept into redundant GUI closure. Five alternate UX PRs #236/#237/#238/#240/#241 were closed as superseded by canonical Draft #286; this PR has a different feature-level contract: enabling 3-phase link while SV is RUNNING should derive the current/voltage A/B/C phase values and propagate one complete verified live transaction. The current #286 Main.qml StudioLinkSwitch.onToggled only changes root.phaseLink as an editor preference; actual ABC propagation occurs on later editSignal(...) and the new auto-live transaction path. That is not parity with this PR's toggle semantics. Do not cherry-pick stale QML directly: another active thread owns ENTER/focus-out auto-live code at 042df0d40ddd3f061ebeebc81c662c585827e346. Decide desired toggle behavior with an isolated regression and preserve full 4I4V atomic commit/no START reset. Physical wire proof still needed. Branch/source retained; no main merge.

Copy link
Copy Markdown
Owner Author

M3.H release=main reconciliation — 2026-10-24

Verified product baseline is a922e9bdba4e57a4e88406eb83344455dd8ff142 (ESP32-P4 firmware Build ID a922e9bdba4e57a4); consolidated PR #299 is MERGED to main with 14/14 integration checks passing. The official M3.H Stable tag and tested source are ancestors of main.

This PR has a distinct historical/divergent change scope and is not part of the immutable shipped M3.H baseline. It remains DRAFT / NOT READY for merge; do not merge against an obsolete stacked base or count its tests as final M3.H acceptance. Reconcile only still-needed unique behavior into a fresh branch from current main, with implementation-level comparison, all affected CI tests, and targeted ESP32-P4 FAT where relevant. If superseded, close with an explicit feature-by-feature disposition instead of importing old firmware wholesale.

Canonical closure record: https://github.com/masarray/arstack61850/blob/main/docs/integration/M3H_MAINLINE_RELEASE_SOURCE_CLOSURE_2026-10-24.md .

Copy link
Copy Markdown
Owner Author

Archived as superseded post-M3.H implementation (2026-10-24)

NOT MERGED, NOT accepted as complete. Historical branch head c35c73fe59ee45cc0d2149152c5969fb0b4a5807 is preserved. The actual M3.H source a922e9bdba4e57a4e88406eb83344455dd8ff142 is already merged into main via PR #299, with 14/14 integration CI.

Older 4I+4V UI and wire-truthful scenario is an alternative to merged M3.H; only outstanding requirements belong in new exact-head changes.

Outstanding feature acceptance lives in #300 and #42; old stacked branch cannot be merged safely. Re-implement missing behavior from current main, supported by focused regressions and hardware evidence, and release under a new tag. Closure ledger.

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