epics-modules / motor

APS BCDA synApps module: motor
https://epics-modules.github.io/motor/
20 stars 47 forks source link

Q: What should happen when JOGR and JOGF are active #170

Open tboegi opened 3 years ago

tboegi commented 3 years ago

May be more a question than an issue. From a partner lab I got the following question:

  1. User set JOGR to 1
  2. User sets JOGF to 1 (without resetting JOGR)
  3. Motor stops
  4. Set JOGF to 1 again -> nothing happens
  5. Set JOGF=1 again -> motor moves backward

From the code we can see, that the motor is commanded backwards if JOGR is set (pmr->jogr), regardless of JOGF.

My understanding is, that most GUIs don't allow JOGF and JOGR to be active at the same time - problem solved. Beside that, is there a good way to make the motorRecord more better-behaved in this scenario? I have some option in mind: a) The latest JOGX command "wins". When JOGR is activated, JOGF is reset, if needed. And vice versa b) The first JOGX command wins. The second one is ignored. c) Both JOGR and JOGF are reset, motor is stopped.

Are there any opinions (before I start the coding) ?

kmpeters commented 3 years ago

I think that it makes sense to take into account the New Target Monitor field (NTM) of the motor record: https://epics.anl.gov/bcda/synApps/motor/motorRecord.html#Fields_misc If NTM is YES, the JOGF=1 overrides the previous JOGR=1, which involves stopping the reverse jog, then initiating the forward jog. If NTM is NO, the JOGF=1 should be delayed until the JOGR=1 is complete.

tboegi commented 3 years ago

Thanks for thinking about that. There are 2 questions:

kmpeters commented 3 years ago

I can see why applying NTM to jogs could be surprising to a user.

If I was the user and I issued a JOGF=1 while the JOGR=1 is in progress, I would expect the JOGF=1 to override the the JOGR=1. Stopping the motor and executing the latest jog seems very reasonable.

One thing to keep in mind is that it is possible to put the motor record into a state where both JOGF and JOGR are set to one at the same time, using NPP output links from other records in the IOC. I don't know what the behavior should be when the record processes in that case.

anjohnson commented 3 years ago

@kmpeters You could prevent both JOGF and JOGR from being set simultaneously by making those fields special(<something>) and having the record's special() routine enforce whatever you decide the behavior should be; special() gets called immediately before and after the field is changed, irrespective of whether the record will get processed, and it knows what field is being written to. I might expect last-put-wins in the above case, but you might choose opposite-cancels-jog instead if that makes more sense.

tboegi commented 3 years ago

@anjohnson, @kmpeters: Something in that style ? https://github.com/EuropeanSpallationSource/motor/commit/731571ec6a81296fcf30f27a4874b32fa638d117

I can rebase it onto the master branch and make a proper PR

anjohnson commented 3 years ago

I don't claim to know whether all the MIP bit manipulation is correct there, but your suggestion looks like it should implement the last-put-wins behavior. I also don't know whether that's appropriate given whatever the NTM field is supposed to do but it sounds like @kmpeters isn't too worried about that. You two are the subject matter experts here, I'm just giving you one possible way to implement stuff.

tboegi commented 3 years ago

As I understand it, we don't mess up the MIP:

https://github.com/epics-modules/motor/pull/171/commits/c6359ddb1fee8d5b2f203cad93b2219bd3a0ae68

kmpeters commented 3 years ago

@anjohnson, @kmpeters: Something in that style ? EuropeanSpallationSource@731571e

I can rebase it onto the master branch and make a proper PR

This looks good to me.

tboegi commented 3 years ago

The master version can be find here. As usual, please double check https://github.com/epics-modules/motor/pull/171