Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge Server into Main. #717

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Merge Server into Main. #717

wants to merge 1 commit into from

Conversation

danomagnum
Copy link

I believe I've got the server functioning well enough to merge into the main branch and let users start trying it. I've updated the readme to detail where the server is from an implementation standpoint.

There are three example servers in the examples/server directory.

  1. A server created with a go map[string]any to integrate a ua server into applications that don't want to mess with ua Nodes.
  2. A server created with custom Nodes,
  3. A server with nodes created by import a NodeSet2.xml file.

There is a server.Namespace interface (which the map namespace and node namespace both implement) that can be used to create custom "node drivers" if a user needs to control how the server interacts with their application.

As far as I can tell, all existing tests are passing.

This retains the integration tests against the python server and adds some "self-integration" tests against the gopcua server for the features that are currently implemented. You'll see them in the makefile and the uatest2 directory.

The crypto part is not working. Neither is authentication.

I was able to get sign/encrypt working at one point if I hard-coded the encryption scheme but not when trying to detect it at runtime, so I would say it is close to functional but there's still some critical parts missing.

@danomagnum
Copy link
Author

Apparently slices and slog were added in a version after the tests run. I'll get that all sorted out. Strangely, the go.mod file still lists 1.20 but it's using packages from 1.22 on my system anyway.

@danomagnum
Copy link
Author

Issues with 1.22 packages resolved.

@kung-foo
Copy link
Member

Apparently slices and slog were added in a version after the tests run. I'll get that all sorted out. Strangely, the go.mod file still lists 1.20 but it's using packages from 1.22 on my system anyway.

@danomagnum Since the policy of the repo is that we support the "current" (1.22) and "previous" (1.21) versions of Go, I think we should be ok to include the newer slices/slog packages.

@kung-foo
Copy link
Member

Also we need to decide if this should be squashed. There's a lot of commits plus merges which makes the history messy. And it's generally been the policy in this repo to squash. Not sure if @magiconair has any input on this.

@danomagnum
Copy link
Author

10-4. I'll squash everything when I get a chance.

@danomagnum
Copy link
Author

That squashed everything into one change relative to the current head.

I have a backup of the commits before the squash in case we want to split it back up for some reason.

@deadprogram
Copy link

@danomagnum first of all great work on getting it this far. 💯

Just wondering a few things...

Regarding the naming of uatest. Now that there is both Python and Go server implementations having uatest and uatest2 is not that clear. perhaps those would be better put into subpackages such as uatest/python and uatest/go or maybe renamed python-uatest and go-uatest or ?

About the tests in uatest/method_test.go I do not see any equivalent uatest2/method_test.go. Is this because that functionality in the server is not yet complete, or it just needs someone to add more test coverage?

Thank you for the work on this PR, looking forward to hopefully getting it merged soon.

@danomagnum
Copy link
Author

Good idea on uatest/python and uatest/go. I'll make that change when I get a chance.

You are correct that there is no equivalent test for methods because that is not implemented in the new server yet. There are a few other tests that don't have equivalents yet in the new server either (like "registered read") and those tests are t.Skip()ped. The updated Readme.md file lists what services are implemented and what aren't.

@deadprogram
Copy link

Thanks for the explanations @danomagnum

@magiconair any chance you could take a peek at this admittedly massive PR? Some of us kinda need it 😸

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants