Add additional callbacks for SharedPV handlers #155

Monarda · 2024-08-25T17:08:59Z

This PR adds three new callback functions for use by SharedPV (p4p.server.raw) handlers. They are:

open() - called when a SharedPV is opened
post() - called each time a post operation is performed on a SharedPV
close() - called when a SharedPV is closed.

The associated changes to the p4p.server.raw.Handler class, additional new decorators, and unit tests (src/p4p/test/test_sharedpv.py) are included. The changes to src/p4p/server/raw.py are relatively minor but the examples and unit tests make this look like a much bigger PR! There was some initial discussion of the idea at #150.

Three examples that make use of the new callback functions are also included:

example/auditor.py - demonstrates using open() and close() callbacks along with the existing put() callback to record when an auditing PV that records who's made changes to other PVs is or isn't available. Probably redundant compared to the more complex example/persist.py.
example/ntscalar_control.py - implements the Control field logic for an NTScalar. This illustrates how put(), post(), and open() lead to a natural separation of concerns. Logic that does not depend on the previous state of the PV may be implemented in the handler open(), e.g. altering the value dependent on control.limitHigh and control.limitLow. Logic that depends on the current state of the PV and the proposed changes may be implemented in the post(), e.g. applying control.minStep. The put() may then be solely concerned with authorisation.
example/persist.py - demonstrates using an SQLite3 database to automatically save and restore the values of PVs. Makes use of the new handler_open_ and handler_post_ prefix arguments to make configuration convenient and to transmit information from a put() to a post() respectively. Makes full use of the new open() callback to automatically restore the value and timeStamp information of a PV and post() to automatically record this information each time it changes.

I believe the example/persist.py file probably makes the strongest case for the kind of new features that these callbacks allow or make easier.

Breaking Changes

I believe these changes may be considered largely backwards compatible. They will only cause compatibility issues:

if an existing handler implements open(), close(), or post() functions.
if a PV includes top-level fields names beginning with handler_open_ or handler_post_.

Monarda · 2024-08-25T17:13:38Z

src/p4p/server/raw.py


+        # Intercept all arguments that start with 'handler_open_' and remove them from
+        # the arguments that go to the wrap and send them instead to the handler.open()
+        post_kws = {x: kws.pop(x) for x in [y for y in kws if y.startswith("handler_open_")]}


This, and the equivalent line 212 for post(), is probably one of the more important changes. It means that any arguments prefixed handler_open_ will be passed to the handler (if it exists) and not to _wrap(). An earlier version of the code added a single new argument that enabled or disabled the handler altogether. But while a smaller change it was also much crude.

Monarda · 2024-08-25T17:16:54Z

src/p4p/server/raw.py

+
    @property
-    def onFirstConnect(self):
+    def on_open(self):


This is a leftover from an earlier version in which I thought I'd needed to supply decorator names that wouldn't clash with the function names. It does make the names PEP8 compliant so I've left the change in for consideration.

Monarda · 2024-08-25T17:17:24Z

src/p4p/server/raw.py

+    # Aliases for decorators to maintain consistent new style
+    # Required because post is already used and on_post seemed the best
+    # alternative.
+    put = on_put


Compatibility for previous decorator names.

Monarda · 2024-08-25T17:25:41Z

src/p4p/server/raw.py

+        except AttributeError:
+            pass
+
        _SharedPV.post(self, V)


Implementing the persist.py example has made me think there may be a case to split the post() callback into two. The one implemented in this PR that changes values before they reach this _SharedPV.post(self, V) and one which is called afterwards. The after_post() callback would then only be supplied the updated state of the PV and would not be triggered by unsuccessful posts.

Monarda · 2024-10-17T10:19:06Z

It was suggested at a meeting on 9 Oct that I tag @coretl and @AlexanderWells-diamond for attention or review. Apologies if this comes as a surprise!

AlexanderWells-diamond

The code changes look fine to me, barring a few things that are little more than nitpicks, although as discussed offline I'm no expert on p4p's design philosophy so can't comment on whether the changes are fundamentally ok. I've tagged @mdavidsaver for further review.

I think the server.rst file needs updating to properly include the newly defined methods for SharedPV; looks like you need to define automethod for each new method.

AlexanderWells-diamond · 2025-03-06T09:19:53Z

example/ntscalar_control.py

+
+        # Because we have not set use_handler_post=False in the post this
+        # will automatically trigger evaluation of the post rule and thus
+        # the application of


Looks like the end of this comment is missing?

AlexanderWells-diamond · 2025-03-06T09:22:28Z

example/persist.py

