QMAPP-mq / qudi

A modular laboratory experiment management suite.
GNU General Public License v3.0
4 stars 0 forks source link

pyVISA error triggered by setting the pm100 averaging window #8

Open mvbnano opened 6 years ago

mvbnano commented 6 years ago

What is affected by this bug?

All functionality with the PM100A is lost after a change to the averaging window.

When does this occur?

When you try and set the PM100A averaging window.

pm100.set_averagin_window(1)

Where on the platform does it happen?

The error is triggered by pyVISA.

How do we replicate the issue?

Load the pm100 module and run:

pm100.set_averagin_window(1)

then try and read the power:

pm100.get_power()

Expected behavior (i.e. solution)

It should not crash.

latchr commented 6 years ago

It works for 300e-3, and for 10, 100, etc. It consistently fails for 0.9 and for 1.

mvbnano commented 6 years ago

Note that the averaging window, read from the configuration, is not used at all. To be implemented at the resolution of this issue.

latchr commented 6 years ago

I just noticed that the get_power() method calls this before it reads the power:

self.ThorlabsPM.sense.average.count = self._averaging_window

Why does the get_power method need to set the average.count value? And self._averaging_window is in seconds, while the average.count value needs to be set to integer number of "units". I suspect this might have something to do with this bug, since it is triggered by calling get_power()

mvbnano commented 6 years ago

@latchr I think you are correct, I do not see a reason for

self.ThorlabsPM.sense.average.count = self._averaging_window

to be called in get_power, as self._averaging_window is set elsewhere.

However, I still don't see how that line would be a problem.

I have commented it out, and will merge into the deployed branch without testing on this branch (bad practice but this should be a non-breaking change.

mvbnano commented 6 years ago

Note that the default averaging window listed in the config file is still not set on activation.