Skip to content

Conversation

hmaerki
Copy link

@hmaerki hmaerki commented Dec 24, 2023

Bug

  • If on_get_cb is defined, the second call to self._create_response() overwrites the value returned by on_get_cb.
  • In other words: The value set by on_get_cb was swallowed.

Bugfix

  • Simplified code
  • Call self._create_response() only once.

@GimmickNG
Copy link

I think I did something like this earlier as well. The problem is that caching it this way means that on_get_cb also has no chance to update the vals the way it's implemented now -- so the tests where it calls set_ireg in the on_get_cb might fail, since it expects the register values to update after it is called - but it would end up returning the old value instead of the new one.

@hmaerki
Copy link
Author

hmaerki commented Jan 24, 2024

I think I did something like this earlier as well. The problem is that caching it this way means that on_get_cb also has no chance to update the vals the way it's implemented now -- so the tests where it calls set_ireg in the on_get_cb might fail, since it expects the register values to update after it is called - but it would end up returning the old value instead of the new one.

Hi @GimmickNG
Thanks for your feedback. I could not follow your explanation as I do not know the library sufficiently.
I was struggling due to lack of documentation or lack of examples (or I did just missed important parts).

Reading the documentation/examples, I failed to understand how to implement these concepts:

  • Set synchron (set a value in a 'on_set_cb').
  • Set asynchron (No 'on_set_cb'. Read the value from 'micropython-modbus' when it is needed)
  • Get synchron (In 'on_get_cb', get the the value, lets say from a sensor, and update it) (This does not work - see this PR)
  • Get asynchron: (No 'on_set_cb'. Ppoll for the sensor value and update the value stored in 'micropython-modbus'.)

Eventually, the developer has to decide between these two concepts

  • The state(value) should be maintained by 'micropython-modbus': Then use Set/Get asynchron.
  • The state (value) is maintained outside of 'micropython-modbus', for example in the sensor/actor. Then use Set/Get synchron.

It might be helpful to have the documentation and examples structured this way.

If you like, I could spend some hours in creating

  • Proposal for update to the documentation
  • Proposal for 4 examples as described above

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants