Fwbell5080 implementation seems to be broken

[BenediktBurger]

https://github.com/pymeasure/pymeasure/blob/d7cbd3c850fe7120eb56ee29039269671b5f443c/pymeasure/instruments/fwbell/fwbell5080.py#L116-L129

The ask/values call to the parent class does not contain a command parameter and cannot work in the current implementation. I guess someone copied the read method and did not adjust it correctly.

[bilderbuchi]

This ask() implementation was added in aba03d216606d3ed7e5f7c98e7cf30d70f2371b2 (#33) and half-fixed in 161aa8a8fc3421745eb5eec325c599c501707e9d (#48), both by @cjermain. I think the fix is straightforward, but this shows impressively that this instrument was probably not used since mid-2016 :sweat_smile: .

[mcdo0486]

Hi,

I noticed this was broken too a few weeks ago, and modified the instrument file to use VISAAdapter, which implements a pyVISA SerialInstrument. I was able to remove those extra functions. After reading this issue I pushed a new branch on my fork: https://github.com/pymeasure/pymeasure/compare/master...mcdo0486:pymeasure:fwbell5080

What do people think about using pyVISA whenever possible?

Dan

[BenediktBurger]

Hey Dan,

Infact, pyvisa is the default connection type and other adapters are for special connections.

Thanks for fixing this device.

Do you have it and can test the code? In this case, I'd ask you to verify the units control. A newer version expects ':UNIT:FLUX:%s', but the code does not contain the :. Could you check this? Does your modified way of getting the field measurement work?

By the way, we changed the default parameter for the adapter from 'resourceName' to 'adapter'. If you pull the current master branch, you'll get a comment regarding the 'unit' control and the name change.

You can define the default serial connection settings via the 'asrl' keyword parameter to Instrument. This parameter takes the dictionary you assigned by default to the kwargs. (see documentation adding instruments)

With these changes, it would look good to me. I encourage you to open a pull request.

If you want, you can write additionally unit tests with our new protocolTest method (see the documentation regarding adding new instruments, bottom of the page).

Thanks for helping keeping the code base working!

[mcdo0486]

I have submitted a pull request with those change: https://github.com/pymeasure/pymeasure/pull/714

[BenediktBurger]

Closed via #714