fix(communication-plugin): Offer only valid connected aps as move targets (openscd#1685)

Author: clepski

.github/workflows/pr-preview.yml

@@ -23,6 +23,12 @@ jobs:
         uses: actions/checkout@v3
         with:
           submodules: "true"
+
+      - name: Use Node.js 18.x
+        uses: actions/setup-node@v1
+        with:
+          node-version: "18.x"
+
       - name: Install and Build OpenSCD
         if: github.event.action != 'closed' # You might want to skip the build if the PR has been closed
         run: |

packages/plugins/src/editors/communication/foundation.ts

@@ -33,3 +33,78 @@ export function getAllConnectedAPsOfSameIED(
     doc.querySelectorAll(`SubNetwork > ConnectedAP[iedName=${iedName}`)
   );
 }
+
+type communicationElementTag = 'GSE' | 'SMV';
+
+const controlTagDictionary: { [key in communicationElementTag]: string } = {
+  GSE: 'GSEControl',
+  SMV: 'SampledValueControl'
+};
+
+export function canMoveCommunicationElementToConnectedAP(
+  communicationElement: Element,
+  connectedAP: Element,
+  doc: XMLDocument
+): boolean {
+  const currentConnectedAP = getCurrentConnectedAP(communicationElement);
+
+  if (!currentConnectedAP || currentConnectedAP === connectedAP) {
+    return false;
+  }
+
+  const apName = currentConnectedAP.getAttribute('apName');
+  const iedName = currentConnectedAP.getAttribute('iedName');
+
+  if (!apName || !iedName) {
+    return false;
+  }
+
+  const ied = doc.querySelector(`IED[name=${iedName}]`);
+
+  if (!ied) {
+    return false;
+  }
+
+  const targetApName = connectedAP.getAttribute('apName');
+  const targetAp = ied.querySelector(`:scope > AccessPoint[name=${targetApName}]`);
+
+  if (!targetAp || !targetApName) {
+    return false;
+  }
+
+  const server = queryServer(ied, targetApName);
+  if (!server) {
+    return false;
+  }
+
+  const ldInst = communicationElement.getAttribute('ldInst');
+  const cbName = communicationElement.getAttribute('cbName');
+  const controlTag = controlTagDictionary[communicationElement.tagName as communicationElementTag];
+
+  const controlElement = server.querySelector(`:scope > LDevice[inst=${ldInst}] ${controlTag}[name=${cbName}]`);
+  const serverHasControl = controlElement !== null;
+
+  return serverHasControl;
+}
+
+function queryServer(ied: Element, apName: string): Element | null {
+  const accessPoint = ied.querySelector(`AccessPoint[name=${apName}]`);
+  if (!accessPoint) {
+    return null;
+  }
+
+  const server = accessPoint.querySelector('Server');
+  if (server) {
+    return server;
+  }
+
+  const serverAt = accessPoint.querySelector('ServerAt');
+  const serverApName = serverAt?.getAttribute('apName');
+  if (!serverApName) {
+    return null;
+  }
+
+  const accessPointWithServer = ied.querySelector(`AccessPoint[name=${serverApName}]`);
+
+  return accessPointWithServer?.querySelector('Server') ?? null;
+}

packages/plugins/src/editors/communication/gse-editor.ts

@@ -15,7 +15,7 @@ import { newWizardEvent } from '@openscd/open-scd/src/foundation.js';
 import { newActionEvent } from '@openscd/core/foundation/deprecated/editor.js';
 import { sizableGooseIcon } from '@openscd/open-scd/src/icons/icons.js';
 import { editGseWizard } from '../../wizards/gse.js';
