python-snap7 icon indicating copy to clipboard operation
python-snap7 copied to clipboard

In the DB class, there is no canonical way to iterate on all rows when id_field is not None

Open vmsh0 opened this issue 3 years ago • 2 comments

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:

  • making DB iterable like dictionaries
  • having a DB.items() method like that of dictionaries
  • having a DB.export() method like that of DB_Row

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 avatar Aug 01 '22 16:08 vmsh0

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:

  • keys(), items() and __contains__() methods, with dict semantics
  • export() method, flattening all available data to a plain dict
  • class- and method-level documentation detailing these semantics

vmsh0 avatar Aug 22 '22 10:08 vmsh0

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.

spreeker avatar Aug 22 '22 14:08 spreeker