Parse nodeId from href #1368

danielpeintner · 2025-04-10T08:55:11Z

I started to look into. Unfortunately, OPC UA protocol is blocked on my machine at the moment and I need to fix that issue first. I just coded what I think should work...

~~TODO: Update readme, see https://github.com/eclipse-thingweb/node-wot/tree/master/packages/binding-opcua#form-extensions~~ b912d32

fixes #1367

keep *old* form as fallback

danielpeintner · 2025-04-11T09:26:52Z

packages/binding-opcua/src/opcua-protocol-client.ts


 export interface FormPartialNodeDescription {
-    "opcua:nodeId": NodeIdLike | NodeByBrowsePath;
+    "opcua:nodeId"?: NodeIdLike | NodeByBrowsePath;


General question/comment:
Why do we have interface FormPartialNodeDescription and OPCUAFormInvoke.
It is somewhat similar without extending (OPCUA)Form.

Moreover, OPCUAForm extends FormPartialNodeDescription again !?
Some dependencies I don't understand, and I wonder whether we can simplify it.

@erossignon

I simplified it, and we do not see any problems when testing via unit tests nor when testing with real devices

danielpeintner · 2025-04-11T09:29:06Z

packages/binding-opcua/src/opcua-protocol-client.ts


 export interface OPCUAFormInvoke extends OPCUAForm {
-    "opcua:method": NodeIdLike | NodeByBrowsePath;
+    "opcua:method"?: NodeIdLike | NodeByBrowsePath;


I don't think this line addition is needed. OPCUAForm has already the same?
OPCUAForm extends FormPartialNodeDescription which contains it...

relu91 · 2025-04-22T08:04:47Z

Are there any blockers here?

danielpeintner · 2025-04-22T10:13:33Z

Are there any blockers here?

Not really from my side. Unfortunately, I cannot really test the changes myself since on my windows box OPCUA is blocked. Need to find another machine.

@egekorkan you mentioned you can (want) to test it. Would be great if you can provide feedback!

relu91 · 2025-04-22T10:20:19Z

Can't we add some additional tests alongside the existing ones (eg. remove the old opcua:method and use the url instead) to test this properly?

--> revert

codecov · 2025-04-22T15:02:29Z

Codecov Report

Attention: Patch coverage is 85.29412% with 5 lines in your changes missing coverage. Please review.

Project coverage is 78.14%. Comparing base (53a9c66) to head (bec5765).
Report is 10 commits behind head on master.

Files with missing lines	Patch %	Lines
...ackages/binding-opcua/src/opcua-protocol-client.ts	85.29%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1368      +/-   ##
==========================================
+ Coverage   77.63%   78.14%   +0.51%     
==========================================
  Files          83       79       -4     
  Lines       16603    15270    -1333     
  Branches     1611     1445     -166     
==========================================
- Hits        12890    11933     -957     
+ Misses       3682     3320     -362     
+ Partials       31       17      -14

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

danielpeintner · 2025-04-22T15:03:14Z

an't we add some additional tests alongside the existing ones (eg. remove the old opcua:method and use the url instead) to test this properly?

I added some counter tests next to the existing ones ... some tests require special syntax w.r.t. to the server which the href approach doesn't offer

danielpeintner · 2025-04-22T15:34:15Z

Note: the ECA check seems to be broken

egekorkan · 2025-04-22T16:47:12Z

@egekorkan you mentioned you can (want) to test it. Would be great if you can provide feedback!

I can try with some real OPC UA device on Thursday morning :)

egekorkan · 2025-04-24T06:37:13Z

Note: the ECA check seems to be broken

See https://gitlab.eclipse.org/eclipsefdn/helpdesk/-/issues/5921

egekorkan · 2025-04-24T15:44:29Z

Happy to confirm that the PR works with real devices. I have tried a TD with base and without base and both worked, meaning the URL arithmetic from the core package cause no issue. Of course, and as expected, one cannot split the query parameter and put a part of it into the base as we are not doing simple string concat.

relu91

I think we can improve the doc a little bit, other than that it's good to go.

packages/binding-opcua/src/opcua-protocol-client.ts

danielpeintner · 2025-04-29T07:29:50Z

I will update the readme before integrating the PR.

danielpeintner · 2025-04-29T08:44:25Z

The PR seems to be ready to merge. README has been updated.

relu91

LGTM

erossignon · 2025-09-02T13:12:53Z

Not really from my side. Unfortunately, I cannot really test the changes myself since on my windows box OPCUA is blocked. Need to find another machine.

Daniel, If you don't have a PLC OPCUA server at hand, you can still start in a standalone mode, the small test server that we use in the unit tests.

>cd packages\binding-opcua
packages\binding-opcua> DEBUG=node-wot*  npx tsx test\fixture\basic-opcua-server.ts

 node-wot:binding-opcua:basic-opcua-server:info Server started: opc.tcp://yourmachine:7890 +0ms

danielpeintner · 2025-09-02T14:39:32Z

The problem was/is that the protocol is blocked on my company device, but meanwhile I found a detour to make it work..

refactor: start trying to parse nodeId from href

e9f2acc

keep *old* form as fallback

danielpeintner requested a review from relu91 as a code owner April 10, 2025 08:55

danielpeintner added the binding-opcua Issues related to opc ua protocol binding label Apr 10, 2025

danielpeintner marked this pull request as draft April 10, 2025 08:55

danielpeintner added 5 commits April 10, 2025 10:59

fix: lint issues

b901da2

chore: install url package

f6d2966

chore: add import for Node 18

eb49847

fix: url.parse' was deprecated. Use 'url.URL' constructor instead

a6ec8bd

chore: add simple href parser tests

52b067c

danielpeintner commented Apr 11, 2025

View reviewed changes

refactor: clean-up/remove interfaces that are not needed/used

40fb5f0

danielpeintner marked this pull request as ready for review April 22, 2025 10:14

danielpeintner added 2 commits April 22, 2025 15:57

test: add href id tests

7fe2a08

refactor: server part expects special syntax in some cases

bec5765

--> revert

relu91 approved these changes Apr 24, 2025

View reviewed changes

packages/binding-opcua/src/opcua-protocol-client.ts Outdated Show resolved Hide resolved

sebastiankb approved these changes Apr 28, 2025

View reviewed changes

danielpeintner added 2 commits April 29, 2025 09:58

docs: mark "opcua:nodeId" as deprecated

2470c80

refactor: update readme and examples

b912d32

danielpeintner mentioned this pull request Apr 29, 2025

OPC UA: Empty href with base is not accepted #1370

Closed

relu91 approved these changes Apr 29, 2025

View reviewed changes

relu91 merged commit 5491801 into eclipse-thingweb:master Apr 29, 2025
17 of 26 checks passed

danielpeintner mentioned this pull request Jul 14, 2025

[OPC UA] Support browsePath on form level #1406

Open

Parse nodeId from href #1368

Parse nodeId from href #1368

Uh oh!

Conversation

danielpeintner commented Apr 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

danielpeintner Apr 11, 2025

Choose a reason for hiding this comment

Uh oh!

danielpeintner Apr 29, 2025

Choose a reason for hiding this comment

Uh oh!

danielpeintner Apr 11, 2025

Choose a reason for hiding this comment

Uh oh!

relu91 commented Apr 22, 2025

Uh oh!

danielpeintner commented Apr 22, 2025

Uh oh!

relu91 commented Apr 22, 2025

Uh oh!

codecov bot commented Apr 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

danielpeintner commented Apr 22, 2025

Uh oh!

danielpeintner commented Apr 22, 2025

Uh oh!

egekorkan commented Apr 22, 2025

Uh oh!

egekorkan commented Apr 24, 2025

Uh oh!

egekorkan commented Apr 24, 2025

Uh oh!

relu91 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

danielpeintner commented Apr 29, 2025

Uh oh!

danielpeintner commented Apr 29, 2025

Uh oh!

relu91 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

erossignon commented Sep 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

danielpeintner commented Sep 2, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

danielpeintner commented Apr 10, 2025 •

edited

Loading

codecov bot commented Apr 22, 2025 •

edited

Loading

erossignon commented Sep 2, 2025 •

edited

Loading