Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wish: More descriptive initialization #132

Open
FilipDominec opened this issue Jan 28, 2021 · 7 comments
Open

Wish: More descriptive initialization #132

FilipDominec opened this issue Jan 28, 2021 · 7 comments

Comments

@FilipDominec
Copy link

FilipDominec commented Jan 28, 2021

Instrumental has implemented a user-friendly module initialization, which auto-detects all available devices and silently skips all options that aren't available.

However, this user-friendliness fades away when everything seems installed properly, a device is attached and found by external commands (like lsusb on Linux), but it does not appear in the listing:

dominecf@machine:~/p/Instrumental $ sudo python3
Python 3.8.5 (default, Jul 28 2020, 12:59:40) 
[GCC 9.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from instrumental import instrument, list_instruments
>>> list_instruments()
/home/dominecf/p/Instrumental/instrumental/drivers/motion/ecc100.py:24: UserWarning: Driver 'instrumental.drivers.motion.ecc100' is out of date and incompatible with the current Instrumental core. Pull requests fixing this are welcome.
  warnings.warn(
[]
>>> 

On my notebook, I could use the TDC001 controllers flawlessly. On my desktop PC, now I have no clue what to do except for digging deep into the code with a debugger. FAQ does not help a bit. I suggest there should be some diagnostics.

By this I mean a function that recursively prints out

  1. which drivers were loaded (and no hardware was found), and
  2. which in contrast were skipped due to insufficient dependencies.

Maybe such a verbose printout should be even the default behaviour. Otherwise, a beginner can become completely lost.

@natezb
Copy link
Contributor

natezb commented Jan 28, 2021

I agree, there is a strong downside to Instrumental's approach of trying to make things "just work" automatically, because this can easily lead to confusion (especially for beginners) when something isn't working.

The currently prescribed method for diagnosing these list_instruments issues is to enable logging, which will tell you which driver modules could be loaded, etc. But I think you bring up two good points:

  1. It should be as easy as possible to enable this diagnostic logging (and easy to discover)
  2. It should be as clear as possible why an instrument doesn't show up (e.g. the driver module failing to load vs the driver itself not seeing the instrument).

I'm hesitant to add verbose output by default, but we can

  • Either add a verbose arg or a second, verbose version of the function
  • Prominently document this, in both the function docstring and all "getting started"-type documentation

@FilipDominec
Copy link
Author

  1. This is a good idea. I vote for adding a verbose arg to list_instruments.
  2. Furthermore I suggest that it could be verbose=True by default: Newcomers will intuitively get acquainted with the autodetection, experienced users can easily silence it and they will mostly use direct initialization instrument('12345') anyway.

@FilipDominec
Copy link
Author

  1. Explicit is better than implicit. We could perhaps distinguish between "serial" as in serial port and corresponding python module, and "serial" as in serial number. When I first saw such a named parameter describing the device, I was confused and thought that its values shall be e.g. COM1 or /dev/ttyS0. But I can see that "serial" is already established in many existing drivers for serial number, so the transition can be hard.

@richyjunior
Copy link

Hello, I am the starter you talked about. I am using a laptop running Win10 and a NI DAQ is connected. I can use it well by NI Max software or vendor-provided library (NI-DAQmx Pytthon). But instrumental tell me

>>> list_instruments()
/home/dominecf/p/Instrumental/instrumental/drivers/motion/ecc100.py:24: UserWarning: Driver 'instrumental.drivers.motion.ecc100' is out of date and incompatible with the current Instrumental core. Pull requests fixing this are welcome.
warnings.warn(
[]

>>>

I checked the FAQ, should I ignore this? otherwise what do I need to do?

@FilipDominec
Copy link
Author

I will try to issue a Pull Request for this. But currently it is hard for me to test Instrumental with real instrumentation as I am either fully occupied in the lab, or on home office. Please be patient.

@natezb
Copy link
Contributor

natezb commented Feb 4, 2021

I've already started working verbose list_instruments() output, so don't worry about it @FilipDominec. However, @richyjunior, that output is not strictly needed for your specific problem. If you have a problem with a particular driver, you can import it directly to see any errors:

>>> from instrumental.drivers.daq import ni

I know the documentation currently recommends using instrument() rather than importing the driver modules directly, but my opinion on this has been changing.

@natezb
Copy link
Contributor

natezb commented Feb 6, 2021

Check out PR #133, where I've added verbose output. Let me know what you think.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants