Add getLimits functions to interfaces

[ericonr]

Right now, if we want to set value limits in records (e.g. DRVHand DRVL) based on information queried from the a device, it's necessary to create auxiliary parameters, then create records for those parameters which set the relevant fields for the original record. That has to be repeated for every record we wish to have limits for.

If the driver has that information, it could set it during initialization, and (possibly) change it during runtime. For initialization, it would be necessary to add a getLimits function to (at least) asynInt32, asynUInt32Digital, asynInt64 and asynFloat64 interfaces, and use them in the init* functions for relevant records (e.g. longout for asynInt32; supporting ao would require converting the integer limits, which might be error prone).

Changing the limits during runtime might be more complicated, as it would require changes to paramVal to store those limits (and developers would have to guarantee that the values returned by getLimits are also stored in the parameter library), or some new kind of callback routine. That would be a separate design step, and I would welcome help.

This is a suggestion, which I believe would simplify driver development and improve the interface to those drivers; it makes it easier to clamp values right at the record layer, instead of having to return errors to out-of-bounds put operations.

Is there interest in something like that, if implemented in an adequate manner? I would like to gather opinions before trying to implement it.

[anjohnson]

There will be some cases where the DRVH+DRVL values from the loaded database file should not be changed by the driver. If this gets implemented, the ability of the driver to override the DB designer's settings should have to be explicitly enabled, maybe using an info tag. That might also allow a way for the DRV fields to be calculated from the driver's limits (e.g. accept a margin parameter which gets added to the driver low and subtracted from the driver high value).

[MarkRivers]

I have a couple of concerns about this.

The first is breaking backwards compatibility of asyn drivers, particularly those using the low-level C interface, rather than asynPortDriver.

Here is an example: https://github.com/epics-modules/motor/blob/752696e08f07251119c1d37ff0d4d8c9775489b0/motorApp/MotorSrc/drvMotorAsyn.c#L254

which is:

static asynFloat64 drvMotorFloat64 = {
    writeFloat64,
    readFloat64
};

If a new getLimits() method is added to the asynFloat64 interface all drivers that use it will need to modify declarations like this. In this case since the structure is "static" the getLimits function pointer will be initialized to 0. That will of course need to be tested in all asyn code that wants to call getLimits(). But if this structure had been an automatic variable would the getLimits function pointer be uninitialized, or would this be a compiler error? If it is uninitialized then the asyn code would deference an illegal pointer. I don't know how many occurrences of such automatic structure initialization there might be.

My second concern is with how useful this feature will be in the real world. The driver may know about what the device limits are, but often the desired limits are tighter. What types of drivers are you thinking of where this would be useful?

[ericonr]

@anjohnson I was thinking the getLimits function should only be called when drvh and drvl are both zero; that way any manual setting from the database won't be overridden. But if we want more flexibility, I guess the info tag would be necessary (I don't fully understand the margin suggestion, can you expand on it?)

@MarkRivers

But if this structure had been an automatic variable would the getLimits function pointer be uninitialized, or would this be a compiler error? If it is uninitialized then the asyn code would deference an illegal pointer. I don't know how many occurrences of such automatic structure initialization there might be.

Declaring the interface in automatic storage can be done in a way that's safe or not, as shown below.

// safe, all other members are zero-initialized
asynFloat64 drvMotorFloat64 = {
    writeFloat64,
    readFloat64
};

// unsafe, all other members might have garbage in them
asynFloat64 drvMotorFloat64;
drvMotorFloat64.writeFloat64 = writeFloat64;
drvMotorFloat64.readFloat64 = readFloat64;

I suppose it's more likely for a function constructing interfaces at runtime to use dynamic storage rather than automatic; if they use malloc, there's danger, if they use calloc, there's not. And up until now, malloc would have been fine (if not future-proof).

It doesn't seem very wieldy to construct interfaces at runtime; have you ever seen any drives which do it?

My second concern is with how useful this feature will be in the real world. The driver may know about what the device limits are, but often the desired limits are tighter. What types of drivers are you thinking of where this would be useful?

I have seen this before with controller gains represented as fixed point , where the representation of the fixed point value is read from the device, so we only know its range at runtime. We don't have actual operation limits, but representation limits.

Similarly, basically any situation where a value is an n-bit register field, meaning its value is constrained to fit inside that field.

This is also useful when utilizing PyDM spinboxes, which require lower and upper limits for variables they are writing to.

[tboegi]

@ericonr Is this about motors ? We have been using a patch to allow the driver to set "Read only limits" into the motorRecord. That is, the driver tell the record that e.g. the "driving range" is from 5.0mm..175.0mm Those values then go into DLLM and HLLM. When the user tries to set DLLM to something outside e.g. 4.0mm the record will refuse this. Restricting the driving range further is allowed, like 10mm..150mm. This functionality has existed (and is still there) for some controller cards not using asyn, but never for motor controllers using asyn.

The motorRecord code is kind of here:

https://github.com/EuropeanSpallationSource/motor/blob/ess-master/motorApp/MotorSrc/motorRecord.cc#L844

The the driver itself sets these values after having talked to the controller: https://github.com/EuropeanSpallationSource/m-epics-ethercatmc/blob/master/ethercatmcApp/src/ethercatmcController.cpp#L843

[ericonr]

It is not about motors, though it seems to be a similar intention.

[anjohnson]

@ericonr Regarding "margin" I was adapting the idea of the cascading-style-sheets properties margin, margin-left and margin-right. The user could specify a single value (margin=0.5 say) which would be added to the lower limit from the driver and subtracted from its upper limit to come up with the DRVL+DRVH field values. If they wanted different offsets they could specify margin-low and/or margin-high for the two. A more specific setting should replace the general one (not be added to it), so if both margin and margin-low were given margin would be used for the DRVH calculation and margin-low for DRVL.