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

Fix for numvalues parameter in Node.read_raw_history() #1351

Conversation

KallistoD
Copy link
Contributor

The description of the function FreeOpcUa / opcua-asyncio / asyncua / common / node.py / read_raw_history() declares that non-zero numvalues parameter should truncate the amount of historical data to receive from a server:

Read raw history of a node
result code from server is checked and an exception is raised in case of error
If numvalues is > 0 and number of events in period is > numvalues
then result will be truncated

Actually, numvalues parameter is practically useless and has no effect on the function behavior. The function always reads and returns full set of historical information as defined by the other parameters regardless of numvalues value.
This pull request eliminates the inconsistence between the function declaration and actual behavior. If the pull request applied, the function asks for the limited amount of historical data from a server (as requested by non-zero numvalues parameter). It makes it possible to effectively control RAM usage of client side without guessing right period of time for historical data to request.
Please, note:

  • This pull request may cause ContinuationPoint to leak on server side. But server stability should not rely on client behavior anyway. The client may crash at anytime, and ContinuationPoint may leak as well. So, possible ContinuationPoint leaks must be managed on server side (possible, through expiration) anyway.
  • This pull request does not violate the standard:

When a continuationPoint is returned, a Client wanting the next numValuesPerNode values shall call ReadRaw again with the continuationPoint set.

So, the client is free to decide if it wants to request the rest of historical data from ContinuationPoint or not.
The pull request was created as a continuation of this issue. Tested against 3rd party OPC UA server.

@schroeder-
Copy link
Contributor

schroeder- commented Jun 23, 2023

I don't like this change, because this is a bug on the third party server. Also if you surpply num = 0 then it is unlimited values. So this change breaks this behaviour as well.

@schroeder-
Copy link
Contributor

Maybe we should make this change optional, via a boolean parameter?

@KallistoD
Copy link
Contributor Author

Thank you for your attention. My bad. I applied the wrong solution from the issue. Now, I fixed it.
There is absolutely no bug in 3rd party OPC UA server. It respects numvalues value according to the standard. Behavior checked with WireShark. If the pull request applied:

  • read_raw_history() call with numvalues=0 executes self.history_read() once. It returns whole set of historical data as requested by starttime and endtime parameters
  • read_raw_history() call with numvalues>0 executes self.history_read() once as well. But it returns the set of historical data limited by numvalues historical values

I feel no need in additional boolean parameter. If you need to limit the returned array size, you set numvalues>0. If you don't need it, you set numvalues=0. This should work with any OPC UA server, which respects standard.

@schroeder-
Copy link
Contributor

You are right and with this change this should work, as expected.

@oroulet oroulet merged commit a2f2636 into FreeOpcUa:master Jun 23, 2023
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.

3 participants