-
Notifications
You must be signed in to change notification settings - Fork 141
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
Enable PDMR measurements by adding analog channel capabilities to National Instruments hardware #258
Conversation
That looks like a great improvement for the odmr code. Thank you for your work. I might even have the possibility to test this pull request with a setup using a photodiode for odmr instead of an apd. Let's hope that the control pc contains a x-series ni card... |
hmm, the PC has the NI 6229 card, so just two counter :/ |
Hey, |
The 0 was probably the APD count value. It is right that the counter GUi can only show one channel as a number right now, but shows all channels in the plot. I will probably add a channel selection box like in the confocal and ODMR GUIs. |
There is now an option letting you select, which count trace should be displayed on top. Additionally you can select which traces are to be plotted. However, the save still saves all the traces.
All known issues were fixed. |
We've been using this at https://github.com/WarwickEPR/qudi/tree/analog_readout - it is working well. There's a small error in the logic for the analog readout of a counter when you have multiple analog channels -- see the fix at https://github.com/WarwickEPR/qudi/blame/1bb7b9512b2d0cb30188ffebfd2a351cc832ab55/hardware/national_instruments_x_series.py#L630 |
There is now an option letting you select, which count trace should be displayed on top. Additionally you can select which traces are to be plotted. However, the save still saves all the traces. ---- Also fixed a small error in returning the analogue values for multiple channels. Thanks to Ben for the Tip!
…to nidaq_analog_etc
Directly included your fix. |
In the current implementation, all the analog additions have been made optional, but the national_instrument_x_series module still explicitly requires APD-style photon counters. This is frustrating in systems with only analog detection, as the default detection channel for all windows (confocal, odmr etc) is the vestigial photon counter channel. I have removed these dependencies and tested them in https://github.com/bens0n/qudi/tree/upstreamnidaq_analog_etc. Do you want to consider including them in this PR or would you rather a separate PR once this current one has been merged? |
I disagree with the merge here.
I am sorry to make this inconvenient for other people but what I saw last it does not agree with our programming standards (logic should be done in logic not in hardware), just as a lot of stuff from NIDAQ does not comply with this standard.
Adding this into the trunk now, would mean that removing this problem would become more difficult.
Therefore I suggest we sit together and see how this could be done better and then merge.
…-Andrea
Von: Kay Jahnke [mailto:notifications@github.com]
Gesendet: Dienstag, 30. Januar 2018 17:58
An: Ulm-IQO/qudi <qudi@noreply.github.com>
Cc: Subscribed <subscribed@noreply.github.com>
Betreff: Re: [Ulm-IQO/qudi] Enable PDMR measurements by adding analog channel capabilities to National Instruments hardware (#258)
All known issues were fixed.
Please double check the code again.
Then someone needs to merge it with the branch (or show me how to do it), before we start including all the changes made by Andrea.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub <#258 (comment)> , or mute the thread <https://github.com/notifications/unsubscribe-auth/AWacL1NCBK7JynUvR3sup1zKyiapgbgAks5tP0oCgaJpZM4PTUj2> . <https://github.com/notifications/beacon/AWacL26WRd2f8wMRhvYuMdO50tppUEcBks5tP0oCgaJpZM4PTUj2.gif>
|
After a short discussion today, we decided to first merge everything in a non breaking way. |
# Conflicts: # gui/counter/countergui.py # hardware/national_instruments_x_series.py
Ok, I tried to resolve the conflicts with master. Can someone please quickly check if this branch still works as intended? I'm not sure - @kay-jahnke - what you mean with non-breaking. |
@bens0n, I think for a better overview and cleanliness we should not rush to put all recent changes to the NI card module into one single pull request. |
No problem - sounds great! ni_card is definitely the thorny beast lurking under the waters of Qudi. |
…race is a constant value. Software tested with physical photodiode.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ready to merge
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested on photodiode setup. Works!
Enable PDMR measurements by adding analog channel capabilities to National Instruments hardware
ConfigOption
-only configurationDescription
ODMR:
Handle multiple channels that are recorded at the same time and can be shown in the GUI.
Requires changes to
ODMRCounterInterface
an the data stored inodmr_logic
.NI Card:
Renamed to
national_instruments_x_series.py
so pople do not wrongly assume that M-Series cards should work fine and then complain.Added the possibility to record voltages on analog inputs.
Made all configuration variables into
ConfigOption
entries.Old configuration:
New configuration:
Motivation and Context
Recording an analog voltage during scans is useful for research like Photoelectric detection of electron spin resonance.
At the same time, we can simplify the parsing of configuration options greatly by using lists and
ConfigOption
:How Has This Been Tested?
Types of changes
Checklist: