ViSR: Tomography form, and better controls drawer capability - #157
ViSR: Tomography form, and better controls drawer capability#157tomkane-dls wants to merge 9 commits into
Conversation
…th vertical seperators
tomkane-dls
changed the title
|
This looks good to me. |
EmsArnold
left a comment
EmsArnold
left a comment
There was a problem hiding this comment.
Looks like the form itself works well - just a few minor changes to match up with the new plans in test-rig-bluesky.
| }} | ||
| > | ||
| <RunPlanButton | ||
| name="calibration" |
There was a problem hiding this comment.
Looks like the calibration plan in test-rig-bluesky is called collect_calibration_images, rather than just calibration
There was a problem hiding this comment.
i will make sure these match thanks! I decided to ask Yousef what would be the correct name for the process as i feel calibration is not quite right
| > | ||
| <RunPlanButton | ||
| name="calibration" | ||
| params={formData} |
There was a problem hiding this comment.
This is currently passing through the number of projections as well - I don't know enough about the background/dark field needs for tomography to comment, but do you need exactly as many projections, or can you make do with fewer?
There was a problem hiding this comment.
Good point, no we don't need that many, we only need 1. The plan will set a number like 20 as standard and they can be averaged, but the number of projections is meaningless here
| name="calibration" | ||
| params={formData} | ||
| instrumentSession={instrumentSession} | ||
| buttonText="Take Dark/Flat" |
There was a problem hiding this comment.
Might be better to say something like "Calibrate For Scan", to make it more clear for people who just turn up. Potentially also good to get feedback from Comms
There was a problem hiding this comment.
Yes i will wait for Yousef and go with what he thinks, and then will make a note to ask Comms about this in future
| // if (intervalRef.current) clearInterval(intervalRef.current); | ||
| // setVolumeVisible(false); | ||
| // setProgress(0); | ||
| // }; |
There was a problem hiding this comment.
Is the plan to put this back at some point? If not, can this just be deleted rather than commented out?
There was a problem hiding this comment.
I think you are right, we will delete and if we do re-implement something similar it will likely be different anyway
|
All of Emily's points addressed. Exact form data and plan names may change but these plans are not active currently Working out whether drawer or plots should be really controlling heights we will approach in another PR |
3 participants
volumeVisible is just left as always True for now, and all the state handlers have been removed. When plans can be submitted we can discuss how we want to hide/reset figures
we could disable plan buttons when light off is selected but i think this is unnecessary .