gpib: send the proper byte when using command

[MatthieuDartiailh]

• Closes GPIB interface.command() calls don't seem to work #275
• Executed black . && isort -c . && flake8 with no errors
• Added an entry to the CHANGES file

[greyltc]

Nah. This still doesn't work.

I don't much like the looks of

pyvisa-py/pyvisa_py/gpib.py

Line 434 in 0651114

ifc = self.interface or self.controller

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]

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

[MatthieuDartiailh]

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]

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 #278

[greyltc]

Should this even be allowed? (writing specifically about the 0x01 command here, but I guess this applies to most of the other stuff this function does) The user might not expect every instrument on their GPIB bus to go into local mode when they call a function on a single instrument. I think it's better to only expose gpib_control_ren() for bus controller objects. This way the user is forced to understand that they're doing something that impacts the whole bus. See #278 for a problem with that today.

I think sending a single instrument to local mode should be done with ibloc() because that's a device action, not a bus action. I don't see ibloc() exposed anywhere in pyvisa-py right now though.

[MatthieuDartiailh]

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]

@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]

@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]

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 pyvisa/pyvisa#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]

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]

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]

Thanks for testing @dirkjankrijnders !!!

Can I take from your last answer I can merge ?

[dirkjankrijnders]

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]

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]

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]

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 ⚠️

Additional details and impacted files

@@            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	Coverage Δ
unittests	24.85% <5.88%> (-0.04%)	⬇️

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.

[MatthieuDartiailh]

I rebased and hopefully fixed the issues you encountered.

[dirkjankrijnders]

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]

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

[MatthieuDartiailh]

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.