+class PersistHandler(Handler):
+    """
+    A handler that will allow simple persistence of values and timestamps
+    across retarts. It requires a post handler in order to persist values


Suggested change

across retarts. It requires a post handler in order to persist values

across restarts. It requires a post handler in order to persist values

AlexanderWells-diamond · 2025-03-06T09:29:43Z

example/persist.py

+
+# Create an SQLite dayabase to function as our persistence store
+conn = sqlite3.connect("persist_pvs.db", check_same_thread=False)
+# conn.execute("DROP TABLE IF EXISTS pvs")


I think this comment is unnecessary, looks like you do the same thing in the next line?

AlexanderWells-diamond · 2025-03-06T09:38:37Z

example/auditor.py

+class Audited(Handler):
+    """Forward information about Put operations to the auditing PV"""
+
+    def __init__(self, pv: SharedPV):


I note that this type hint doesn't work in Python2; I don't know how important it is for all the examples to still support Python2, but the overall library still does.

AlexanderWells-diamond · 2025-03-06T09:43:24Z

src/p4p/test/test_sharedpv.py

+    def tearDown(self):
+        self.server.stop()
+        _defaultWorkQueue.sync()
+        #self.pv._handler._pv = None


Suggested change

#self.pv._handler._pv = None

AlexanderWells-diamond · 2025-03-06T09:44:42Z

src/p4p/test/test_sharedpv.py

+            pass
+
+    def setUp(self):
+        # gc.set_debug(gc.DEBUG_LEAK)


Another comment that can probably be removed

AlexanderWells-diamond · 2025-03-06T09:44:55Z

src/p4p/test/test_sharedpv.py

+    def tearDown(self):
+        self.server.stop()
+        _defaultWorkQueue.sync()
+        #self.pv._handler._pv = None


Suggested change

#self.pv._handler._pv = None

AlexanderWells-diamond · 2025-03-06T09:50:09Z

src/p4p/server/raw.py

+        Called each time an Open operation is performed on this Channel
+
+        :param value:  A Value, or appropriate object (see nt= and wrap= of the constructor).
+        """


Should this (and the post and close methods) call op.done(error='Not supported')), as other methods do?
At the very least for consistency this one should have a pass, to be the same as the other methods that don't call that method.

Monarda added 10 commits July 18, 2024 17:14

Begun adding RulesHandler

f1fd91d

Implementation and example for nthandlers

4ccfb7c

Merge branch 'epics-base:master' into master

df902e4

Merge branch 'epics-base:master' into master

6c45ed9

Add new handler functions with examples and tests

406ede6

Add another example showing how a handler.post can allow persistence

fecb939

Duplicate pvs do work

6b67839

Minor formatting changes

c2a7f98

Change to how handler arguments are treated

9aa3a0a

Most of the restore work can be done in the handler open()

706602f

Monarda commented Aug 25, 2024

View reviewed changes

Monarda added 2 commits August 26, 2024 08:29

Improvements to persist example

2f4a32c

Move control field initialisation to open initial. Improve comments

49c7e90

Merge remote-tracking branch 'upstream/master' into post_handler

fdf3228

AlexanderWells-diamond requested changes Mar 6, 2025

View reviewed changes

AlexanderWells-diamond requested a review from mdavidsaver March 6, 2025 09:53

Merge branch 'master' into post_handler

9a7cc63

Monarda mentioned this pull request May 27, 2025

New methods for Handler and SharedPV #172

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add additional callbacks for SharedPV handlers #155

Add additional callbacks for SharedPV handlers #155

Uh oh!

Monarda commented Aug 25, 2024

Uh oh!

Monarda Aug 25, 2024

Uh oh!

Monarda Aug 25, 2024

Uh oh!

Monarda Aug 25, 2024

Uh oh!

Monarda Aug 25, 2024

Uh oh!

Monarda commented Oct 17, 2024

Uh oh!

AlexanderWells-diamond left a comment •

edited

Loading

Uh oh!

AlexanderWells-diamond Mar 6, 2025

Uh oh!

AlexanderWells-diamond Mar 6, 2025

Uh oh!

AlexanderWells-diamond Mar 6, 2025

Uh oh!

AlexanderWells-diamond Mar 6, 2025

Uh oh!

AlexanderWells-diamond Mar 6, 2025

Uh oh!

AlexanderWells-diamond Mar 6, 2025

Uh oh!

AlexanderWells-diamond Mar 6, 2025

Uh oh!

AlexanderWells-diamond Mar 6, 2025

Uh oh!

Uh oh!

	across retarts. It requires a post handler in order to persist values
	across restarts. It requires a post handler in order to persist values

Add additional callbacks for SharedPV handlers #155

Are you sure you want to change the base?

Add additional callbacks for SharedPV handlers #155

Uh oh!

Conversation

Monarda commented Aug 25, 2024

Breaking Changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Monarda commented Oct 17, 2024

Uh oh!

AlexanderWells-diamond left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

AlexanderWells-diamond left a comment •

edited

Loading