-
Notifications
You must be signed in to change notification settings - Fork 49
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 MINP for soft motors #211
base: master
Are you sure you want to change the base?
Add MINP for soft motors #211
Conversation
@tboegi, I switched the master branch to use the sequencer mirror. If you rebase this pull request on the master branch, the CI builds should succeed. |
a179d57
to
b994981
Compare
Make it possible to set bits of the MSTA field for soft motors. Add a new input link, called MINP, beside the existing DINP and RINP MINP will read the new MSTA bits, but filter out the RA_DONE bit. The RA_DONE bit will be taken from DINP (as before) and the RA_DONE bit inside MINP will be ignored, preserving the RA_DONE coming from DINP Note that the MINP needs to follow the bit-definition in MSTA. It may update RA_PLUS_LS, RA_MINUS_LS. RA_HOMED or other bits may be set as well, whatever the application needs. MINP may be pointed to the output of a calc or transform record.
b994981
to
2dc3101
Compare
2023-12-11T07:45:00.0015590Z File "/Users/runner/work/motor/motor/.ci/cue.py", line 14, in |
I'm very confused. Why did the build of the master branch succeed on OS X: https://github.com/epics-modules/motor/actions/runs/7144493328 While the build of this branch failed: https://github.com/epics-modules/motor/actions/runs/7164362343 With the same |
I might just ignore the CI failure for now. I've seen weird errors on pull requests that weren't problems on the master branch. |
@kmpeters I am confused as well. More digging needed. |
Pedro Nariyoshi reports that these changes work well in his test setup: |
I'm testing this pull request with using the following databases in the motorMotorSim example IOC, which implements the soft motor example from the motor documentation and links an mbboDirect record to the MINP field of the soft motor: SoftMotorExample.db.txt I'm using the first simulated motor as the "real" motor:
I'm able to set the high hardware limit by doing this: I observe the following:
|
About the STOP: I need to check, if we want to have this functionality ? @kmpeters The homed bit is bit 14, not 15.
14 unsigned int RA_HOMED :1; /* Axis has been homed./ |
@kmpeters Thanks for testing I just realize that the .MSTA field is declared as So I think that the PR needs improvements ! |
I don't understand the use-case for the MINP field, if we don't want the actual motors to stop when a soft-motor's hardware limit is activated. |
The original motivation comes from a soft-slit database: The STOP function is another nice feature. |
If we add a STOP feature it should definitely be optional. Presumably the underlying motor controller or PLC will handle stopping on a limit switch. Additional software STOPs can cause issues with motion programs implemented under the feet of the motor record. |
Yes, the more I think about it, the more I am tempted to say that a STOP function could and should be implemented in |
I'll do some testing with an external stop calcout record. |
This updated
Every move that is made while the hardware limit of the soft motor is active is stopped, but the motor is able to move small amounts that would, in theory, allow moving off of the limit switch. |
@kmpeters |
@tboegi, I believe you understand it correctly. The motor record checks the hardware limit states in the commanded direction of motion for retries, jogging and homing, but not normal moves. This is not likely a problem for real motor controllers, nor is it a problem for simulated motors, because the controllers know there is a limit violation and the move command that is sent should result in an error. The soft-channel device support, however, does not know there is a limit violation and it unconditionally tells the linked motor to move: motor/motorApp/SoftMotorSrc/devSoft.cc Lines 163 to 166 in f8a3239
I think adding a check for a hardware limit violation and returning an error instead of moving the linked motor in devSoft.cc is a better solution than adding checks for the hardware limit states before moves in motorRecord.cc. I would expect hardware limit enforcement in the motor record to cause unable-to-move-off-of-limit problems, because of the delay polling the motor's status. |
Implementing hardware-limit enforcement in the soft-channel device support will likely require the direction bit to be set properly, which is also not implemented. |
When implementing the MINP (MSTA input) it was overseen, that MSTA is defined as 32 bit (where only 15 bits are used). Correct this: use long instead of short.
Add the MINP field: It is "similar" to e.g. DINP
Make it possible to set bits of the MSTA field for soft motors. Add a new input link, called MINP, beside the existing DINP and RINP
MINP will read the new MSTA bits, but filter out the RA_DONE bit. The RA_DONE bit will be taken from DINP (as before) and the RA_DONE bit inside MINP will be ignored, preserving the RA_DONE coming from DINP
Note that the MINP needs to follow the bit-definition in MSTA. It may update RA_PLUS_LS, RA_MINUS_LS.
RA_HOMED or other bits may be set as well, whatever the application needs. MINP may be pointed to the output of a calc or transform record.