micropython-modbus
micropython-modbus copied to clipboard
Bugfix: 'on_get_cb': return value was swallowed
Bug
- If
on_get_cbis defined, the second call toself._create_response()overwrites the value returned byon_get_cb. - In other words: The value set by
on_get_cbwas swallowed.
Bugfix
- Simplified code
- Call
self._create_response()only once.
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.
I think I did something like this earlier as well. The problem is that caching it this way means that
on_get_cbalso has no chance to update thevalsthe way it's implemented now -- so the tests where it callsset_iregin theon_get_cbmight 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