Discussion on subscriptions

[magiconair]

I am starting to work with subscriptions and would like to resurface @kung-foo 's suggestions in #207 (comment). I think they seem like a good improvement/addition to the current approach.

Also, I'm having trouble shutting down a client with a subscription properly. It looks like it always times out when the context gets cancelled which is not what I'd expect. But I need to dig a bit deeper. Are your subscription clients shutting down quickly?

[magiconair]

Also referring to a post #239 (comment) from @jkissinger. Good article from Prosys about timeouts. https://www.prosysopc.com/blog/opc-ua-sessions-subscriptions-and-timeouts/

[kung-foo]

What are your thoughts on how closely the golang API should follow the OPC API as defined by the network protocol?

My general approach is that I like having an opinionated, high level API that is designed from the perspective of the SDK user and follows the "zen" of the language. If the "opinion" is wrong, then I (as the user) can fall back to the lower level API that closely (or exactly) matches the network protocol.

An example is MonitoringParameters.ClientHandle. It doesn't seem like that ever needs to be exposed. From a user's perspective, it is just an implementation detail that they don't care about. Much in the same way that pagination APIs usually have some concept of a continuation token.

Elasticsearch's Python SDK has a package of "helpers" that hide some implementation details. Another example is the AWS CLI. It has aws s3 ... which has all the high level functions you expect (cp, rm, ls) and aws s3api ... which has functions which map directly to RESTful calls (get-object, head-object, put-bucket-replication, etc).

I don't know the network protocol well enough yet to know what to have a strong opinion on.

[magiconair]

I think that's a good general guideline. Close to the API the lower you get and more abstractions the higher you get.

We already have functions in the client that map directly to the API calls with (hopefully) sane defaults. If that does not work you can just copy the function and send the request yourself.

For one of our projects I've just written a NodeMonitor based on your ideas in the comment which abstracts a lot of the subscription handling away including the ClientHandle but this was just a 30 min work of moving existing code around. I'm all for something more generic and robust. Things like these are definitely useful but could/should live in a separate package.

If you can work with the existing api then add your code on top and make small tweaks where necessary. If you think the api or the implementation need a general refactor to make this work then I'd suggest two separate PRs.

type Notif struct {
	NodeID string
	Value  interface{}
}

func (m *M) Run(ctx context.Context) error {
	mon := &NodeMonitor{
		EndpointURL: "opc.tcp://localhost:4840",
		NodeIDs: []string{
			`ns=2;s="aaa"`,
			`ns=2;s="bbb"`,
			`ns=2;s="ccc"`,
		},
		Notifs: make(chan Notif, 1),
	}
	go mon.Run(ctx)

	for {
		select {
		case <-ctx.Done():
			return ctx.Err()

		case n := <-mon.Notifs:
			log.Printf("x: %#v", state)
		}
	}
}

[kung-foo]

Do you think the Browse API qualifies as high-level?

[magiconair]

There is Browse and BrowseNext in the View Service so these should exist as lower-level API functions. I think my more-browsing branch has the implementation for both. On top of that we can have some more generic browsers. Maybe with pluggable strategies and so forth.

[kung-foo]

Naming is going to be a challenge...

[magiconair]

Naming is always a challenge :)

[magiconair]

A separate browse package might solve that.

[kung-foo]

Naming and cache-invalidation :)

[kung-foo]

I've started moving a bit of the gist here: https://github.com/Intelecy/opcua/blob/node-monitor/monitor/monitor.go

Very much a WIP, but I wanted to ensure a discussion on the interface/features.

The callback-based use would look like:

monitor, _ := NewMonitor(client)

sub, _ := monitor.Subscribe(
	func(node *ua.NodeID, value *ua.DataValue) {
		fmt.Printf("node=%s value=%s\n", node, value.Value.Value())
	},
	`ns=2;s="aaa"`,
	`ns=2;s="bbb"`,
)

time.Sleep(time.Second * 5)

sub.Unsubscribe()

And the chan-based use:

monitor, _ := NewMonitor(client)

ch := make(chan *DataChangeMessage, 16)

sub, _ := monitor.ChanSubscribe(ch, `ns=2;s="aaa"`, `ns=2;s="bbb"`)
defer sub.Unsubscribe()

for {
	select {
	case msg := <-ch:
		fmt.Printf("node=%s value=%s\n", msg.NodeID, msg.Value.Value())
	case <-time.After(time.Second * 5):
		return
	}
}

[kung-foo]

@magiconair I can confirm the same delayed disconnect on an unsubscribe/close

[kung-foo]

something is definitely hanging for 10 seconds (about capture is from a 5 second subscription followed by sub.Unsubscribe(), client.Close())

[kung-foo]

addressed by #252