Prepare opcilloscope for the next release#181
Merged
Merged
Conversation
Full read-through of production source, build, and test run ahead of the next major release. Documents 6 correctness findings (thread-marshalling inconsistency in config load, mixed UTC/local CSV timestamps, scope plotting the truncated display value, invisible --connect warning, UTC timestamp display, dead scope key mappings), robustness notes, tech debt (Terminal.Gui CS0618 set), and confirms security/release-engineering posture. 696/696 tests passing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PD9QJLoPtFsT3JTcKtB4j4
- CSV recording: normalize all timestamps to UTC with an explicit 'Z' designator. Previously the OPC UA SourceTimestamp (UTC) and the DateTime.Now fallback (local) could mix timezones in one file with no way to tell them apart. Kind=Unspecified is treated as UTC per the OPC UA convention; Kind=Local is converted. - Scope: sample the full-precision, culture-invariant RawValue instead of the "F2"-truncated display Value, which quantized plots to 0.01 resolution and flattened smaller-amplitude signals entirely. Booleans now plot as 0/1 so digital signals are visible. - MainWindow: marshal all UI work after awaits onto the UI thread in LoadConfigurationAsync, SaveConfigurationAsync, ConnectAsync and ReconnectAsync, matching the convention used everywhere else in the file. The password prompt now runs via a new UiThread.RunAsync helper that queues a modal dialog on the UI loop and awaits its result. - Program: print the "--connect not implemented" warning before Application.Init() so it is not lost in the alternate screen buffer (same pattern as the existing config-path validation). 710/710 tests pass; adds 14 tests covering UTC normalization, the Z designator, RawValue preference, boolean plotting and invariant parsing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PD9QJLoPtFsT3JTcKtB4j4
Route all UI-layer access to the deprecated static Application object through a small helper surface in Utilities/, alongside the existing UiThread.Run wrapper: - TerminalUi.AddTimeout/RemoveTimeout for main-loop timers - TerminalUi.RunModal/RequestStop for modal dialogs - TerminalUi.Query/ErrorQuery so call sites stop passing Application.Instance to MessageBox - TerminalUi.TrySetClipboardData for the static Clipboard class - TerminalUi.TopRunnableView/IsTopRunnable for top-level view queries All call sites in App/ now use the helpers; behavior is unchanged. The remaining direct uses of the deprecated statics are confined to TerminalUi/UiThread plus the entry points that need the application lifecycle itself (Program.cs Init/Run/Shutdown, MainWindow Application.KeyDown, ThemeManager Application.Driver), which the next commit migrates to the instance-based IApplication API. CS0618 count: 101 -> 27, now concentrated in 5 files. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Cb6n3buYyULFMq4AursDkF
Switch from the deprecated legacy static Application object to the instance-based model introduced in Terminal.Gui 2.4: - Program.cs creates the application with Application.Create(), runs app.Init()/app.Run(mainWindow), and disposes it on exit (Dispose is the instance-based replacement for Application.Shutdown). The IL2026 pragma is dropped: the instance lifecycle carries no trim attributes. - TerminalUi holds the IApplication instance (TerminalUi.App, assigned once at startup) and all helpers now call the non-deprecated instance members: Invoke, AddTimeout/RemoveTimeout, Run/RequestStop, MessageBox overloads taking IApplication, App.Clipboard, TopRunnable. - UiThread.Run marshals via TerminalUi, preserving the existing threading discipline for OPC UA background-thread callbacks. - MainWindow subscribes to keyboard events via App.Keyboard.KeyDown (TerminalUi.Add/RemoveKeyDownHandler) instead of Application.KeyDown. - ThemeManager reads the driver from the instance (TerminalUi.Driver). When no application is running (headless unit tests), fire-and-forget helpers (Invoke, timers, clipboard) are no-ops and interactive ones (modal dialogs, message boxes) throw, matching the effective legacy behavior where those paths were never reachable without a session. CS0618 count: 27 -> 1. The single remaining warning is the unrelated TextView deprecation (superseded by the external gui-cs/Editor package), handled in the next commit. Verified: 696/696 tests pass; TUI smoke-tested under a pseudo-tty (full render of menus/panes, Ctrl+Q quits, exit code 0; --help exits 0). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Cb6n3buYyULFMq4AursDkF
With the Terminal.Gui static-API migration complete the build is warning-free, so enable TreatWarningsAsErrors in Opcilloscope.csproj to stop deprecations from accumulating silently until a package bump turns them into a wall of errors. The one genuinely unfixable warning is suppressed at the call site with a targeted #pragma: HelpDialog's TextView is obsolete in Terminal.Gui 2.4.5, but its designated successor (EditorView) ships in the separate gui-cs/Editor package, and a read-only help pane does not justify a new dependency. Verified clean under warnings-as-errors: Debug and Release builds, dotnet publish (single-file, as used by ci.yml/release.yml), and the full test suite (696/696); TUI pseudo-tty smoke test still exits 0. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Cb6n3buYyULFMq4AursDkF
The Terminal.Gui v2 API notes still told contributors to use the now- deprecated static Application.Invoke()/AddTimeout(). Replace that guidance with the instance-based model the codebase migrated to: route all UI-thread marshalling, timers, modal dialogs, message boxes, and clipboard access through the UiThread/TerminalUi helpers, and note that TreatWarningsAsErrors makes a static-Application call a build error. Also list Utilities/TerminalUi.cs in the project structure and fix the thread-safety and common-issues examples. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Cb6n3buYyULFMq4AursDkF
Cover the TerminalUi contract the migration introduced: fire-and-forget members (Invoke, AddTimeout/RemoveTimeout, TrySetClipboardData, TopRunnableView, IsTopRunnable, Driver, key-down handlers) degrade to no-ops when no application is running, while the interactive members (RunModal, RequestStop, Query, ErrorQuery) throw InvalidOperationException rather than silently skip. Also verify delegation to the running IApplication instance via a mock, and that UiThread.Run marshals through IApplication.Invoke. Closes the coverage gap flagged in PR review; both helpers previously had no dedicated tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Cb6n3buYyULFMq4AursDkF
…i-migration-2sbki2
…w-fc47ew # Conflicts: # App/MainWindow.cs # Program.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This supersedes the older draft E2E approach in #166.
Why
The pre-release review found several issues that could produce stale-session updates, reconnect races, partial subscription state, silent CSV data loss, misleading configuration state, insecure or ambiguous endpoint selection, and non-reproducible release artifacts. The release workflow also lacked complete artifact and legal validation.
User and developer impact
Connections now fail closed unless the requested security and authentication profile is actually available. Explicit lifecycle intents cannot be overtaken by stale reconnect work, monitored data is tied to the active connection generation, recording shutdown reports data loss, and release artifacts are built and tested from reviewed locked graphs.
Validation