search

mesoSPIM / mesoSPIM-control

Image acquisition software for mesoSPIM light-sheet microscopes.
http://www.mesospim.org
GNU General Public License v3.0
37 stars 12 forks source link

Pi xyzf stages #52

Closed drchrisch closed 3 years ago

drchrisch commented 3 years ago

Hi folks, Here is an updated pull request to integrate stepper motor stage control into mesoSPIM-control.

I made some efforts to reduce code redundancy. The new "PIStages" class was developed for integrating stage drives with stepper motors but may also work with other PI setups.

@nvladimus, it is a bit funny with the pipython software. When I set up python using miniconda and pip last year, I unintentionally installed pipython==2.3.0.3 from pip. And that works just fine ("startup" command is still present). There is no need to copy the GCS translator dll, we probably got this with other PI software that we needed anyway to generate a stage description for the custom focus drive.

nvladimus commented 3 years ago

Hi, @drchrisch Thanks for the updated PR! I tested your code on our V5 system (merging the latest development & #52) with the new PIstage configuration. The stages started fine, your class constructor works for both controller configs (multiple C-663 vs single C-884). Unfortunately the code crashes when tried to get the current position, because our C-884 controller knows that it has 5 axes connected, while the code assumes the controller is always single-axis.

There are some chunks of code that came from development branch in my commit, feel free to ignore them.

I also tested pipython==2.3.0.3, but the error module 'pipython.pitools' has no attribute 'startup'still happens, so I rolled back to the original pipython==1.3.5.37.

nvladimus commented 3 years ago

After more pondering, I think merging the two classes is actually not worth the effort, and it will make config file ugly. I suggest we keep both classes and config settings, but rename them to avoid confusion in future: In config file:

In mesoSPIM_Stages.py:

And keep the rest of the code as is. What do you think?

drchrisch commented 3 years ago

Looks really good, I'm absolutely fine with this and would appreciate if you merge and close the pull request. It may be possible to split the pidevice object holding connections to several stages into individual objects right after connecting but it's probably not worth the effort.

BTW, PhysikInstumente suggested to import "future" to get the startup command running. That did actually not work...

nvladimus commented 3 years ago

Hi, @drchrisch Sounds good! I will do class renaming, config update, and code cleanup tomorrow, then test it again on our system. After that, it will be ready to merging into development, and I will close the PR.

nvladimus commented 3 years ago

Hi, @drchrisch, I pulled your changes in dev-test-hw-BIMSB#52 and tested them on our system, everything works on our side. You PR have been merged to development, with some minor modifications. I am closing this PR and #51. Thanks for your contribution!

drchrisch commented 3 years ago

Hi @nvladimus,

I'm afraid, the dev-test-hw-BIMSB#52 version tries hard to use the "mesoSPIM_PI_1toN" class and overwrites use of "mesoSPIM_PI_NtoN" class . Could you check this?

Thanks

Am Fr., 30. Apr. 2021 um 14:09 Uhr schrieb Nikita Vladimirov < @.***>:

Hi, @drchrisch https://github.com/drchrisch, I pulled your changes in dev-test-hw-BIMSB#52 and tested them on our system, everything works on our side. You PR have been merged to development, with some minor modifications. I am closing this PR and #51 https://github.com/mesoSPIM/mesoSPIM-control/pull/51. Thanks for your contribution!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mesoSPIM/mesoSPIM-control/pull/52#issuecomment-830049597, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKAXFMV4DQU7ZCUEJEAHL2DTLKMYLANCNFSM43O6X3OQ .

-- Christian Schulze

nvladimus commented 3 years ago

Oops, thanks for checking, I parsed stage_type parameter incorrectly. A fix is committed to development, please try again.

drchrisch commented 3 years ago

Right. I have to make some more changes to my class because I managed to kick out the tiling procedure. Sorry.

Am Fr., 30. Apr. 2021 um 16:54 Uhr schrieb Nikita Vladimirov < @.***>:

Oops, thanks for checking, I parsed stage_type parameter incorrectly. A fix is committed to development, please try.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mesoSPIM/mesoSPIM-control/pull/52#issuecomment-830150907, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKAXFMWO2BGXU7EKTT3CNH3TLLAC3ANCNFSM43O6X3OQ .

-- Christian Schulze

nvladimus commented 3 years ago

No worries. When you are ready, I can test on our system and then merge into development again.