Skip to content
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

Add onStagePositionChanged callback to TE2000 adapter #188

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ianhi
Copy link
Contributor

@ianhi ianhi commented Apr 28, 2022

I did this based on the Demo camera though not entirely sure that this is the right place to put this. Should something also go into the setStagePositionSteps? If this is correct I'll add the same thing to the PFS offset.

(I hvaen't tested this because I've never really approached building this on windows)

@ianhi
Copy link
Contributor Author

ianhi commented Apr 28, 2022

(I hvaen't tested this because I've never really approached building this on windows)

@marktsuchida any chance we could have an action that runs on PRs here that creates teh deviceadapters that I could download as an artifact in order to test?

@marktsuchida
Copy link
Member

Did you mean to do this for PFSOffset and not the FocusStage?

For the PFSOffset, you will find that SetPositionUm delegates to the property (OnPosition), which in turn delegates to SetPositionSteps. (Unlike most Z stages, the PFSOffset exposes its "position" as a property as well.)

So, in theory, SetPositionSteps (the common code path) should be the (only) one issuing the callback. However, knowing that the callback expects "micrometers", and that SetPositionSteps is never called by MMCore (and will never be called as long as I'm maintaining it, because we can't trust all stages to have implemented the never-tested SetPositionSteps correctly), it's probably okay to issue the callback from the property handler (OnPosition).

It would be good to test this on hardware if possible -- to make sure that there is no deadlock or infinite loop hiding in the interaction between property browser, stage control, etc.


Building single device adapters on Windows is not that hard, especially for adapters that don't have any special dependencies outside of 3rdpartypublic.

@marktsuchida any chance we could have an action that runs on PRs here that creates teh deviceadapters that I could download as an artifact in order to test?

Yet again, something that will be easy once we reorganize things into individual repos (and use GitHub Actions for build). In fact, in my mind this is one of the major motivations for such a reorganization: run the build and tests before merging.

It's hard now, because we are triggering Jenkins based on pushes to main of the micro-manager repo. Triggering this from PRs on mmCoreAndDevices seems too complicated to be worthwhile. Also, blind building of PRs is a security risk unless specific measures are taken, and those measures are hard to take under the current setup. (I'd set it up with a manual approval step if that were easy.)

We do now have downloadable installers after merge (so you don't need to wait for the next nightly build): https://download.micro-manager.org/ci/2.0/Windows/

@ianhi
Copy link
Contributor Author

ianhi commented Apr 29, 2022

It would be good to test this on hardware if possible -- to make sure that there is no deadlock or infinite loop hiding in the interaction between property browser, stage control, etc.

This is why I was hopeful of automated builds. Unfortunately the only windows computer I have access to is the one that runs this shared scope, so I'm a bit wary of mucking around with installing from source and potentially leaving things in a non-working state

@marktsuchida
Copy link
Member

Hmm. Not sure I have a good solution other than using a VM (still needs install of Visual Studio C++ Desktop Development) or a cloud instance.

I think at least for now having a local build (seconds to build a single device adapter) is a good idea for dev/testing, because an automated build of full Micro-Manager takes 15–20 min even if we come up with some workaround to build PRs (should be much faster when each device adapter has a GitHub Actions build).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants