Client Read File Transfer Method

[jadamroth]

This is the pull request regarding the issue I created an example in issue #371 for Opcua File Transfers for opcua clients

I couldn't immediately get the write file method working as shown in the example, so I only included the read file method for now.

I can try to work on writes in the coming weeks when I have more time, in which case I'll submit another pull request

[AndreasHeine]

What if the node has another identifier type
i | NUMERIC (UInteger)
s | STRING (String)
g | GUID (Guid)
b | OPAQUE (ByteString)
https://documentation.unified-automation.com/uasdkhp/1.0.0/html/_l2_ua_node_ids.html

[AndreasHeine]

cosmetics:
index is not None -> if index

[jadamroth]

I removed the last 2 parameters (index, and name_of_node) and will just have user pass in their node as the only parameter.

I think this is easier to understand and less likely for user to make mistakes which will throw exception

[AndreasHeine]

is there a way you can provide a python server example aswell

[jadamroth]

I will try to get to a working example of the server when I get some free time. I'll need to dive in to the source for server.

[AndreasHeine]

i am working in statemachines atm in opc us spec there is a FileTransferStateMachine discribed maybe we can join forces !?
https://reference.opcfoundation.org/v104/Core/ObjectTypes/FileTransferStateMachineType/
https://reference.opcfoundation.org/v104/Core/docs/Part5/C.4.6/

[jadamroth]

I apologize, but I'm not familiar with this spec and do not currently have enough time to commit to working on this right now.

If we run across any specs needed for our application that aren't including in this library, I'll commit to working on those.

I'll try to get the working server and write file methods in the next few days

[AndreasHeine]

there is no need for apologize! i am just writen the statemachine code. the file topic is quite interesting for machines like milling-machines where you could deploy g-code files via opcua :)

[oroulet]

I have an issue with that stuff. The Node and Client classes are already far too big and I am not really keen into adding new methods there for a functionality that is a bit suspicious. Why adding a file read to network protocol??? file read can be implemented as you want on sevrer side, using node value or custom methods.

we could have it but then we need to find some smart way to move the code away in another file and have at most one metod in Client class...Ideally none at all by using some external class ala

class MyFileReader:
   def __init__(self, my_file_node):
        xxx
   def read(self):
        # do all your shit here

[jadamroth]

You're correct in that it shouldn't have been in the base Client class. I am new to the library and don't fully understand the structure yet. I created a new client class ua_file.py, updated the example, and restored client class to its original.

Certainly people could write a custom method on the server, but I'm implementing based upon what the opcua specs state.

Please let me know of any other objections

[jadamroth]

One thing that I think should be reconsidered is if the user should have more control over the open, close methods.

For reads, you can use the same handle from open method several times before closing. However, I tried to make the code easier under the hood.

[oroulet]

looks much better. that way you can add as much functionality as you want.
if you want to make even more fancy you can implement the _aenter__, __aexit__stuff so people can write

async with Uafile(node) as f:
    f.read()

instead of you read_once method()

[oroulet]

The idea is to call self.open() here. As the stdlib open doors o

[jadamroth]

I was thinking about opening it in this method, but I ultimately decided not to because I wanted to make one class to handle all file method operations (read, write, erase, append). For example, it wouldn't work well if a user wanted to open, read a file, then write a new file because when the client opens the file on our opcua server, we need to specify which operation we're using.

I could create classes that are solely designated for each operation FileRead(), FileWrite(), FileErase() that inherits this UaFile class, and then it would make sense to open with this function and close on exit.

[jadamroth]

Check new commit for separating classes

[oroulet]

And self.exir() here

[oroulet]

That makes that method obsolete. It does not exist in the stdlib open api

[oroulet]

looks fine, but I do not really see why you splitted these in two classes. One was fine.. I cannot remember any file API that differentiate between a File and a FileRead object. That would aso reduce the number of liens in example. But if you have reasons for that I am fine

[jadamroth]

I personally don't like separating them into two classes. I was just showing it as an option to use open with aenter .

I like keeping one class. However, with one class, I don't think you should call open() on the aenter because you need to specify your file operation to the server on open(fileOperation) (which will be different for read, write, erase, and append)

[oroulet]

How do they do with the built-in open() we should try to copy that api so we do not surprise anyone
So you can either use a function like open() or pass (optionally?) the open type to class constructor

[jadamroth]

Here's an example of how the node-opcua public api handles their file operations, and how I'd recommend ours to similarly reflect - https://www.npmjs.com/package/node-opcua-file-transfer

[oroulet]

@jadamroth The node-opcua guys choose (or had to implement due to limited language) a much lower level ua api. So I do not think they are a good reference

[oroulet]

here is the standard (modern) python way to open a file

    with open('sample.txt', "r") as file_object:
        # read file content
        data = file_object.read()

I think we should do the same. something like

with asyncua.open(node, readtype) as uafile:
      data uafile.read()
      ```
      

[oroulet]

but your proposition (althoug with one class ) is fine too

   async with Client(url=url) as client:
        file_node = client.get_node("ns=2;s=NameOfNode")
        async with UaFile(file_node, asyncua, WriteOnly) as ua_file:
            # read file
            contents = await ua_file.read()
            print(contents)

[oroulet]

anyway that code looks good. I am wondering if we could remove code from Node and Client classes by writing such classes for other features. The entire server discovery is a good candidate. And history reading in Node

[jadamroth]

I would certainly think that you can separate classes for most features that have no required attributes internal to Node and Client classes

I still need to update the UaFile branch to revert back to one class. I like the idea of structuring the UaFile as pythonic. I'll commit changes tonight

[jadamroth]

Updated to make it pythonic and one class.

I think it looks good for reads, but it might have to change when we include writes. Currently as is, if a user wants to handle different consecutive file operations, then they'll have to one of the following

1. create a separate UaFile class for writes. Something like this

async with UaFile(file_node, 'r') as ua_file:
    contents = await ua_file.read()  # read file
    print(contents)
async with UaFile(file_node, 'w') as ua_file:
    await ua_file.write('new file contents')  # write file

2. handle the file logic internally themselves. Something like

async with UaFile(file_node, 'r') as ua_file:
    contents = await ua_file.read()  # read file
    await ua_file.close()
    await ua_file.open('w')
    await ua_file.write('new file contents')

[oroulet]

I think that limitation is completely fine. Do you often both read and open from the same file?
I am merging that one so you can always improve in another PR if you want