pymodbus-dev / pymodbus

A full modbus protocol written in python
Other
2.16k stars 889 forks source link

datastore: add async_setValues/getValues methods #2143

Closed ilkka-ollakka closed 2 months ago

ilkka-ollakka commented 3 months ago

By default those just call self.set/getValues but can be used to implement more async datastore access in modbus server.

Split from https://github.com/pymodbus-dev/pymodbus/pull/2127

ilkka-ollakka commented 3 months ago

macos CI seems to fail from not able to create working venv from cache.

janiversen commented 3 months ago

I had the same problem earlier today, but it went away....have you rebased to newest d3v.

ilkka-ollakka commented 3 months ago

I had the same problem earlier today, but it went away....have you rebased to newest d3v.

Yes, it should be up to date, and the PR itself doesn't show it being behind of dev branch

janiversen commented 3 months ago

Actually if you look at https://github.com/pymodbus-dev/pymodbus/blob/c4c14cab294e166b1efb1b7733b14fb3dcb019a3/pymodbus/bit_write_message.py#L90

you see the right way to use the datastore:

However that collided with remote, which pr definition cannot implement a validate(), so a number of work arounds was invented....but they are going to be removed when refactoring and most likely validate() will also be removed, allowing get/setValues to do a raise instead.

ilkka-ollakka commented 3 months ago

Actually if you look at

https://github.com/pymodbus-dev/pymodbus/blob/c4c14cab294e166b1efb1b7733b14fb3dcb019a3/pymodbus/bit_write_message.py#L90

you see the right way to use the datastore:

* validate() first

* then call get/setValues

However that collided with remote, which pr definition cannot implement a validate(), so a number of work arounds was invented....but they are going to be removed when refactoring and most likely validate() will also be removed, allowing get/setValues to do a raise instead.

That codeblock same assumes that it can get ExceptionResponse back (https://github.com/pymodbus-dev/pymodbus/blob/c4c14cab294e166b1efb1b7733b14fb3dcb019a3/pymodbus/bit_write_message.py#L94)

If things are gonna be removed when refactoring, then typehints should be changed accordingly, but I think currently typehints should indicate current flow?

janiversen commented 3 months ago

Type hint should not reflect wrong code.

janiversen commented 3 months ago

Also you have not changed bit_write_message, and I do not think that mypy complains about that.

Since remote inherits from ModsbuBaseSlaveContent, you do not need to make changes in that file. However if you choose to make changes, you should remove the problem.

janiversen commented 3 months ago

Instead of having this, in reality stupid, conversation about adding faulty typing, I went ahead and changed the code.

The effect for this PR is that there are no need to change simulator.py

janiversen commented 2 months ago

How are you doing with this one, seems you still have confusion about ExceptionResponse.

janiversen commented 2 months ago

It set at "draft" so I assume you are still working.

janiversen commented 2 months ago

hmmm seems I lost a message edit.

I went ahead and polished the PR, so it follows the rest of the code.

Superseded by #2165 (merged, with @ilkka-ollakka as committer)