Conversation
…in motor serial number set function
…enter into NewSaturation
…level (see set_metapref branch)
…it to class-based prefs
ichristen
left a comment
ichristen
left a comment
There was a problem hiding this comment.
"Optionally also monitors a power meter to calibrate measurement with power."
This is important for a proper experiment, but doesn't seem to have been added back in.
| end | ||
|
|
||
| function set.motor_serial_number(obj,val) | ||
| function val = set_motor_serial_number(obj,val,~) |
There was a problem hiding this comment.
obj.motor_serial_number = val; is now unnecessary (below).
| prefs = {'linein', 'lineout', 'acquire', 'nsamples', 'wavelength'}; | ||
| prefs = {'angles','exposure','motor_move_time','motor_home_time','motor_serial_number'}; % String representation of desired prefs | ||
| % show_prefs = {}; % Use for ordering and/or selecting which prefs to show in GUI | ||
| %readonly_prefs = {}; % CC will leave these as disabled in GUI (if in prefs/show_prefs) |
| APD_line = Prefs.String('APD1','help_text','NiDAQ line to apd','allow_empty',false); | ||
| APD_sync_line = Prefs.String('CounterSync','help_text','NiDAQ synchronisation line','allow_empty',false); |
There was a problem hiding this comment.
Why not make these multiple choice? You can get the nidaq in/out lines from the device. Also, why don't you have to set the nidaq device anywhere O_O?
| spacing = 2.25; | ||
| % Set methods allow validating property/pref set values | ||
| function newVal = setAngle(obj,val,pref) | ||
| obj.angle_list = str2num(val); |
There was a problem hiding this comment.
str2num won't error on bad input. You should check for an empty array before assigning to angle_list.
Consider:
a = str2num('abc');a will be [].
| if isnan(val_as_double) | ||
| return | ||
| end |
There was a problem hiding this comment.
unreachable because of assert before.
| % Setup graphics | ||
| y = NaN(1,Nangles); | ||
| hold(ax,'on'); | ||
| plotH(1) = plot(obj.angle_list, y,'color', 'k','parent',ax); |
There was a problem hiding this comment.
if only one plotH, why index into it? Also why specify color (not an issue, just curious)?
| plotH(1) = plot(obj.angle_list, y,'color', 'k','parent',ax); | ||
| ylabel(ax,'Counts (cps)'); | ||
| xlabel(ax,['Angle (' char(176) ')']); | ||
| yyaxis(ax, 'left'); |
There was a problem hiding this comment.
Why is this being called? I only see one plot.
| % Continuously acquires data from the APD and Thorlabs PM100 power meter and plots them. | ||
| % User should rotate the polarizer/HWP. | ||
|
|
||
| % Saturation changes intensity on a sample using a HWP (motorised or manually moved) and monitors the APD to measure saturation. Optionally also monitors a power meter to calibrate measurement with power. |
There was a problem hiding this comment.
Does this really still support manually moving HWP?
| while (obj.rot.Moving || ~obj.rot.Homed) && (toc(t) < maxTime) | ||
| drawnow | ||
| if toc(t) > maxTime |
There was a problem hiding this comment.
The chance that line 73 executes is extremely tiny (toc(t) would have had to go from < maxTime to > maxTime with only a drawnow between)! Your while loop is set to exit if maxTime is exceeded without erroring. This seems problematic?
| if ~isempty(obj.rot) | ||
| status.String = sprintf( 'Navigating to %g (%i/%i)', theta, ... | ||
| i, Nangles); drawnow; | ||
| obj.rot.move(theta); | ||
| else | ||
| pause(5) | ||
| end |
There was a problem hiding this comment.
Assert above would prevent the else from ever happening. This seems to be the only thing that might give a user access to manually do it. But so many other things would break as they currently stand. I recommend just removing manual mode for now.
The easiest way to fix this would be to actually create a manual motor. Then, never change the code, but simply drop in a different motor that calls questdlg.
|
Hey @ibwharri , what's the status of this pull request? Would be great to add this to CC after comments have been addressed! |
4 participants
Changes experiments that use APT motors to have class based prefs. Updated emitter saturation experiment.