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

Moving stage at set velocity #351

Open
danish-islam opened this issue May 31, 2023 · 12 comments
Open

Moving stage at set velocity #351

danish-islam opened this issue May 31, 2023 · 12 comments

Comments

@danish-islam
Copy link

After a discussion on the image.sc forum, I was advised to open an issue to have the int Move(double vx, double vy) = 0; function in the API implemented for stages.

@nicost
Copy link
Member

nicost commented May 31, 2023

Thanks @danish-islam. What about:

void moveXY(double vx, double vy); // speed in microns / second
void moveXY(const char *xyStageLabel, double vx, double vy); // speed in microns / second

These will likely not be implemented by most (all?) stages, but this would give us incentive to start implementing them.
Clearly, these should not be blocking. OK to go ahead with these @marktsuchida and @henrypinkard?

@marktsuchida
Copy link
Member

@marktsuchida
Copy link
Member

I'm okay with putting these in the MMCore API, but would choose a name that is very explicit about what it does. moveXY might be mistaken for moving to a position (or a relative move) and cause a bad accident. For the same reason it might be better to only expose the version that takes the xyStageLabel.

Maybe startXYMoveWithVelocity(xyLabel, vx, vy)?

(Nitpick: I wouldn't characterize these as non-blocking: they may wait for an acknowledgment from the device before returning. ASITiger, for example, does this. But of course they shouldn't block beyond that.)

One slightly bad thing that could happen is that there may be existing stages that implement Move(), but incorrectly, and now the up-to-now-hidden incorrect implementation could be exposed. I didn't check all stages, but it looks like at least some override the method to return DEVICE_OK (whereas the DeviceBase default returns DEVICE_UNSUPPORTED_COMMAND). It would be good to review all existing implementations.

git grep -nP 'int *Move *\('
ASITiger/ASIDacXYStage.h:63:	int Move(double vx, double vy);
ASITiger/ASIXYStage.h:61:   int Move (double vx, double vy);
CNCMicroscope/RAMPSStage/XYStage.h:65:  int Move(double /*vx*/, double /*vy*/);
Conix/Conix.h:173:	int Move(double /*vx*/, double /*vy*/) {return DEVICE_OK;} // ok
Conix/Conix.h:237:	int Move(double /*vz*/) {return DEVICE_OK;} // ok
DemoCamera/DemoCamera.h:498:   int Move(double /*v*/) {return DEVICE_OK;}
DemoCamera/DemoCamera.h:578:   int Move(double /*vx*/, double /*vy*/) {return DEVICE_OK;}
LeicaDMR/LeicaDMR.h:191:   int Move(double speed);
LeicaDMSTC/LeicaDMSTC.h:102:   //int Move(double speedX, double speedY);
Marzhauser-LStep/LStep.h:90:	int Move(double vx, double vy);
Marzhauser-LStep/LStep.h:149:   int Move(double velocity);
Marzhauser/Marzhauser.h:90:int Move(double vx, double vy);
Marzhauser/Marzhauser.h:147:   int Move(double velocity);
Marzhauser/Marzhauser.h:209:   int Move(double velocity);
NewportCONEX/Conex_Axis.h:109:   int Move(double velocity);
NewportCONEX/Conex_Axis.h:147:   int Move(double velocity);
NewportCONEX/Conex_Axis.h:184:   int Move(double velocity);
ObjectiveImaging/OasisControl.h:215:      int Move(double velocity);
PIEZOCONCEPT/PIEZOCONCEPT.h:110:   int Move(double /*v*/) { return DEVICE_UNSUPPORTED_COMMAND; }
PIEZOCONCEPT/PIEZOCONCEPT.h:181:   int Move(double /*vx*/, double /*vy*/) { return DEVICE_OK; }
SouthPort/microz.h:50:	int Move(double /*v*/);
Standa8SMC4/Standa8SMC4.h:49:   virtual int Move(double velocity);
Standa8SMC4/Standa8SMC4.h:92:   virtual int Move(double vx, double vy);
StandaStage/StandaStage.h:115:   int Move(double /*v*/) {return DEVICE_OK;}
StandaStage/StandaStage.h:265:   int Move(double /*vx*/, double /*vy*/) {return DEVICE_OK;}
Tofra/Tofra.h:112:	int Move(double speed);
Tofra/Tofra.h:191:	int Move(double speedx, double speedy);
TriggerScope/TriggerScope.h:347:   int Move(double /*v*/) {return DEVICE_OK;}
Utilities/Utilities.h:210:   virtual int Move(double) { return DEVICE_UNSUPPORTED_COMMAND; }
Utilities/Utilities.h:268:   virtual int Move(double /* vx */, double /* vy */ ) { return DEVICE_UNSUPPORTED_COMMAND; }
Utilities/Utilities.h:327:   virtual int Move(double) { return DEVICE_UNSUPPORTED_COMMAND; }
WieneckeSinske/ZPiezoCanDevice.h:70:    int Move(double /*v*/) {return DEVICE_OK;}
WieneckeSinske/ZPiezoWSDevice.h:70:    int Move(double /*v*/) {return DEVICE_OK;}
Zaber/Stage.h:52:	int Move(double velocity);
Zaber/XYStage.h:47:	int Move(double vx, double vy);

