Add event handling for TCPIP instruments, allowing SRQ interrupts. Closes #544 by SimonMerrett · Pull Request #577 · pyvisa/pyvisa-py

SimonMerrett · 2026-04-28T22:06:36Z

My second attempt at providing the event handling I would like to see for instruments capable of sending SRQ service requests. @MatthieuDartiailh helpfully advised on the approach and reviewed my previous PR but I did not and do not have enough Python skill to take that review into action. I have done my best in this PR to provide the feature but actually was able to test it on hardware which took 5 measurements at 1Hz and returned SRQ. I have attached Wireshark pcap showing the communication of a successful test (measurement results are not retrieved during the test).
pyvisa-py_srq.zip

I used an agentic workflow in opencode to plan, code, test, review. Some of the wider-reaching changes were bound up in reviewer recommendations for robustness, such as compatibility "tweaks" in attributes.py. Everything is up for editing but with this PR I am happy that it should pass some degree of testing (the agents were testing against the built-in tests and added their own for the event aspects). I am also more confident because I was able to take my script for my instrument and test the functionality was there, that measurements were taken, SRQ was sent properly as far as I can tell (not a hidden python thread polling the instrument status registers, like one of the earlier agents tried to sneak in).

Please go easy on the feedback and feel free to edit! If you agree that this is closer to the mark than #547 I will close that PR.

Closes Event handlers - is it possible for TCPIP INSTR resource? #544 (maybe)
Executed black . && isort -c . && flake8 with no errors
The change is fully covered by automated unit tests (I belive so)
Documented in docs/ as appropriate ( index.rst has been updated)
Added an entry to the CHANGES file

…cluding tests

for more information, see https://pre-commit.ci

MatthieuDartiailh · 2026-05-04T23:53:16Z

Thanks for the PR given the size of the changes ot will take me time to do a thorough review. Do not hesitate to ping me if I go silent for too long.

xyphro · 2026-05-05T18:24:35Z

BTW: I really like that SRQs support gets added. Was missing it!

codecov · 2026-05-06T08:13:54Z

Codecov Report

❌ Patch coverage is 78.52273% with 189 lines in your changes missing coverage. Please review.
✅ Project coverage is 39.00%. Comparing base (7c458f2) to head (bdc446a).

Files with missing lines	Patch %	Lines
pyvisa_py/protocols/vxi11.py	17.64%	126 Missing ⚠️
pyvisa_py/tcpip.py	49.33%	32 Missing and 6 partials ⚠️
pyvisa_py/highlevel.py	77.41%	13 Missing and 1 partial ⚠️
pyvisa_py/events.py	95.91%	2 Missing and 4 partials ⚠️
pyvisa_py/sessions.py	81.25%	3 Missing ⚠️
pyvisa_py/protocols/rpc.py	0.00%	1 Missing ⚠️
pyvisa_py/testsuite/test_events.py	99.76%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #577      +/-   ##
==========================================
+ Coverage   30.72%   39.00%   +8.27%     
==========================================
  Files          25       27       +2     
  Lines        4182     5053     +871     
  Branches      459      520      +61     
==========================================
+ Hits         1285     1971     +686     
- Misses       2882     3055     +173     
- Partials       15       27      +12