-import { getAllConnectedAPsOfSameIED } from './foundation.js';
+import { canMoveCommunicationElementToConnectedAP, getAllConnectedAPsOfSameIED } from './foundation.js';
 
 @customElement('gse-editor')
 export class GseEditor extends LitElement {
@@ -62,11 +62,15 @@ export class GseEditor extends LitElement {
   }
 
   render(): TemplateResult {
-    const allConnectedAPsOfSameIED = getAllConnectedAPsOfSameIED(
+    const validTargetConnectedAPs = getAllConnectedAPsOfSameIED(
       this.element,
       this.doc
-    );
-    const hasMoreThanOneConnectedAP = allConnectedAPsOfSameIED.length > 1;
+    ).filter(cap => canMoveCommunicationElementToConnectedAP(
+        this.element!,
+        cap,
+        this.doc
+      ));
+    const hasValidConnectedAPMoveTarget = validTargetConnectedAPs.length > 0;
 
     return html`<action-icon label="${this.label}" .icon="${sizableGooseIcon}"
       ><mwc-fab
@@ -86,7 +90,7 @@ export class GseEditor extends LitElement {
         mini
         icon="forward"
         class="gse-move-button"
-        ?disabled="${!hasMoreThanOneConnectedAP}"
+        ?disabled="${!hasValidConnectedAPMoveTarget}"
         @click="${() => this.openGseMoveDialog()}"
       >
       </mwc-fab>

packages/plugins/src/editors/communication/smv-editor.ts

@@ -15,7 +15,7 @@ import { sizableSmvIcon } from '@openscd/open-scd/src/icons/icons.js';
 import { newWizardEvent } from '@openscd/open-scd/src/foundation.js';
 import { newActionEvent } from '@openscd/core/foundation/deprecated/editor.js';
 import { editSMvWizard } from '../../wizards/smv.js';
-import { getAllConnectedAPsOfSameIED } from './foundation.js';
+import { canMoveCommunicationElementToConnectedAP, getAllConnectedAPsOfSameIED } from './foundation.js';
 
 @customElement('smv-editor')
 export class SmvEditor extends LitElement {
@@ -62,11 +62,15 @@ export class SmvEditor extends LitElement {
   }
 
   render(): TemplateResult {
-    const allConnectedAPsOfSameIED = getAllConnectedAPsOfSameIED(
+    const validTargetConnectedAPs = getAllConnectedAPsOfSameIED(
       this.element,
       this.doc
-    );
-    const hasMoreThanOneConnectedAP = allConnectedAPsOfSameIED.length > 1;
+    ).filter(cap => canMoveCommunicationElementToConnectedAP(
+        this.element!,
+        cap,
+        this.doc
+      ));
+    const hasValidConnectedAPMoveTarget = validTargetConnectedAPs.length > 0;
 
     return html`<action-icon label="${this.label}" .icon="${sizableSmvIcon}"
       ><mwc-fab
@@ -86,7 +90,7 @@ export class SmvEditor extends LitElement {
         mini
         icon="forward"
         class="smv-move-button"
-        ?disabled=${!hasMoreThanOneConnectedAP}
+        ?disabled=${!hasValidConnectedAPMoveTarget}
         @click="${() => this.openSmvMoveDialog()}"
       >
       </mwc-fab>

packages/plugins/src/editors/communication/subnetwork-editor.ts

@@ -24,7 +24,7 @@ import {
 import { newActionEvent } from '@openscd/core/foundation/deprecated/editor.js';
 import { createConnectedApWizard } from '../../wizards/connectedap.js';
 import { wizards } from '../../wizards/wizard-library.js';
-import { getAllConnectedAPsOfSameIED } from './foundation.js';
+import { canMoveCommunicationElementToConnectedAP, getAllConnectedAPsOfSameIED } from './foundation.js';
 
 /** [[`Communication`]] subeditor for a `SubNetwork` element. */
 @customElement('subnetwork-editor')
@@ -200,7 +200,12 @@ export class SubNetworkEditor extends LitElement {
       this.moveTargetElement,
       this.doc
     );
-    const currentConnectedAP = this.moveTargetElement.closest('ConnectedAP');
+
+    const validTargetConnectedAPs = allConnectedAPs.filter(cap => canMoveCommunicationElementToConnectedAP(
+      this.moveTargetElement!,
+      cap,
+      this.doc
+    ));
 
     return html`
       <mwc-dialog
@@ -218,7 +223,7 @@ export class SubNetworkEditor extends LitElement {
                 class=${connectedAP === this.newlySelectedAP ? 'selected' : ''}
                 @click=${() => (this.newlySelectedAP = connectedAP)}
                 ?selected=${connectedAP === this.newlySelectedAP}
-                ?disabled=${connectedAP === currentConnectedAP}
+                ?disabled=${!validTargetConnectedAPs.includes(connectedAP)}
               >
                 ${connectedAP.getAttribute('iedName')} >
                 ${connectedAP.getAttribute('apName')}

packages/plugins/test/testfiles/communication/move-gse-testfile.scd

@@ -0,0 +1,41 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<SCL xmlns="http://www.iec.ch/61850/2003/SCL" version="2007" revision="B" release="4">
+    <Communication>
+        <SubNetwork name="SN1" type="Ethernet">
+            <ConnectedAP iedName="IED1" apName="AP1">
+            </ConnectedAP>
+            <ConnectedAP iedName="IED1" apName="AP3">
+            </ConnectedAP>
+        </SubNetwork>
+        <SubNetwork name="SN2" type="Ethernet">
+            <ConnectedAP iedName="IED1" apName="AP2">
+                <GSE ldInst="LD1" cbName="GSE1">
+                </GSE>
+                <SMV ldInst="LD1" cbName="SM1">
+                </SMV>
+            </ConnectedAP>
+            <ConnectedAP iedName="IED1" apName="AP4">
+            </ConnectedAP>
+        </SubNetwork>
+    </Communication>
+    <IED name="IED1">
+        <AccessPoint name="AP1" type="IED">
+            <Server>
+                <LDevice inst="LD1">
+                    <LN0 lnClass="LLN0" lnInst="INST1">
+                        <GSEControl name="GSE1" desc="GSE Control Example" appID="GSEApp1" confRev="1" />
+                        <SampledValueControl name="SM1" />
+                    </LN0>
+                </LDevice>
+            </Server>
+        </AccessPoint>
+        <AccessPoint name="AP2">
+           <ServerAt apName="AP1" />
+        </AccessPoint>
+        <AccessPoint name="AP3">
+        </AccessPoint>
+        <AccessPoint name="AP4">
+           <ServerAt apName="AP1" />
+        </AccessPoint>
+    </IED>
+</SCL>

packages/plugins/test/unit/editors/communication/foundation.test.ts

@@ -0,0 +1,54 @@
+import { expect } from '@open-wc/testing';
+import { getAllConnectedAPsOfSameIED, canMoveCommunicationElementToConnectedAP } from '../../../../src/editors/communication/foundation.js';
+
+describe('Communication plugin foundation', () => {
+  let doc: XMLDocument;
+  
+  beforeEach(async () => {
+    doc = await fetch('/test/testfiles/communication/move-gse-testfile.scd')
+    .then(response => response.text())
+    .then(str => new DOMParser().parseFromString(str, 'application/xml'));
+  });
+
+  it('should determine if gse can be moved to connected AP', () => {
+    const gseElement = doc.querySelector('GSE')!;
+
+    const aps = getAllConnectedAPsOfSameIED(gseElement, doc);
+
+    const connectedAP1 = aps.find(ap => ap.getAttribute('apName') === 'AP1')!;
+    const connectedAP2 = aps.find(ap => ap.getAttribute('apName') === 'AP2')!;
+    const connectedAP3 = aps.find(ap => ap.getAttribute('apName') === 'AP3')!;
+    const connectedAP4 = aps.find(ap => ap.getAttribute('apName') === 'AP4')!;
+
+    const canMoveToAP1 = canMoveCommunicationElementToConnectedAP(gseElement, connectedAP1, doc);
+    const canMoveToAP2 = canMoveCommunicationElementToConnectedAP(gseElement, connectedAP2, doc);
+    const canMoveToAP3 = canMoveCommunicationElementToConnectedAP(gseElement, connectedAP3, doc);
+    const canMoveToAP4 = canMoveCommunicationElementToConnectedAP(gseElement, connectedAP4, doc);
+
+    expect(canMoveToAP1).to.be.true;
+    expect(canMoveToAP2).to.be.false;
+    expect(canMoveToAP3).to.be.false;
+    expect(canMoveToAP4).to.be.true;
+  });
+
+    it('should determine if smv can be moved to connected AP', () => {
+    const smvElement = doc.querySelector('SMV')!;
+
+    const aps = getAllConnectedAPsOfSameIED(smvElement, doc);
+
+    const connectedAP1 = aps.find(ap => ap.getAttribute('apName') === 'AP1')!;
+    const connectedAP2 = aps.find(ap => ap.getAttribute('apName') === 'AP2')!;
+    const connectedAP3 = aps.find(ap => ap.getAttribute('apName') === 'AP3')!;
+    const connectedAP4 = aps.find(ap => ap.getAttribute('apName') === 'AP4')!;
+
+    const canMoveToAP1 = canMoveCommunicationElementToConnectedAP(smvElement, connectedAP1, doc);
+    const canMoveToAP2 = canMoveCommunicationElementToConnectedAP(smvElement, connectedAP2, doc);
+    const canMoveToAP3 = canMoveCommunicationElementToConnectedAP(smvElement, connectedAP3, doc);
+    const canMoveToAP4 = canMoveCommunicationElementToConnectedAP(smvElement, connectedAP4, doc);
+
+    expect(canMoveToAP1).to.be.true;
+    expect(canMoveToAP2).to.be.false;
+    expect(canMoveToAP3).to.be.false;
+    expect(canMoveToAP4).to.be.true;
+  });
+});