kevinherron / ua-client-sdk

OPC-UA Client SDK for Java
27 stars 16 forks source link

OpcUaMonitoredItem does not cache the last DataValue #8

Closed tosuns closed 8 years ago

tosuns commented 8 years ago

It seems like the above mentioned instance does not cache the last retrieved value. Therefore if a value consumer is added after the value was published the consumer will not get the value until a data value change notification occurrs again. In case of data points whose values do not change very frequently, the user is forced to actually invoke a read operation.

I noticed this behaviour while modifying the "testSubscribe" test case. Instead of subscribing only one monitored item I tried to subscribe two items at once. The test always failed because of a time out exception. The value of the first item will be printed out like expected, but the second causes the time out exception, because the consumer was registered and its get-method gets called after the notification occurred, which is to late.

` public void testSubscribe() throws Exception { LOG.info("testSubscribe()");

    // create a subscription and a monitored item
    UaSubscription subscription = opcUaClient.getSubscriptionManager()
            .createSubscription(1000.0)
            .get();

    AtomicInteger clienthandles = new AtomicInteger(1);

    List<MonitoredItemCreateRequest> requests = Arrays.asList(
            new NodeId(4,
                       "<datapoint.one>"),
            new NodeId(4,
                       "<datapoint.two>"))
            .stream()
            .map(nodeId -> new ReadValueId(
                    nodeId,
                    AttributeId.Value.uid(),
                    null,
                    QualifiedName.NULL_VALUE))
            .map((readValueId) -> {
                final int clientHandle = clienthandles.getAndIncrement();
                MonitoringParameters parameters = new MonitoringParameters(
                        uint(clientHandle),
                        1000.0,
                        null,
                        uint(10),
                        true);
                MonitoredItemCreateRequest request = new MonitoredItemCreateRequest(
                        readValueId,
                        MonitoringMode.Reporting,
                        parameters
                );
                return request;
            })
            .collect(Collectors.toList());

    List<UaMonitoredItem> items = subscription
            .createMonitoredItems(TimestampsToReturn.Both,
                                  Collections.unmodifiableList(requests)).get();

    // do something with the value updates
    items.forEach((item) -> {

        try {
            LOG.log(Level.INFO,
                    "item [clientHandle: {0}, monitoreditemId: {1}",
                    new Object[]{item.getClientHandle(),
                                 item.getMonitoredItemId()});
            CompletableFuture<DataValue> f = new CompletableFuture<>();

            item.setValueConsumer(f::complete);
            DataValue datavalue = f.get(15, TimeUnit.SECONDS);

            assertNotNull(datavalue);
            assertNotNull(datavalue.getValue());
            LOG.log(Level.INFO, "subscribtion Result: {0}",
                    datavalue.getValue().getValue());
        } catch (InterruptedException |
                 TimeoutException |
                 ExecutionException ex) {
            LOG.log(Level.SEVERE, null, ex);
            fail(ex.getMessage());
        }

    });

}`
kevinherron commented 8 years ago

You're right, it does not cache the last value... and I'm not sure it should.

When you create items, you should create them in MonitoringMode.Sampling, do any bookkeeping you need, including setting up the value consumers, and then switch the MonitoringMode to reporting.

If you don't use this technique there will always be a race condition. Even if I cache one value, it would then open up the possibility that two value changes arrive before you've set a consumer, or three etc..., and you only get notified of one without any indication there were others...

I will think about this a little more though...

kevinherron commented 8 years ago

Btw, project has moved: https://github.com/eclipse/milo

tosuns commented 8 years ago

How can I switch the MonitoringMode ? I don't see any set method in the UaMonitoredItem class and it seems like the MonitoredItemModifyRequest class does not have any option to change the MonitoringMode.

kevinherron commented 8 years ago

It's on the subscription itself - UaSubscription#setMonitoringMode

kevinherron commented 8 years ago

A discussion about this issue is now happening here: https://github.com/eclipse/milo/issues/9

kevinherron commented 8 years ago

@tosuns

A proposed fix to this is in https://github.com/eclipse/milo/pull/13

If you could give some feedback on the modified API that would be nice.