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

pyVISA error triggered by setting the pm100 averaging window #8

Open
mvbnano opened this issue Nov 22, 2017 · 5 comments
Open

pyVISA error triggered by setting the pm100 averaging window #8

mvbnano opened this issue Nov 22, 2017 · 5 comments
Labels

Comments

@mvbnano
Copy link
Member

mvbnano commented Nov 22, 2017

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.

@mvbnano mvbnano added the bug label Nov 22, 2017
@latchr
Copy link
Member

latchr commented Nov 30, 2017

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

@mvbnano
Copy link
Member Author

mvbnano commented Nov 30, 2017

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

@latchr
Copy link
Member

latchr commented Nov 30, 2017

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
Copy link
Member Author

mvbnano commented Nov 30, 2017

@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
Copy link
Member Author

mvbnano commented Nov 30, 2017

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

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

No branches or pull requests

2 participants