fix(opctreemanager): resolve OpcFbPath exactly - #147
Merged
Merged
Conversation
GetProtocol took the configured OpcFbPath, and when no OPC UA FB node sat there it stripped the last segment and searched the parent, up to the tree root. A path naming a child of the FB node, or any path with a wrong segment in the middle, silently resolved to an ancestor: the module then worked against a node the operator never wrote, and the log reported that node rather than the setting. A wrong first segment walked to the root and failed with "or any of its ancestors", which says nothing about where the path went wrong. The operator names the node directly in the property grid, so the path is now taken as written. A miss fails with "No OPC UA FB node at path 'X'. OpcFbPath must name the OPC UA FB node itself." GetProtocol takes a Func<string, ITreeItemHlp?> resolver instead of IProjectHlp, matching the seam used by TreeReshaper, PlanExecutor and ProjectSubtreeDisconnector, which is what makes the refusal testable.
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.
Summary
GetProtocoltook the configuredOpcFbPath, and when no OPC UA FB node sat there it stripped the last segment and searched the parent, up to the tree root. A path naming a child of the FB node, or any path with a wrong segment in the middle, silently resolved to an ancestor: the module then worked against a node the operator never wrote, and the log reported that node rather than the setting. A wrong first segment walked to the root and failed withor any of its ancestors, which says nothing about where the path went wrong.The operator names the node directly in the property grid, so the path is now taken as written. A miss fails with
No OPC UA FB node at path 'X'. OpcFbPath must name the OPC UA FB node itself.No new setting and nothing to fill in differently:
OpcFbPathis the same property, with the same defaultСистема.АРМ.OPC UA Siemens. Only the behaviour on a wrong value changes.Type of change
Changes
GetProtocoltakes aFunc<string, ITreeItemHlp?>resolver instead ofIProjectHlp, matching the seam already used byTreeReshaper,PlanExecutorandProjectSubtreeDisconnector. That is what makes the refusal testable.Changes touching FB code
const int *PinIdconstants in the FB class[NonSerialized][ComVisible(true)]+[Guid]untouched on existing FBsDocs/architecture/masterscada-fb-primer.mdandDocs/architecture/architecture.mdDocs/known_issues/Testing
dotnet build NtoLib.sln— 0 errors, 0 warningsdotnet test NtoLib.sln— 373 passeddotnet format NtoLib.sln --verify-no-changes— exit 0GetProtocol_PathBelowTheFbNode_Failsasserts the refusal sentence, not justIsFailed. Verified by experiment: reinstating the ancestor walk restores the old behaviour and the test goes red. AssertingIsFailedalone did not discriminate, because the walk found the node and then failed later onInstance is null.