search

pyvisa / pyvisa-py

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

gpib: send the proper byte when using command #276

Open MatthieuDartiailh opened 4 years ago

MatthieuDartiailh commented 4 years ago
greyltc commented 4 years ago

Nah. This still doesn't work.

I don't much like the looks of https://github.com/pyvisa/pyvisa-py/blob/06511146938631a7c8984b5c33405b14bb28a171/pyvisa_py/gpib.py#L434 The boolean logic here is pretty opaque to me, so I don't exactly know what the idea is with this. I do think that what comes out of that line when I step through this code is not something that .command() should be called on later though and I think that's why this isn't working atm.

If I change that line to ifc = self.controller then the command goes out properly on the GPIB bus.

There are a bunch of these ors between these same objects floating around that file in other places, now I'm wondering how many of those are doing the right thing.

MatthieuDartiailh commented 4 years ago

@greyltc in your testcase are you calling gpib_control_ren from an INTFC or an INSTR ?

MatthieuDartiailh commented 4 years ago

Actually in that particular case the or is wrong and we should be using self.controller always. This comes from my confusion of thinking that commands could be sent by INSTR (they cannot) so we must always use the controller (global) rather than the interface (device specific). I pushed a change, if you can check before tomorrow it would be great !

greyltc commented 4 years ago

in your testcase are you calling gpib_control_ren from an INTFC or an INSTR ?

I'm just gonna open a new issue to answer that question so as not to combine two different bugs here :-) see https://github.com/pyvisa/pyvisa-py/issues/278

MatthieuDartiailh commented 4 years ago

It does seem I have been confused by the wording of the documentation that suggest using GTL but for a single device (since I thought command could be per device it kind of made sense) https://documentation.help/NI-VISA/viGpibControlREN.html . Indeed ibloc seems like a higher level alternative to fiddling to much with commands. I will try to push another patch tonight.

Note that I am still not sure how to deal with VI_GPIB_REN_ASSERT_ADDRESS_LLO that suggest LLO is sent to a single device. See my latest comment in #278 for a more up to date point of view.

MatthieuDartiailh commented 3 years ago

@LongnoseRob @tivek if you get a chance to test things over this GPIB, this PR could use some testing. I do not have access to GPIB at the moment but I hope to solve this issue over the summer.

LongnoseRob commented 3 years ago

@LongnoseRob @tivek if you get a chance to test things over this GPIB, this PR could use some testing. I do not have access to GPIB at the moment but I hope to solve this issue over the summer.

I gave it a qucik spin, but could not use the gpib_command() feature. please have a look at this https://gist.github.com/LongnoseRob/6e48ab2df96ef7ae5fa0083bd6181a12

is there also a special branch of gpib-ctypes if have to use?

MatthieuDartiailh commented 3 years ago

It has been some since I last looked at it but I do not think you need a new gpib-ctypes for this. Note that this PR focuses on fixing control_ren on pyvisa-py and the code could be nicer when https://github.com/pyvisa/pyvisa/pull/552 lands, so it may be worth testing this one first. Regarding gpib commands those can only be emitted from a GPIB::INTFC resource since they are send to the whole bus, hence the error you are seeing in your test.

dirkjankrijnders commented 10 months ago

Just getting my bearings around the code first.

I reran the steps @LongnoseRob did and got the same results. Not surprising of course as the unsupported is returned in line 517 of sessions.py. The gist of my script and its output is here.

Now on to the control_ren stuff.

dirkjankrijnders commented 10 months ago

Okay, that was faster than I expected. Test script and output are here. And I can confirm the device goes in remote for about a second and than returns to local. Others devices on the bus do not go to local.

MatthieuDartiailh commented 10 months ago

Thanks for testing @dirkjankrijnders !!!

Can I take from your last answer I can merge ?

dirkjankrijnders commented 10 months ago

I only tested the GTL command, the other one would take some time to time test as I do not complete understand what their effects would be. If you're happy with that you can merge indeed.

MatthieuDartiailh commented 10 months ago

I will give this another read before doing anything else since this PR is old. If I need to change anything I may ping you to test again what you can easily test.

dirkjankrijnders commented 10 months ago

Just to see what it does I tried sending VI_GPIB_REN_ASSERT_ADDRESS_LLO:

rm.visalib.sessions[my_inst.session].gpib_control_ren(pyvisa.constants.VI_GPIB_REN_ASSERT_ADDRESS_LLO)

and that resulted in an error:

Traceback (most recent call last):
  File "/home/dirkjan/src/pyvisa-commands/test_GTL.py", line 22, in <module>
    rm.visalib.sessions[my_inst.session].gpib_control_ren(pyvisa.constants.VI_GPIB_REN_ASSERT_ADDRESS_LLO)
  File "/home/dirkjan/src/pyvisa-commands/pyvisa-py/pyvisa_py/gpib.py", line 498, in gpib_control_ren
    board_pad = int(self.controller.parsed.primary_address)
                    ^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'Gpib' object has no attribute 'parsed'

Next tried VI_GPIB_REN_ASSERT_LLO:

rm.visalib.sessions[my_inst.session].gpib_control_ren(pyvisa.constants.VI_GPIB_REN_ASSERT_LLO)

And that did not cause error, but also no visible change to the device. The latter can be that it is not in the correct mode.

This is all the testing I can do now. I can probably test more tomorrow.

codecov[bot] commented 10 months ago

Codecov Report

Attention: 16 lines in your changes are missing coverage. Please review.

Comparison is base (21dd100) 24.89% compared to head (8b49388) 24.85%.

Files Patch % Lines
pyvisa_py/gpib.py 5.88% 16 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #276 +/- ## ========================================== - Coverage 24.89% 24.85% -0.04% ========================================== Files 25 25 Lines 3515 3524 +9 Branches 490 490 ========================================== + Hits 875 876 +1 - Misses 2622 2630 +8 Partials 18 18 ``` | [Flag](https://app.codecov.io/gh/pyvisa/pyvisa-py/pull/276/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pyvisa) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/pyvisa/pyvisa-py/pull/276/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pyvisa) | `24.85% <5.88%> (-0.04%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pyvisa#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

MatthieuDartiailh commented 10 months ago

I rebased and hopefully fixed the issues you encountered.

dirkjankrijnders commented 10 months ago

Hi, the rebase did introduce a mismatch between PyVISA and pyvisa-py. Pyvisa-py now depends on pyvisa>=1.13.0 and the gpib-commands branch of PyVISA is only 1.11. So I think PyVISA also needs a rebase. Installing with --no-deps results (of course) in many errors...

MatthieuDartiailh commented 10 months ago

The PyVISA branch has been rebased @dirkjankrijnders can you test again ?

MatthieuDartiailh commented 9 months ago

I rebased this on latest main and modified it to make use of the GPIBCommand definition of PyVISA. @dirkjankrijnders could you give this another spin and then I will merge and make new releases of pyvisa and pyvisa-py both.