@henrypinkard
Copy link
Member

I'm okay with putting these in the MMCore API, but would choose a name that is very explicit about what it does. moveXY might be mistaken for moving to a position (or a relative move) and cause a bad accident. For the same reason it might be better to only expose the version that takes the xyStageLabel.

Maybe startXYMoveWithVelocity(xyLabel, vx, vy)?

I agree--calling it moveXY would be confusing. What exactly is this function doing? Is it setting the speed for a subsequent call to setposition? Or does this function call actually initiate the move (and if so, how does it know where to move to)?

If its the former, I'd suggest setXYMoveVelocity. If the latter than I'm confused...

@nicost
Copy link
Member

nicost commented Jun 1, 2023

What about:

/**
 * Start moving the given XY stage at the velocity (and direction) given by the parameters (in microns per second).
 *
 * xyStageLabel - name of the XY stage that should start moving.
 * vx - velocity in x direction (in microns per second, can be negative to reverse direction)
 * vy - velocity in y direction (in microns per second, can be negative to reverse direction)
 */
void startXYMoveWithVelocity(const char *xyStageLabel, double vx, double vy); 

Do we need a stopMove? or does calling the above function with 0, 0 do the job?

@danish-islam
Copy link
Author

danish-islam commented Jun 1, 2023

Yes, the above description from @nicost is exactly what I was thinking about in my image.sc post.

@marktsuchida
Copy link
Member

Do we need a stopMove? or does calling the above function with 0, 0 do the job?

There is already a stop() which works for both XY and Z stages. Calling with 0, 0 should also work (and is the default implementation of Stop() in DeviceBase; see comment there).

@danish-islam
Copy link
Author

danish-islam commented Jun 15, 2023

I was looking through the CMMCore documentation and I wanted some feedback for if what I found is helpful for this topic.

If you were to do:
core.setSerialPortCommand(trackerXYStagePort, velocityCommand, "\r");

where:
velocityCommand = "VECTOR X=" + String.valueOf(xVelocity) + " Y=" + String.valueOf(yVelocity);
trackerXYStagePort = core.getProperty(stageDeviceLabel, "Port");

Would this result in the stage beginning to move with this velocity or is it simply setting the drive speed?

Referring to this CMMCore documentation

@marktsuchida
Copy link
Member

@danish-islam You might be able to hack something like that, but it will probably be fragile because the communication with the stage is not necessarily in a known state when you send the command. You are basically injecting a message without the device adapter's knowledge, and things will go very wrong if the device adapter is trying to talk to the stage (or waiting for a reply to another command) at the same time. (At the very least, you should read the response to the command you send, so that the device adapter doesn't think it is the reply to whatever command it sends next.)

@danish-islam
Copy link
Author

Right, I see. So if I'm understanding this correctly if you ask the micromanager core to do two things at once it will break. (E.g. snap image and start moving at a certain velocity).

@marktsuchida
Copy link
Member

No, it won't break if you ask it to do two things at the same time in different devices (such as a camera and XY stage). It is also usually safe to do two things in the same device (in hardware), as long as it goes through the same device (as known to MMCore) -- they will either run in parallel or one after the other, but should be handled correctly.

The problem is when you have one stage device, and two uncoordinated pieces of code (your script and the stage device adapter) talking to it at the same time.

@jondaniels
Copy link
Contributor

I am OK with updating the device-level API to add startXYMoveWithVelocity() or setXYMoveVelocity(). I lean slightly towards the former because it is clear that the stage will start moving immediately.

While we are augmenting the device-level API can we also add a setMaxSpeed() at the same time? I've wished this were an API call instead of a property. (Feel free to propose a different name, maybe XY should be part of it?)

@danish-islam, if you have an ASI stage -- the only type I know of to actually allow these "vector moves" then you can make use of the existing properties VectorMoveX-VE(mm/s) and VectorMoveY-VE(mm/s) that exist in the ASIStage and ASITiger device adapters.

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

No branches or pull requests

5 participants