pyvisa / pyvisa-py

A pure python PyVISA backend
https://pyvisa-py.readthedocs.io
MIT License
282 stars 120 forks source link

assert_trigger() is defined twice #169

Closed tivek closed 5 years ago

tivek commented 5 years ago

https://github.com/pyvisa/pyvisa-py/blob/5f41742ca41beb0701d879558d2a5db5924a8485/pyvisa-py/highlevel.py#L246

https://github.com/pyvisa/pyvisa-py/blob/5f41742ca41beb0701d879558d2a5db5924a8485/pyvisa-py/highlevel.py#L198

assert_trigger() is doubly implemented by #136 and #139

MatthieuDartiailh commented 5 years ago

Good catch ! Feel free to open a PR to fix it if you have the bandwidth.

tivek commented 5 years ago

Sure, I can do that. I have a related question that I would like to go through with you first. When an invalid session is used, some PyVisaLibrary functions return the tuple 0, constants.StatusCode.error_invalid_object, while others return just constants.StatusCode.error_invalid_object. The latter seems to be more in line with what the NI backend does. I am inclined to return just a simple return code. What do you think? If you agree, the remaining functions that return tuples should also be touched up.

MatthieuDartiailh commented 5 years ago

At the C level in VISA, all functions return only the status. But pointers are used to pass additional information.

On the Python side, we usually return the status code and whatever value would have been returned through a pointer (the status code is usually returned last). You can have a look at the implementation of write for example (https://github.com/pyvisa/pyvisa/blob/master/pyvisa/ctwrapper/functions.py#L1852) and generally the proper signature for the library methods are defined in https://github.com/pyvisa/pyvisa/blob/master/pyvisa/highlevel.py#L374

I hope this clarifies the situation.

MatthieuDartiailh commented 5 years ago

Yes please fix all those at once in a dedicated PR (separate from the one in which you will introduce support for events). I really wish we could have a good test suite ... Thanks again for spotting those inconsistencies !

tivek commented 5 years ago

Thanks @MatthieuDartiailh. The question was originally about assert_trigger() since the two copies had different return types. But, as far as I can tell there is more. The following return type signatures are different than specified in VisaLibraryBase (pyvisa master):

This looks like a trivial refactor, but the changes are going to be over the place. What do you think? While I'm at it I can make a pull request you can review.

EDIT: Sorry, I edited my message so now it landed after your reply...