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 iterate on all rows when id_field is not None #379

Closed vmsh0 closed 1 year ago

vmsh0 commented 2 years ago

When id_field is set to something that is not None, there is no canonical way or public API to iterate on all rows or even to obtain a list of the available keys.

The only way to do that currently is by accessing the index field, which however is not documented and supposedly can't be presumed to be part of the public API. (Furthermore, even if it was, directly accessing attributes to get data is usually bad API design, although the `@property' decorator somewhat mitigates the associated problems.)

What I propose is one or more of - with my personal preference being the first two items of the list:

Like for the other issues, I'm available to submit PRs. However, since I already have two PRs pending, I'm going to wait for your feedback on those and this issue before opening more.

vmsh0 commented 2 years ago

I just noticed the class has an __iter__ method without any documented semantics. It went right over my head, probably because it's not documented. It iterates over the (key, value) pairs, which is better than nothing! However, this method, in other mappable types such as dict, iterates over the keys, so it's a bit inconsistent.

Since I'm guessing it's too late to change that interface, I'm going to put forward a PR soon with the following changes:

spreeker commented 2 years ago

Hai that might me a good idea. At the time I was in a hurry building all of this.

I do remember there where some random inconsistency between DB objects with different starting offsets. So I let go of solving that In the abstract class. I believe you can just send the entire Arraybuffer of the abstract dict class into a single Snap7 client function to replace an entire DB.

I've implemented code that every few seconds would read the entire DB and did updates to some values. The update would take 5ms. On a single value. The reading of the entire DB could take up almost a whole second and was a rather hefty operation (for a plc). 500plus rows. Doing a key by key update is slowish. / Not efficient.

So I could give a guarantee the software would respond < 2 seconds. On changes in PLC of external.input.

It is as fast as possible.

My fist Implantation involved some OPC middle ware and took seconds and was unrealizable when I was reading just a few variables.