gijzelaerr / python-snap7

A Python wrapper for the snap7 PLC communication library
http://python-snap7.readthedocs.org/
MIT License
643 stars 246 forks source link

In the DB class, there is no canonical way to read or write the entire DB #380

Closed vmsh0 closed 1 year ago

vmsh0 commented 2 years ago

When using the DB class, you provide initial data, and then you're able to provide a new underying bytearray to refresh the data.

While DB_Row allows to write to and read from the PLC to move data in and out of this bytearray (each row on its own slice), there is no way to write the entire DB (i.e. all the rows) at once. This is especially detrimental to our use case because we have a large number of rows, and separating the reads/writes is degrading our application performance.

As far as I can make out, when row_offset is 0 there is no real problem with reading and writing all the rows at once. As such, I propose the implementation of the DB.read(client) and DB.write(client) methods.

Again, I'm available & happy to submit PRs after any relevant feedback on the existing PRs and issues.

spreeker commented 2 years ago

This is implemented. Rather Dangerous Operation and you better be sure the mapping is correct :).

https://www.youtube.com/watch?v=G-Gj_r2BQBk

def write_data_db(dbnumber, all_data, size):

vmsh0 commented 2 years ago

Hi,

This is implemented. Rather Dangerous Operation and you better be sure the mapping is correct :).

I absolutely agree it's a dangerous operation, just as dangerous as writing any value at all on a live system. This is why we have the DB class: you define your mapping and then the class sticks to it instead of you having to do that. It's an abstract model. So I argue that no, it is not implemented: currently, to perform the operation, you need to leave the abstract model and manipulate a byte buffer and some loosely-linked metadata.

Other than the purely pragmatic argument that it makes the model more expressive, it's also a matter of coherence: why is it possible to write back single rows, but not all of them? (in some cases that's literally not possible, when using the DB abstraction, without working with non-public parts of the API or - again - manually manipulating the byte buffer and loosely-linked metadata; see #379)

https://www.youtube.com/watch?v=G-Gj_r2BQBk

Sorry, I'm not sure what I'm supposed to be seeing in this video!

def write_data_db(dbnumber, all_data, size):

vmsh0 commented 2 years ago

Just for sake of reference, I have now documented in #379 that there is a way after all to iterate on keys, with some caveats.