[WIP] Gpib events #175
[WIP] Gpib events #175
Conversation
There was a problem hiding this comment.
Overall this looks good. I left some minor comments that are easy to address.
I am fine with keeping sessions separate from events, it makes things easier to follow on the whole I believe.
Regarding your question about Attribute the question is more complicated. Actually pyvisa does not have an event type (wait_on_event does not return an event object) and looking through the event support in pyvisa it all looks very low-level (I am not even sure what handler should be but it looks like some ctypes handler) . Since I have never worked with VISA events it would be helpful if you could describe your use-case and the kind of API you would like to see.
pyvisa-py/highlevel.py
Outdated
| except KeyError: | ||
| return StatusCode.error_invalid_object | ||
|
|
||
| # TODO: add support for VI_HNDLR, VI_SUSPEND_HNDLR, VI_ALL_MECH |
There was a problem hiding this comment.
I feel like the session should filter itself for what kind of event it supports like you already did in GPIB.
There was a problem hiding this comment.
Before factoring out common functionality to the Session parent class, I am inclined to see some other Session types support events. Do you think different Session subclasses would need fancier filtering, eg. by mechanism and event type?
There was a problem hiding this comment.
I think there is a misunderstanding here. My point is that the VisaLibrary should not filter the mechanisms but let the session decide if it supports the mechanism.
There was a problem hiding this comment.
Ah, you're correct, sorry. I agree of course, the session should filter itself.
Here I was quite confused about separation of concerns. For instance, I see that pyvisa.VisaLibraryBase already defines a .handlers member instead of having handler registration, storage, mechanism etc. delegated to (EDIT) sessions.
There was a problem hiding this comment.
I guess that it is because pyvisa when using NI-VISA (or similar) does have access to sessions but need to keep track of handlers.
There was a problem hiding this comment.
Looks that way, yes. I'd say keeping track of handlers is an implementation detail that should go into NIVisaLibrary.
pyvisa-py/highlevel.py
Outdated
| except KeyError: | ||
| return StatusCode.error_invalid_object | ||
|
|
||
| # TODO: add support for VI_HNDLR, VI_SUSPEND_HNDLR, VI_ALL_MECH |
| except KeyError: | ||
| return None, None, StatusCode.error_invalid_object | ||
|
|
||
| # FIXME: for now calling Session's own wait_on_event() is the only way |
There was a problem hiding this comment.
I am not sure I follow why this is an issue. Could you elaborate ?
There was a problem hiding this comment.
This particular comment probably does not belong in there anymore, but the gist of it is I am unclear how to nicely implement the VI_HNDLR mechanism. Event handlers are sitting stored in PyVisaLib.handlers. They are supposed to be called by Sessions (async nonetheless, and that is a whole other can of worms) which should pass event information to them. This was a note for me that the only clean way for sessions to pass events back to the library is to return them from wait_on_event.
There was a problem hiding this comment.
Once again I feel like I don't have a good enough understanding of VISA events. And of how this would translate in Python (is the ctype handler called in a different thread). If we go further in the direction of event support we should investigate making handler easier to use. In particular I am unclear how to make both backends behave in a similar manner.
Ideally I would like top be able to pass a python function as handler but that would require some work in the case of the ni backend.
There was a problem hiding this comment.
I feel implementing event handlers correctly in pyvisa-py is going to be very difficult for little gain. This part of VISA seems to be best implemented at driver level with callbacks into the app.
For example, I have no idea how to have the VI_HNDLR mechanism detect REN line changes of a GPIB board without continuously polling in a separate thread, and even with all the polling a quick pair of on-off events could easily be skipped completely.
I suggest testing and stabilizing the VI_QUEUE mechanism first. It is synchronous and simple. It only needs to implement Session.wait_on_event() which empties the session's own event queue or waits for an event to happen. That alone would already cover most of GPIBInstrument synchronization needs.
There was a problem hiding this comment.
I do agree let's focus on the queue mechanism that is easier. It is just that diving into the pyvisa side of handlers made me realize that they may be some work to do there to to make it more usable.
There was a problem hiding this comment.
Thanks for asking. I'm happy to join and help with testing whenever the lab is available.
There was a problem hiding this comment.
I got the proper privileges so I will invite you directly @tivek.
There was a problem hiding this comment.
@tivek ping me when this is ready for further reviews.
Right now I feel GPIB event queues are done. It still needs another round of testing and comparing with the NI backend. Right now I only have access to linux-gpib, but during the winter break I will most likely get my hands on a Windows machine with NI hardware.
Apart from that there is the event handler rework on the pyvisa side as discussed above: https://github.com/tivek/pyvisa/tree/expose_events. I should have a WIP pull request soon.
There was a problem hiding this comment.
I got the proper privileges so I will invite you directly @tivek.
Great, thank you.
pyvisa-py/gpib.py
Outdated
| @@ -38,6 +38,46 @@ def _inner(self): | |||
| raise | |||
|
|
|||
|
|
|||
| class GPIBEvent(object): | |||
There was a problem hiding this comment.
As I mentionned in my review, I am not sure how to expose this since pyvisa does not expose events. We should perhaps think about how to expose this in pyvisa and how to make it compatible with pyvisa-py.
There was a problem hiding this comment.
GPIBEvent is not a user-facing class, it is an implementation detail: PyVisaLibrary stores GPIBEvent objects in the PyVisaLibrary.events dict and passes any get_attribute() and close() calls down to them. Such "attribute query workers" are required pieces of the puzzle to fully conform to pyvisa's handle-based backend interface.
I agree that pyvisa needs a more pythonic way to expose events, and I think it is actually quite straightforward to get there. Currently, events are actually exposed to the user as raw VISA event contexts/handles. This is not very pythonic but is nevertheless there as a public and documented API. Our user can get hold of an event handle in two main ways:
visalibcalls the installed event handlers and passes the event handle as a parameter,Resource.wait_on_event()returns aWaitResponseobject which exposes a member calledcontext.
In both cases, the user can find out more about the event by calling visalib.get_attribute(handle, attribute) or close the event using visalib.close(handle). EDIT: ... and this is why GPIBEvent is there, to provide access-by-handle functionality for PyVisaLibrary in order to fully emulate NIVisaLibrary's behavior.
Instead of low-level handles/contexts, pyvisa should give users some kind of an event object that can be queried and knows how to clean up when it is done. The WaitResponse class already fits the bill nicely: all it needs to become user-friendly are get_attribute() and close() methods and renaming it eg. Event. The remaining step is to wrap user-provided event handlers so that they are passed Event objects when called instead of raw event handles.
There was a problem hiding this comment.
Indeed this sounds like a plan.
For the work at hand, even if we do not implement event support right away, we can probably set up a generic class that could be used by all resources rather than an explicitly GPIB restricted class, no ?
There was a problem hiding this comment.
Right. Looks pretty generic actually. If any session needs to compute event attributes on the fly, they can override get_attributes(). Right now I'm struggling with bikeshedding :) I think "Event" should only be the name of the user-facing pyvisa class.
|
@tivek ping me when this is ready for further reviews. |
|
A quick update. The event work in pyvisa was on hold due to hardware issues. I expect to be able to continue working on it in the following weeks. In the meantime I plan to thoroughly test the changes in pyvisa-py on our test equipment. |
|
Sounds good @tivek . Let me know if I can do anything. I am currently working on pyvisa documentation and I will try to give a shot at implementing GPIB INTFC for pyvisa-py once I am done(see pyvisa/pyvisa#400 for a use case). |
|
@tivek any news on this ? |
I am trying to replicate the behavior of event queues in NI-VISA. There appear to be some differences in how my KUSB-488A with linux-gpib handles GPIB events with respect to NI adapters on Windows. Not sure if it is down to my code (most likely), the instrument (Tek osci), OS, driver version... This is taking a bit more tinkering than I thought :) |
|
@tivek did you made any progress on this ? I am planning new releases for end of July, beginning of August, would like to push to get this in or will this wait for the next one ? |
Thank you for your patience. I was until recently out of town and without lab access. I am having increasing difficulties to reproduce behavior of Windows NI-VISA with Linux-GPIB with my instruments. I am not sure if it is something to do with how I initialize the instruments over GPIB or if I am misunderstanding VISA events. My estimate is GPIB event support will not be ready for the next release. I am planning to exclusively work on this in August when the instruments are not likely to be used for regular experiments. |
|
Sounds good and thanks for the update. Could you also have a look st tivek/gpib_ctypes#3 if you have a moment ? |
|
Was looking for events use with pyvisa-py and found this PR, any info available when work on this will continue? |
|
I do not have the bandwidth at the moment to take over @tivek work, and I did not hear from him since his last post. If this is something you are interested in however, I will be happy to review a PR. |
Hello, working from home during the last year has left me without access to gpib equipment so unfortunately I am mostly stuck as far as testing on real-life equipment goes. Where I got stuck was: the linux-gpib driver for my NI controller either has missing or buggy automatic serial polling. This is needed for any kind of practically useful event support in pyvisa-py. With that bit not really working on my setup, I was not able to make meaningful progress. Hopefully with vaccinations going forward I'll be able to pinpoint what exactly is the problem and see how to continue. |
|
@tivek thank you for the feedback |
Great. Actually, what would help is testing some short gpib code to see if and how autopoll works on your setup. Then we can proceed with rudimentary events. Would that be ok? If so, it would be great if you could find some quick-but-not-too-quick action or measurement on HP3478A which would eg. raise the SRQ bit when it is done. @MatthieuDartiailh is it ok that we continue this gpib-specific communication here? |
|
Sure go ahead |
Yes, we could set the SRQ mask for the data ready feature, and some ranges the conversation rates are very low (only 2-3 readings per second). If this is would be too fast we still could using single trigger and GET (group exceute trigger) to time it even slower. Could you point me to which source to checkout to get ready for the testing? |
Sounds like a plan. The code I used for testing sits locally in my lab - sorry about that. In the next one or two days I am going on site and then I can prepare a minimal example for you to test on linux-gpib. Basically, in order to implement wait_on_srq() for pyvisa-py we need to ibwait() for a RQS event and then check if the device in question generated the request. I observed that linux-gpib with the NI GPIB-USB-HS ignores the IbcAUTOPOLL configuration option and treats it as always disabled, so an ibwait() on the RQS event times out. On the other hand, NI GPIB drivers on Windows respects autopoll as expected. |
|
@tivek just wanted to check if you had a chance to retrieve the code used for testing? |
@LongnoseRob thanks for your patience, I had to leave it for next week because of some sideffects from the covid shot. Luckily next week I should get more time allotted to play with the instruments. |
|
@tivek |
|
Has this PR been abandoned? Will apply against the latest master branch? |
|
@tivek has not given any news recently. If you are interested by this work I think you can pick it up where he left it. |
Request for comments, related to Issue #142