search

stlehmann / pyads

Python wrapper for TwinCAT ADS
MIT License
264 stars 96 forks source link

split read and write ads sum into parts of MAX_SUB_COMMANDS #198

Closed chrisbeardy closed 3 years ago

chrisbeardy commented 3 years ago

This PR aims to split the read and write ads to that if the number of sub commands required are more than 500 (Beckhoff recommended). Just putting in the draft early to check if the functionality is desired. This does conflict with PR #187 but it should be compatible if this PR or #187 gets reworked after one is merged first.

I have used an optional override, in the event the user sees it being critical that they read or write that list in one ADS call (e.g. trying to trigger >500 things at the same time).

If this functionality and implementation seem OK, I can finish off the PR by adding tests and docs and rebasing on top of the last few commits. I can also remove the affected areas in ads.py that got modified by black.

stlehmann commented 3 years ago

@chrisbeardy That's a great enhancement to read_list_by_name and write_list_by_name. Feel free to finish the PR. Just a suggestion from my side: Would it make sense to replace ignore_max_ads_limit by max_ads_subcommands = 500 to make the maximum number of subcommands configurable?

chrisbeardy commented 3 years ago

This makes sense, I shall do:

def read_by_list(..., max_ads_subcommands = MAX_ADS_SUB_COMMANDS): ...

this way its included as a constant should Beckhoff ever change the recommendation, but then it can also be overridden with a specific value if desired.

stlehmann commented 3 years ago

To improve clearness of the PR removing the style-changes would be great. If you use black you might want use --line-length=120 to avoid all the wrapping.