Feature/s7 implementation. Refactored S7 module according to the documentation.#3
Merged
EvaTheSalmon merged 3 commits intomasterfrom Apr 7, 2026
Merged
Conversation
Simplified overall protocol. Added PLC synchronisation loop with execution monitoring and reconnect reconciliation. Introduce recipe debounce write and write verification. Added constant state polling. Implement reconnect reconciliation: detect committed/uncommitted state on connect and resolve conflicts via PlcConflictDialog. Wire sync toggle through DomainFacade and RecipeMutationCoordinator to MainWindowViewModel. DataGrid becomes read-only while a recipe is active, with current and past step rows highlighted.
There was a problem hiding this comment.
Pull request overview
Refactors the S7/PLC integration to simplify the protocol (commit flag + recipe_lines), adds continuous execution-state monitoring, and wires PLC sync/reconnect reconciliation into the UI (including conflict resolution and read-only grid behavior during execution).
Changes:
- Simplified PLC “managing area” protocol and removed legacy checksum/transaction handshake types.
- Added PLC sync coordination (debounced writes + verification), execution polling/monitoring, and reconnect reconciliation with conflict dialog.
- Updated UI to expose sync controls/status, execution info, and grid row highlighting/read-only behavior while a recipe is active.
Reviewed changes
Copilot reviewed 80 out of 81 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| SemiStep/UI/UiDi.cs | Registers PLC monitor VM in UI DI. |
| SemiStep/UI/UI.csproj | Adds dialog code-behind DependentUpon mapping. |
| SemiStep/UI/Styles/DataGridStyles.axaml | Adds pseudo-class styles for step highlighting. |
| SemiStep/UI/Styles/ColorPalette.axaml | Adds brushes for current/past step highlighting. |
| SemiStep/UI/RecipeGrid/RecipeRowViewModel.cs | Adds IsCurrentStep/IsPastStep flags for styling. |
| SemiStep/UI/RecipeGrid/RecipeGridViewModel.cs | Binds read-only state and updates step highlighting from PLC execution state. |
| SemiStep/UI/RecipeGrid/CellPresenter.cs | Adds pseudo-classes and bindings for execution highlighting. |
| SemiStep/UI/Plc/PlcMonitorViewModel.cs | New VM to surface PLC execution counters/timing to status bar. |
| SemiStep/UI/Plc/PlcConflictDialog.axaml.cs | Dialog code-behind for conflict choice handling. |
| SemiStep/UI/Plc/PlcConflictDialog.axaml | Conflict dialog UI layout. |
| SemiStep/UI/MainWindow/MainWindowViewModel.cs | Adds sync toggle, sync status display props, and conflict handling. |
| SemiStep/UI/MainWindow/MainWindow.axaml.cs | Passes MainWindow reference to VM for dialogs. |
| SemiStep/UI/MainWindow/MainWindow.axaml | Binds grid read-only state to PLC execution. |
| SemiStep/UI/MainWindow/AppStatusBar.axaml | Adds sync controls + execution monitor display in status bar. |
| SemiStep/UI/Coordinator/RecipeQueryService.cs | Exposes sync/execution-related state from DomainFacade. |
| SemiStep/UI/Coordinator/RecipeMutationCoordinator.cs | Adds sync enable/disable + conflict/event relays + execution state wiring. |
| SemiStep/TypesShared/Plc/PlcSyncStatus.cs | Introduces shared sync status enum for UI/Domain. |
| SemiStep/TypesShared/Plc/PlcProtocolSettings.cs | Adds KeepAliveIntervalMs to protocol settings. |
| SemiStep/TypesShared/Plc/PlcProtocolLayout.cs | Removes HeaderDb from protocol layout. |
| SemiStep/TypesShared/Plc/PlcManagingAreaState.cs | New shared managing-area state record. |
| SemiStep/TypesShared/Plc/PlcExecutionInfo.cs | New shared execution info record for monitoring. |
| SemiStep/TypesShared/Plc/Memory/ManagingDbLayout.cs | Simplifies managing DB layout to committed + recipe_lines. |
| SemiStep/TypesShared/Plc/Memory/HeaderDbLayout.cs | Removes legacy header DB layout type. |
| SemiStep/TypesShared/Domain/IS7Service.cs | Extends IS7Service with execution state + PLC read APIs. |
| SemiStep/TypesShared/Domain/IPlcSyncService.cs | Introduces sync service abstraction for Domain/UI. |
| SemiStep/Tests/YamlConfigs/WithGroups/connection/connection.yaml | Updates managing DB config fields to new layout. |
| SemiStep/Tests/YamlConfigs/Standard/connection/connection.yaml | Adds keep-alive interval and new managing layout fields. |
| SemiStep/Tests/YamlConfigs/Standalone/UnknownYamlFields/connection/connection.yaml | Updates managing DB config fields to new layout. |
| SemiStep/Tests/UI/RecipeMutationCoordinatorTests.cs | Updates coordinator construction + initialization. |
| SemiStep/Tests/UI/RecipeMutationCoordinatorLoadRecipeTests.cs | Adds IPlcSyncService registration and new coordinator ctor usage. |
| SemiStep/Tests/UI/RecipeGridViewModelTests.cs | Updates coordinator construction + initialization. |
| SemiStep/Tests/UI/Helpers/UIFixture.cs | Updates coordinator construction + initialization. |
| SemiStep/Tests/Tests.csproj | Adds S7 project reference for new unit tests. |
| SemiStep/Tests/S7/S7ServiceTests.cs | Adds tests for keep-alive and disconnect detection behavior. |
| SemiStep/Tests/S7/PlcTransactionExecutorTests.cs | Adds tests for new write/verify sequence and retry behavior. |
| SemiStep/Tests/S7/PlcSyncCoordinatorTests.cs | Adds tests for debounce sync + status transitions. |
| SemiStep/Tests/S7/PlcExecutionMonitorTests.cs | Adds tests for execution polling lifecycle/behavior. |
| SemiStep/Tests/S7/ManagingAreaCodecTests.cs | Adds tests for new managing-area serialization. |
| SemiStep/Tests/S7/Helpers/StubIs7ServiceForSync.cs | Adds IS7Service stub for sync coordinator tests. |
| SemiStep/Tests/S7/Helpers/FakeS7Transport.cs | Adds transport stub for executor/sync tests. |
| SemiStep/Tests/S7/Helpers/FakeS7Driver.cs | Adds driver stub for S7Service tests. |
| SemiStep/Tests/S7/Helpers/FakeExecutionTransport.cs | Adds execution-state transport stub for monitor tests. |
| SemiStep/Tests/Helpers/StubPlcSyncService.cs | Adds IPlcSyncService stub used across tests. |
| SemiStep/Tests/Helpers/StubIs7Service.cs | Extends IS7Service test stub with new API surface. |
| SemiStep/Tests/Domain/DomainFacadeReconnectTests.cs | Adds tests for reconnect reconciliation + conflict detection/reset behavior. |
| SemiStep/Tests/Csv/Helpers/CsvTestHelper.cs | Registers IPlcSyncService stub in CSV test DI. |
| SemiStep/Tests/Core/Helpers/CoreTestHelper.cs | Registers IPlcSyncService stub in core test DI. |
| SemiStep/S7/Sync/SyncStatus.cs | Removes legacy internal sync status enum. |
| SemiStep/S7/Sync/PlcWriteVerificationException.cs | Adds exception for write verification exhaustion. |
| SemiStep/S7/Sync/PlcTransactionExecutor.cs | Refactors to IS7Transport + new managing-area protocol + read-back verification. |
| SemiStep/S7/Sync/PlcSyncCoordinator.cs | Implements IPlcSyncService with debounce + status/error reporting. |
| SemiStep/S7/Sync/PlcExecutionMonitor.cs | Adds polling monitor that publishes PlcExecutionInfo stream. |
| SemiStep/S7/Serialization/RecipeConverter.cs | Refactors serialization/deserialization for clarity and validation. |
| SemiStep/S7/Serialization/ManagingAreaCodec.cs | Updates codec for simplified managing-area layout and validates layout. |
| SemiStep/S7/Serialization/ArrayCodec.cs | Makes ReadArrayCurrentSize static. |
| SemiStep/S7/S7Driver.cs | Switches driver to implement IS7Driver interface. |
| SemiStep/S7/S7Di.cs | Updates DI wiring for new S7Service, execution monitor, and sync service. |
| SemiStep/S7/S7.csproj | Adds System.Reactive dependency. |
| SemiStep/S7/Protocol/ProtocolConstants.cs | Removes unused protocol version constant. |
| SemiStep/S7/Protocol/PlcSyncTimeoutException.cs | Removes legacy timeout exception type. |
| SemiStep/S7/Protocol/PlcSyncStatus.cs | Removes legacy PLC status enum (protocol handshake removed). |
| SemiStep/S7/Protocol/PlcSyncException.cs | Removes legacy sync exception type. |
| SemiStep/S7/Protocol/PlcSyncError.cs | Removes legacy PLC error enum. |
| SemiStep/S7/Protocol/PlcRecipeActiveException.cs | Removes legacy recipe-active exception type. |
| SemiStep/S7/Protocol/PlcBusyException.cs | Removes legacy busy exception type. |
| SemiStep/S7/Protocol/PcStatus.cs | Removes legacy PC status enum. |
| SemiStep/S7/Protocol/ManagingAreaState.cs | Simplifies managing-area state record to committed + recipe_lines. |
| SemiStep/S7/Protocol/ManagingAreaPcData.cs | Simplifies PC managing-area data record to committed + recipe_lines. |
| SemiStep/S7/Protocol/ChecksumCalculator.cs | Removes checksum calculator (CRC protocol removed). |
| SemiStep/S7/IS7Transport.cs | Introduces transport abstraction for testing/executor decoupling. |
| SemiStep/S7/IS7Driver.cs | Introduces driver abstraction (connect/disconnect + transport). |
| SemiStep/S7/Facade/S7Service.cs | New service with keep-alive + reconnect loop + execution monitor integration. |
| SemiStep/S7/Facade/Is7Service.cs | Removes prior IS7Service implementation. |
| SemiStep/Domain/Facade/DomainFacade.cs | Adds sync enable/disable, reconnect reconciliation, conflict detection, and exposes sync/execution state. |
| SemiStep/Domain/DomainDi.cs | Injects IPlcSyncService into DomainFacade. |
| SemiStep/Config/Mapping/ConnectionMapper.cs | Maps new keep-alive + simplified managing layout; removes header mapping. |
| SemiStep/Config/Dto/ConnectionDto.cs | Adds keep_alive_interval_ms and removes header/legacy managing offsets. |
| SemiStep/Application/Program.cs | Initializes RecipeMutationCoordinator on startup. |
| Docs/05-plc-protocol.md | Updates protocol documentation to simplified commit-based scheme. |
| Docs/04-plc.md | Updates PLC integration docs for new scheme and UX behavior. |
| .gitignore | Ignores Docs/plans directory. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+92
to
+107
| private void OnExecutionStateChanged(PlcExecutionInfo info) | ||
| { | ||
| for (var i = 0; i < RecipeRows.Count; i++) | ||
| { | ||
| if (info.RecipeActive) | ||
| { | ||
| RecipeRows[i].IsCurrentStep = i == info.ActualLine; | ||
| RecipeRows[i].IsPastStep = i < info.ActualLine; | ||
| } | ||
| else | ||
| { | ||
| RecipeRows[i].IsCurrentStep = false; | ||
| RecipeRows[i].IsPastStep = false; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
On every ExecutionState update, this loops through all RecipeRows and raises property changes for every row. With a ~100ms polling interval and potentially hundreds/thousands of rows, this will create a lot of UI churn. Consider tracking the previous (RecipeActive, ActualLine) and only updating the rows whose state actually changes (e.g., previous current/past rows + new current/past rows), and skip setting properties when the value is already correct.
Comment on lines
+35
to
+48
| public void Stop() | ||
| { | ||
| _pollCts?.Cancel(); | ||
| _pollCts?.Dispose(); | ||
| _pollCts = null; | ||
|
|
||
| var taskToWait = _pollTask; | ||
| _pollTask = null; | ||
|
|
||
| // Wait for the poll loop to terminate before nulling the task, so that a | ||
| // subsequent Start() cannot start a second concurrent loop. | ||
| taskToWait?.Wait(TimeSpan.FromSeconds(5)); | ||
|
|
||
| PublishAndTrack(PlcExecutionInfo.Empty); |
There was a problem hiding this comment.
Stop() synchronously waits on the poll task. When PLC disconnect is detected inside PollLoopAsync, it calls onConnectionLost(), and S7Service.OnConnectionLost() calls executionMonitor.Stop(); in that path Stop() can end up waiting on the current poll task (self-wait), delaying shutdown by up to 5s and risking thread starvation. Consider avoiding a blocking wait here (prefer StopAsync), or explicitly detect self-wait and skip waiting, while still preventing multiple concurrent loops (e.g., via an interlocked state flag).
Comment on lines
+238
to
+244
| // Floats are serialised to raw IEEE 754 bytes, written to the PLC, and read back without | ||
| // any arithmetic transformation. The round-trip is byte-exact, so bit-exact equality is | ||
| // the correct comparison here — not an epsilon-based approximation. | ||
| private static bool CompareFloats_ExpectedBytesEqual(float actual, float expected) | ||
| { | ||
| return actual == expected; | ||
| } |
There was a problem hiding this comment.
CompareFloats_ExpectedBytesEqual claims bit/byte-exact comparison, but uses actual == expected, which is not bit-exact for NaN (NaN != NaN even if the payload bits match) and treats +0 and -0 as equal (different IEEE-754 bit patterns). If you really want byte-exact verification, compare BitConverter.SingleToInt32Bits(actual) to BitConverter.SingleToInt32Bits(expected).
Comment on lines
+18
to
+47
| private string _lineTimeLeft = "0.0 s"; | ||
| private int _forLoopCount1; | ||
| private int _forLoopCount2; | ||
| private int _forLoopCount3; | ||
|
|
||
| public PlcMonitorViewModel(RecipeMutationCoordinator coordinator) | ||
| { | ||
| coordinator.ExecutionState | ||
| .ObserveOn(RxApp.MainThreadScheduler) | ||
| .Subscribe(OnExecutionStateChanged) | ||
| .DisposeWith(_disposables); | ||
| } | ||
|
|
||
| public bool IsRecipeActive | ||
| { | ||
| get => _isRecipeActive; | ||
| private set => this.RaiseAndSetIfChanged(ref _isRecipeActive, value); | ||
| } | ||
|
|
||
| public int ActualLine | ||
| { | ||
| get => _actualLine; | ||
| private set => this.RaiseAndSetIfChanged(ref _actualLine, value); | ||
| } | ||
|
|
||
| public string LineTimeLeft | ||
| { | ||
| get => _lineTimeLeft; | ||
| private set => this.RaiseAndSetIfChanged(ref _lineTimeLeft, value); | ||
| } |
There was a problem hiding this comment.
LineTimeLeft is populated from PlcExecutionInfo.StepCurrentTime, which (per docs) is elapsed time since the step started, not time left. Either rename this property (and backing field) to reflect elapsed time (e.g., StepElapsed) or compute and display actual remaining time if that’s what the UI intends.
Comment on lines
13
to
26
| @@ -21,18 +22,31 @@ | |||
| | Параметр | Значение | | |||
| |----------|----------| | |||
| | Протокол | S7 (Siemens) | | |||
| | Библиотека | S7.Net | | |||
| | Библиотека | Sharp7 (чистый C# порт Snap7) | | |||
| | PLC | Siemens S7-1200 / S7-1500 | | |||
There was a problem hiding this comment.
This doc now states the app uses “Sharp7 (чистый C# порт Snap7)”, but the codebase references and describes S7NetPlus (package S7netplus), not Sharp7. Please align the documentation with the actual library used (or update the implementation/dependencies if Sharp7 is intended).
Docs/05-plc-protocol.md
Outdated
| │ Шаг 2: DBWrite DB_INT │ │ | ||
| │ DBWrite DB_FLOAT │ │ | ||
| │ DBWrite DB_STRING │ │ | ||
| │ (Sharp7 разбивает на PDU) │ │ |
There was a problem hiding this comment.
The workflow text mentions “Sharp7 разбивает на PDU”, but elsewhere in this document you describe S7NetPlus as the client, and the repo depends on S7netplus. Please make the library naming consistent here (Sharp7 vs S7NetPlus) to avoid misleading readers.
Suggested change
| │ (Sharp7 разбивает на PDU) │ │ | |
| │ (S7NetPlus разбивает на PDU) │ │ |
… bit-exact float comparison, rename StepElapsedTime, fix Sharp7 references in docs
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
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.
Simplified overall protocol.
Added PLC synchronisation loop with execution monitoring and reconnect reconciliation.
Introduce recipe debounce write and write verification.
Added constant state polling.
Implement reconnect reconciliation: detect committed/uncommitted state on connect and resolve conflicts via PlcConflictDialog.
Wire sync toggle through DomainFacade and RecipeMutationCoordinator to MainWindowViewModel. DataGrid becomes read-only while a recipe is active, with current and past step rows highlighted.