search

pcdshub / pcdsdaq

Utilities for using the DAQ's pydaq, pycdb, and pyami libraries in conjunction with Bluesky
https://pcdshub.github.io/pcdsdaq
Other
0 stars 9 forks source link

Cryptic Error Message in DAQ Class #5

Closed ZLLentz closed 6 years ago

ZLLentz commented 6 years ago

This is a copied issue from https://github.com/pcdshub/pcdsdevices/issues/154

Expected Behavior

If daq.connect() fails, it should always give a helpful error message.

Current Behavior

Abdullah observed the following response today:

In [12]: daq.connect()
pdsdaq_connect(): get dbpath len = 0

Possible Solution

Go to a hutch, find some common failure states (wrong platform, daq not allocated), see what the response is inside the code. Make sure we catch these and give proper error messages.

Steps to Reproduce (for bugs)

  1. create a Daq object with the wrong or right platform
  2. forget to allocate the daq
  3. try to connect

Context

This bug creates extra debugging time when something goes wrong in the setup of a Daq class.

Your Environment

pcds-0.5.2

ZLLentz commented 6 years ago

If you try to connect to a daq with the correct platform, but the daq is not allocated, you get:

pdsdaq_connect(): get dbpath len = 0
INFO                 Failed to connect to DAQ - check that it is up and allocated.

Where the print statement comes from pydaq when we talk to "something" but get no data back. The message on the exception object is 'Initial query failed'. (str(err) to access the message)

If you have the wrong platform, you just get:

INFO                 Failed to connect to DAQ - check that it is up and allocated.

If you have the right platform but the daq is down, you also get:

INFO                 Failed to connect to DAQ - check that it is up and allocated.

And the exception's message is 'Connect failed'

So we can pinpoint the error for when the daq isn't allocated. This can become part of a PR. However, for the platform being wrong vs daq down there is actually no difference in python.

So the options are:

Actually, scanning all the platforms seems... not unreasonable. @teddyrendahl can you think of an obvious reason why this would break things? Was all this platform config unneeded?

teddyrendahl commented 6 years ago
  1. I think it is good to have proper error handling on the DAQ. This helps the user and helps us when debugging user complaints 👍
  2. I think scanning all the platforms is not unreasonable. I'm fine with protecting the user from themselves, thought I also don't think it is unreasonable just to specify the right platform in the config file. I'm wishy-washy on making a software fix for this as it is something that we will probably only have to do ~7 times. If you are interested in writing it that is fine, but I don't think this should be high priority.
ZLLentz commented 6 years ago

I've already implemented the scanning and will make a PR. For me the big advantage here was that I no longer have to find a reasonable error message for having the wrong platform, since connecting to the wrong daq and there not being a daq to connect to look exactly the same. So now there are two very clear error messages:

  1. There is no daq running on this host at all! You should start and allocate it, or switch to the machine that is running the daq.
  2. There is a daq running on this host, but it's not allocated yet! You should allocate it!