-
Notifications
You must be signed in to change notification settings - Fork 35
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
started work on adding discrete inputs and coils working. #56
base: master
Are you sure you want to change the base?
Conversation
winnme
commented
May 10, 2024
- added additional tables
- added relevant pymodbus code to _scan_value_range
- modified return statement to differentiate between analogue and digital result-objects
- added additional tables - added relevant pymodbus code to _scan_value_range - modified return statement to differentiate between analogue and digital result-objects
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general I would suggest to stick with more explicit nomenclature:
di
-> discrete_input
do
-> coils
Also i think method read_do
does not exist.
Please let me know if you are willing to improve it, otherwise I can submit another PR; since I am also interested in merging new feature.
@@ -226,11 +226,18 @@ def _scan_value_range(self, table, start, count): | |||
result = self._mb.read_input_registers(start, count, unit=self._unit) | |||
elif table == 'holding': | |||
result = self._mb.read_holding_registers(start, count, unit=self._unit) | |||
elif table == 'do': | |||
result = self._mb.read_do(start, count, unit=self._unit) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
read_do
do not exists, you would like to call read_coils
instead.
PyModBus docs
@@ -50,10 +50,10 @@ def __init__(self, | |||
self._port = port | |||
# This is a dict of sets. Each key represents one table of modbus registers. | |||
# At the moment it has 'input' and 'holding' | |||
self._tables = {'input': set(), 'holding': set()} | |||
self._tables = {'input': set(), 'holding': set(), 'do': set(), 'di': set()} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
improve table names
|
||
# This is a dicts of dicts. These hold the current values of the interesting registers | ||
self._values = {'input': {}, 'holding': {}} | ||
self._values = {'input': {}, 'holding': {}, 'do': {}, 'di': {}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general I would suggest to stick with more explicit nomenclature:
di
-> discrete_input
do
-> coils
Also i think method read_do
does not exist.
Please let me know if you are willing to improve it, otherwise I can submit another PR; since I am also interested in merging new feature.