search

ZhuangLab / storm-control

Microscope control software
Other
65 stars 67 forks source link

added ms2000Module for RS232 #105

Closed aaronhalpern closed 3 years ago

aaronhalpern commented 4 years ago

For ASI MS2000 stage with RS232. XML Settings file usage:

....

<asi_stage>
  <module_name type="string">storm_control.sc_hardware.appliedScientificInstrumentation.ms2000Module</module_name>
  <class_name type="string">ASIStageRS232</class_name>
  <configuration>   
<com_port type="string">COM3</com_port>
  </configuration>
</asi_stage>

<!-- Stage control GUI -->
<stage>
  <class_name type="string">Stage</class_name>
  <module_name type="string">storm_control.hal4000.stage.stage</module_name>
  <configuration>
<stage_functionality type="string">asi_stage</stage_functionality>
  </configuration>
</stage>

....

HazenBabcock commented 4 years ago

Please separate this into two requests, one for the ms2000Module and the other for the single beam focus lock.

Comments:

aaronhalpern commented 4 years ago

Oops I thought I only pulled the MS2000!

I was playing with the single beam focus lock to get it to cleanly merge - not sure why that one showed up in the pull request. Sorry!

On Thu, Mar 5, 2020 at 11:18 AM Hazen Babcock notifications@github.com wrote:

Please separate this into two requests, one for the ms2000Module and the other for the single beam focus lock.

Comments:

  • ms2000Module - Please remove the print statements. I believe that if this is used for automated acquisition it will generate 100s of messages about the calculated stage move time.
  • Single beam lock - You are changing the default behavior by setting so many of the optional argument to True when they used to be False, and you add a lot of command line output. I'd suggest not doing this.
  • Single beam lock - You add code to uc480Camera that assumes that there is an ini file in the same folder. This isn't a good idea. Also, the Camera classes already accept an ini file as keyword argument, so why is this necessary?
  • Single beam lock - You should look at pointGrey.pointGreyLockCamera, SSLockCamera for some ideas about improving your single beam lock. In particular it isn't clear to me why you are writing a new fitting function instead of using the existing Python or C functions for this.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ZhuangLab/storm-control/pull/105?email_source=notifications&email_token=AB32R6FX76VK5SGQF3ORQX3RF73JFA5CNFSM4LCDFF5KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEN6RKOI#issuecomment-595399993, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB32R6A5SOWBC5BN3JBYO3LRF73JFANCNFSM4LCDFF5A .

HazenBabcock commented 4 years ago

I think you can see all the differences between your branch and master on your personal Github repository prior to opening the pull reqest, example.

HazenBabcock commented 4 years ago

Looking again, I think what happened is that you pushed additional changes to this branch after you created the pull request. When you do this, these changes are automatically added to the pull request.

aaronhalpern commented 4 years ago

Ahh crud. Sorry for that Hazen.

I will look at your Pointgrey focus lock. I have to admit my single beam is very hacky...

On Thu, Mar 5, 2020 at 11:49 AM Hazen Babcock notifications@github.com wrote:

Looking again, I think what happened is that you pushed additional changes to this branch after you created the pull request. When you do this, these changes are automatically added to the pull request.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ZhuangLab/storm-control/pull/105?email_source=notifications&email_token=AB32R6FDVZFOGTHYQCU67TDRF764JA5CNFSM4LCDFF5KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEN6U2LI#issuecomment-595414317, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB32R6GJL2A5EUTK3GYHSF3RF764JANCNFSM4LCDFF5A .