Repository navigation
feat(studio): premium SMV injection workspace with live main PTP and voltage-first views - #202
Conversation
…-first plots and shared menus
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 89726b3b54
ℹ️ 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".
| const wantsRunning = checked | ||
| checked = Qt.binding(function() { return Boolean(tray.device && tray.device.ptpRunning) }) | ||
| if (wantsRunning) { | ||
| if (tray.controller) tray.controller.startPtpRole(tray.roleValues[tray.selectedRoleIndex]) |
There was a problem hiding this comment.
Add the missing Main PTP delegation
controller is bound to the ApplicationWindow root, but Main.qml defines no startPtpRole() method; the only implementation is on ConfigurationHub. Turning this switch on therefore raises a QML TypeError instead of starting PTP. The newly added workflow also fails unconditionally because check_operator_workspace.py explicitly expects the missing return profilePanel.startPtpRole(role) delegation in Main.qml.
Useful? React with 👍 / 👎.
| property string uiFont: "Inter" | ||
| // Qt 6.8+ can prefer native Windows menus, which ignore custom QML | ||
| // delegates and render in OS colors. Force our portable styled overlay. | ||
| popupType: Popup.Item |
There was a problem hiding this comment.
Preserve the declared Qt 6.5–6.7 support
Popup.popupType was introduced in Qt 6.8, according to the Qt Popup documentation, while this application still declares and documents Qt 6.5 as its minimum. With Qt 6.5–6.7, this component fails QML compilation/loading because popupType is not a recognized property, preventing the Studio UI from starting; either avoid the property for those versions or raise the required Qt version consistently.
Useful? React with 👍 / 👎.
| text: "Details" | ||
| implicitWidth: 66 | ||
| toolTipText: "PTP settings, timing evidence and diagnostics" | ||
| onClicked: if (tray.controller) tray.controller.openTimingSettings() |
There was a problem hiding this comment.
Wire the Details button to an implemented controller method
The tray receives root as its controller, but neither Main.qml nor any other component defines openTimingSettings(). Clicking Details therefore raises a QML TypeError and leaves the operator on the main screen instead of opening the Time Sync page; the root should open the configuration window and delegate to the existing profilePanel.showTiming() path.
Useful? React with 👍 / 👎.
Paket 2 validation handoff (2026-10-17)Built on top of Package 1: Windows Native Preview for New Acceptance gate: Qt build/smoke (including no QML TypeError), Windows Lifecycle + new geometry/screenshot step, Native Preview packaging, RC2 policy, and screenshots user-reviewed for visual polish. Do not merge just because prior preview was green; this new head must pass. Human inspection still needed for hover/focus/Escape/menu clipping/contrast at scaling, plus bench PTP/SV; automated geometry cannot claim visual UX acceptance. If geometry gate fails, fix the actual placement/density root cause, not widen tolerances to hide clipping. Preserve exact firmware build identity and physical v0.1.1 baseline. |
Paket 2 viewport QA result — exact
|
Paket 2 · operator UX hardening (exact head
|
…compact action menus
Paket 3 · offline-first injection lab and Material UI (exact HEAD
|
Final engineering audit — pending exact-head CILast source head:
Do not rerun green jobs or edit code for cosmetic changes during gate execution. |
Merged integration resultSquash merge: Final verified PR head:
Both checksums independently matched the Operational caution: This is an ENGINEERING PREVIEW and not a public/stable production release. Real board readback of all 8 channels, continuous PTP↔SMV sample-phase lock, atomic ABC update and independent on-wire quality/sequence/timing acceptance remain separate engineering tasks. Preserve the working original v0.1.1 flasher/installer baseline, and do not confuse preview state with physical SV output. Post-merge |
Operator problem / screenshot root cause
The October 16 Windows preview showed no Main-window PTP controls, a largely unused lower-left signal workspace, inverted voltage/current ordering across table/phasor/wave, and cramped platform-rendered context menus. Operator provided OMICRON QuickCMC screenshots for interaction patterns only, not for copying product branding.
Paket 1 — one cohesive QML integration, exact branch head
da9cc493ec9aa5bee937f8904307a1fa3a60e7c4Popup.Item(Qt 6.10 Windows native popup would otherwise ignore custom QML styling). Shared divider, tokenized hover/focus/colors/padding, menu width 250, field-anchored popup, zero, honest 50/100% demo setpoints, Equal ABC Magnitudes, Equal ABC Angles, Balance 120°, plus frequency right-click presets/DC.StudioTheme, shared PTP switch used in Main/Advanced, and all new components registered in Qt QML CMake module.Safety & scope
Presentation-only QML/CMake/Qt-workflow structural test; no firmware, network/PTP timestamping, SMP counter, serial lease, profile compiler or wire codec change. Existing 4I+4V model, continuous SMV behaviour and setpoint dispatch unchanged. Simulated
smpSynchis still not clock lock. The 3-channel group action is still sequential, not wire-atomic, and needs separate protocol hardening.Validation and CI discipline
All QML editing committed before opening this PR, so this exact head will get only one normal Qt + Windows Lifecycle + Native Preview suite. Inspect detailed CI logs/QML load, native installer checks and user Windows screenshots before merge. No manual re-run without a failure. Physical board/IED acceptance still required; preserve the known-good v0.1.1 installation during preview testing.
QA findings addressed before merging
TypeErrormessages: an action delegate'smenuidentifier shadowedMenuItem.menu(null when constructed). Fixed by renaming the root identifier toactionMenu.