user/dial limits: change when motor/encoder resolution changes

[rerpha]

Currently the motor record does nothing with dial/user limits when a motor encoder resolution changes.

this is potentially problematic if a device stores soft limits on the controller (in our case at ISIS/STFC a Galil) which are not scaled (ie in steps), as the motor record does not re-scale these values by calling set_dial_highlimit etc.

I propose that, when changing the motor/encoder resolution, either:

• we re-scale the soft limits based on the scaling factor of the motor resolution change, so that an MRES change from 0.05 to 0.1 means a dial low limit of -10 becomes -20 (or the other way round, my maths might be wrong?) - currently we re-scale the VELO, VMAX, BVEL, VBAS and update them according to the new motor resolution, but this is easy because they are all derivative of mres * something else which is not scaled
• we re-send the limits to the device, but scale them using the new resolution. this means the fields stay the same EPICS side but are re-adjusted on the device.

The former feels like it would require caching the old mres so we can compare against it when trying to rescale the limits.
I think the latter would just be a case of calling set_dial_{high, low}limit() and then set_user_limits from motorRecord.cc

[kmpeters]

I prefer the second of the two proposed options, as I expect it would result in fewer potential problems.

I also think a cleaner approach would be to add raw limit fields, analogous to the raw position fields, that would allow appropriately-scaled limits to be sent to the driver without keeping track of the previous MRES.

[kmpeters]

I also think a cleaner approach would be to add raw limit fields, analogous to the raw position fields, that would allow appropriately-scaled limits to be sent to the driver without keeping track of the previous MRES.

Specifially, RHLM and RLLM fields could be added, with the following relationships to DHLM and DLLM:

RHLM = DHLM / MRES
RLLM = DLLM / MRES

The raw limits would update and be consistent with the dial limits. If the raw limits are changed, the dial limits would be recalulcated using the current MRES. If the MRES is changed, the dial limits would be recalculated based on the the raw limits, along with the velocities.

[rerpha]

yes, that seems a better solution. I think I prefer that as it follows what we currently do with scaled velo etc.
I think this could potentially simplify device support as well, as no scaling will be needed outside of the motor record for setting the limits on a controller.

[rerpha]

as a (very) rough proof of concept this seems to work...

https://github.com/ISISComputingGroup/EPICS-motor/compare/rawlims?expand=1

as this is the first time i've looked at the motor record i'm not super confident! will test some more and make a PR when happy

[tboegi]

If I may join the discussion:
Taking a short look:
I think that updating the fields RLLM and RHLM needs review.
Are we are missing the calculation of LLM and HLM here ?
And what happens when MRES < 0 ?
We need to make sure that LLM/HLM, DLLM/DHLM and the new RLLM/RHLM
always stay synchronized, right ?
And in that sense:
The introduction of set_user_highlimit() and set_user_lowlimit() are a good change.

However, synching of Raw/Dial/User limits seems incomplete.
It may be easier, to keep RLLM/RHLM as "read only", at least to start with.
Does somebody wants to write to them ?
Or are they only needed internally ?

[rerpha]

many thanks for having a look and giving feedback! RLLM/RHLM being read-only to the outside world is fine I think. Have just committed a change to make them read-only, hopefully it's correct. edit: think i needed to add SPC_NOMOD in the dbd as well?

MRES < 0 currently applies *-1 to both lims, not sure if this is correct or not

looks like dhlm and dllm are not affected by DIR, only llm and hlm are - i think it makes sense for RLLM and HLLM to also not be affected?

[kmpeters]

MRES < 0 currently applies *-1 to both lims, not sure if this is correct or not

This is correct.

looks like dhlm and dllm are not affected by DIR, only llm and hlm are - i think it makes sense for RLLM and HLLM to also not be affected?

This is also correct.

[kmpeters]

However, synching of Raw/Dial/User limits seems incomplete.

@tboegi, I don't understand. How would the syncing be incomplete?

[tboegi]

My first impression, after reading all the changes, was that RLLM and RHLM are writable.
And if somebody writes to them, the synching into DLLM and LLM / DHLM HLM may
be wrong because we may need to look at MRES...
(But I may be wrong)

However, I wonder, if a simple
set_dial_highlimit(pmr, pdset);
set_dial_lowlimit(pmr, pdset);
At the end of
velcheckB:
would be enough ?

[tboegi]

@rerpha I think that this patch may solve your issue ?
https://github.com/tboegi/motor/pull/new/update-raw-limits-on-MRES-change

[rerpha]

i think so - its just whether we want to update on the controller or update values on our end instead?

[tboegi]

What is our end ?

[rerpha]

DHLM/DLLM/HLM/LLM being updated instead of new limit being pushed to controller

[rerpha]

I think the former now works on my branch, with raw lims read-only? just needs some tidying up

[tboegi]

I see: I did the wrong solution.

[rerpha]

have made #193 , feedback welcomed!