Flag	Coverage Δ
unittests	`39.00% <78.52%> (+8.27%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

MatthieuDartiailh

This is a first pass. I think one of the main thing that could be improved is the tracking of the type of events for which we handle events. Using an int flag internally could simplify many things I think.

MatthieuDartiailh · 2026-04-29T13:38:20Z

+# vicp may not exist in older pyvisa versions
+_vicp = getattr(constants.InterfaceType, "vicp", None)
+


We enforce a recent enough version of pyvisa (or we should I will have to double check). So this is not needed.

This should be removed now.

MatthieuDartiailh · 2026-04-29T13:44:34Z

-        (constants.InterfaceType.vicp, "INSTR"),
+        *([(_vicp, "INSTR")] if _vicp is not None else []),


MatthieuDartiailh · 2026-05-06T08:14:20Z

+from .common import LOGGER
+
+
+@dataclass


This should be marked as frozen since the intent is for instances to be immutable. Additionally we could have slot=True

Done. Both frozen and slots.

MatthieuDartiailh · 2026-05-06T08:16:55Z

+                self._deque.extend(kept)
+
+
+HandlerCallback = Callable[[Any, constants.EventType, int, Any], None]


A docstring explaining the meaning of each argument would be welcomed.

Added docstring. Please confirm if formatting is acceptable or I need to look at this.

MatthieuDartiailh · 2026-05-06T08:19:49Z

+    def __init__(self) -> None:
+        self._lock = threading.RLock()
+        # event_type -> list of (handler, user_handle)
+        self._handlers: dict[


Using a defaultdict here would simplify the code

defaultdict implemented

MatthieuDartiailh · 2026-05-06T08:23:51Z

+    def __init__(self) -> None:
+        # {event_type: {mechanism_int}}
+        self._lock = threading.RLock()
+        self.enabled: dict[constants.EventType, set[int]] = {}


Using a Flag here rather than unnamed integer values in a set would be much more readable.

flags implemented. Following pyvisa flag values.

MatthieuDartiailh · 2026-05-06T08:27:47Z


 """

+from __future__ import annotations


If this does not bring anything I would rather not add it.

MatthieuDartiailh · 2026-05-06T08:28:34Z

+            sess = self.sessions[session]
+        except KeyError:
+            return self.handle_return_value(session, StatusCode.error_invalid_object)
+        if event_type not in sess._supported_event_types:


Some blank lines between logic block would improve the readability.

Blank lines added.

MatthieuDartiailh · 2026-05-06T08:41:32Z

+        if queue_enabled:
+            self._event_state.queue.put(ctx)
+        if handler_enabled:
+            session_handle = getattr(self, "_session_handle", self)


This looks a bit suspicious. What is the rational.

I do not know but I asked for a review and this was the output

1. Design intent: why the getattr fallback exists _session_handle is assigned only once in the entire codebase — in highlevel.py:201 immediately after an instrument session is created: visa_session = self._register(sess) sess._session_handle = visa_session The getattr(self, "_session_handle", self) on sessions.py:387 is defensive coding: the author knew _session_handle is not part of the Session base class __init__ and added a fallback in case it was missing. However, the fallback self (the Python Session object) is never the correct value to pass to a VISA event handler. 2. The Resource Manager session The RM session is created in highlevel.py:524 by registering the PyVisaLibrary object itself. _session_handle is never set on it. Relevance: None. The RM session lacks _event_state, _supported_event_types, and any event monitor. If event methods were called on it they would AttributeError long before reaching _fire_event. _fire_event can only be invoked on instrument sessions, and those always receive _session_handle. 3. The test fixture lib_and_session creates a MagicMock, manually attaches _event_state and _supported_event_types, registers it with the library, but forgets to set _session_handle. The existing test test_vxi11_fire_event_handler_mechanism passes only because its handler ignores the first argument, masking the fact that the mock object (not the integer handle) is being passed. Verdict: This is a test bug, not a code bug. The fallback accommodates an incomplete test fixture. 4. VISA spec compliance The handler contract (per the skill context and upstream pyvisa conventions) is: handler(session: VISASession, event_type: EventType, event_ctx: int, user_handle: Any) -> None VISASession is an integer handle. Passing self (a Python object) violates this contract. --- Recommendation ✅ Recommended: Option A Replace getattr(self, "_session_handle", self) with a direct attribute access: session_handle = self._session_handle Rationale: - Every session that can legitimately fire events is an instrument session created through highlevel.open(), which unconditionally sets _session_handle. - The fallback self is spec-violating and serves only to hide the incomplete test fixture. - Direct attribute access makes the invariant obviously correct: "a session firing events must have a VISA handle". - Option B documents incorrect behavior; Option C introduces unnecessary indirection for a problem that is really a missing test fixture line. --- Required Test Changes 1. Fix lib_and_session fixture (test_events.py:~345) Add one line so the mock session mirrors what real highlevel.open() does: @pytest.fixture def lib_and_session(): lib = PyVisaLibrary() sess = MagicMock() sess._event_state = EventState() sess._supported_event_types = {constants.EventType.service_request} sess._start_event_monitor.return_value = StatusCode.success session_id = lib._register(sess) sess._session_handle = session_id # <-- ADD THIS return lib, sess, session_id 2. Strengthen test_vxi11_fire_event_handler_mechanism (test_events.py:~695) Update the handler to capture the session argument and assert it is the integer handle (sid), not the mock object: def test_vxi11_fire_event_handler_mechanism(self, lib_and_session): lib, sess, sid = lib_and_session calls = [] def my_handler(session, event_type, context_id, user_handle): calls.append((session, event_type, context_id, user_handle)) # ... install and enable as before ... Session._fire_event(sess, constants.EventType.service_request, ctx) assert calls == [(sid, constants.EventType.service_request, 5555, "uh")] No other tests require modification because they either use the queue mechanism (which does not touch _session_handle) or test EventState/highlevel directly without calling Session._fire_event.

So I implemented the recommendation. I will need more guidance if this is leading in the wrong direction.

MatthieuDartiailh · 2026-05-06T08:47:50Z

+            session_handle = getattr(self, "_session_handle", self)
+            self._event_state.registry.fire(event_type, session_handle, ctx.context_id)
+
+    def _start_srq_monitor(self) -> StatusCode:


The name seems a bit narrow since other types of event can be monitored

Generalised the names to _start_event_monitor / _stop_event_monitor

…etween logic sections

…eric

…e access: session_handle = self._session_handle Rationale: - Every session that can legitimately fire events is an instrument session created through highlevel.open(), which unconditionally sets _session_handle. - The fallback self is spec-violating and serves only to hide the incomplete test fixture. - Direct attribute access makes the invariant obviously correct: 'a session firing events must have a VISA handle'.

for more information, see https://pre-commit.ci

SimonMerrett · 2026-05-07T00:15:41Z

@MatthieuDartiailh thanks for your review comments. I think I have addressed everything, although I cannot be confident I haven't introduced other mistakes. All pytest tests are passing but I haven't confirmed function on my test instrument yet. Please continue your review and I will confirm functionality asap.

…'_session_handle' [attr-defined]

bring local in line with remote origin.

SimonMerrett · 2026-05-07T18:32:28Z

pyvisa-py_srq-test_script_log.txt
test_pcap_after_commit-bdc446a.zip
Just tested with my test instrument and had successful script output. pcap attached to assist with verification of the protocol.

SimonMerrett and others added 2 commits April 28, 2026 22:40

Add event handling for TCPIP instruments, allowing SRQ interrupts, in…

ea7361e

…cluding tests

[pre-commit.ci] auto fixes from pre-commit.com hooks

7977ff1

for more information, see https://pre-commit.ci

MatthieuDartiailh reviewed May 6, 2026

View reviewed changes

SimonMerrett and others added 12 commits May 6, 2026 21:14

revert _vicp guards

787e741

freeze @DataClass and add slots

9d31db8

replace integer values with flags

4ca2692

use defaultdict in HandlerRegistry

61b9c93

add docstring argument descriptions to HandlerCallback

5a8f338

remove annotations import from __future__ and add readability lines b…

5e3fad5

…etween logic sections

rename _start and _stop _srq_monitor to _event_monitor to be more gen…

c08c610

…eric

replace error logging with warning

89db334

update tests with warning instead of error logging

047214e

Merge branch 'main' into events

4da2055

[pre-commit.ci] auto fixes from pre-commit.com hooks

f27fb19

for more information, see https://pre-commit.ci

SimonMerrett added 2 commits May 7, 2026 01:37

fix mypy pyvisa_py/sessions.py:387: error: Session' has no attribute …

322c2e7

…'_session_handle' [attr-defined]

Merge branch 'events' of github.com:SimonMerrett/pyvisa-py into events

bdc446a

bring local in line with remote origin.

		# vicp may not exist in older pyvisa versions
		_vicp = getattr(constants.InterfaceType, "vicp", None)

		(constants.InterfaceType.vicp, "INSTR"),
		*([(_vicp, "INSTR")] if _vicp is not None else []),

		self._deque.extend(kept)


		HandlerCallback = Callable[[Any, constants.EventType, int, Any], None]

Uh oh!

Conversation

SimonMerrett commented Apr 28, 2026

Uh oh!

MatthieuDartiailh commented May 4, 2026

Uh oh!

xyphro commented May 5, 2026

Uh oh!

codecov Bot commented May 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

MatthieuDartiailh left a comment

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!

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!

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!

SimonMerrett commented May 7, 2026

Uh oh!

SimonMerrett commented May 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov Bot commented May 6, 2026 •

edited

Loading