Added prop(YES) to motorRecord.dbd

[kmpeters]

Added prop(YES) to the PREC field and fields that should update when the PREC field changes in motorRecord.dbd.

Phoebus users can add the following line to the Phoebus settings.ini file:

org.phoebus.pv.ca/dbe_property_supported=true

Changes to the PREC field will be immediately reflected in Phoebus displays, without having to close and reopen them.

[MarkRivers]

Added prop(YES) to the PREC field and fields that should update when the PREC field changes in motorRecord.dbd.

Are you sure it is necessary to add prop(YES) to the fields other than PREC? I see all fields in the motorx_all.bob screen change their precision when only PREC has prop(YES) in the dbd file.

I asked Michael Davidsaver what motor record fields need prop(YES), and this is his response:

> - What fields in the motor record would you suggest adding prop(YES) to?

Those whose values may be returned/copied out by one of these record support functions:

>     get_units
>     get_precision
>     get_enum_str
>     get_enum_strs
>     put_enum_str
>     get_graphic_double
>     get_control_double
>     get_alarm_double

[MarkRivers]

I just checked out the master branch where prop(YES) was added to many motor fields like VBAS, BDST, etc. It worked fine to update the precision of those fields when PREC was changed. I then removed the prop(YES) from most of those fields (but not from PREC) with this change:

corvette:motor/motorApp/MotorSrc>git diff .
diff --git a/motorApp/MotorSrc/motorRecord.dbd b/motorApp/MotorSrc/motorRecord.dbd
index 0a2ea8c..ea9650c 100644
--- a/motorApp/MotorSrc/motorRecord.dbd
+++ b/motorApp/MotorSrc/motorRecord.dbd
@@ -90,7 +90,6 @@ recordtype(motor) {
                 prompt("User Offset (EGU)")
                 special(SPC_MOD)
                 pp(TRUE)
-                prop(YES)
         }
         field(FOFF,DBF_MENU) {
                 asl(ASL0)
@@ -143,21 +142,18 @@ recordtype(motor) {
                 promptgroup(GUI_COMMON)
                 special(SPC_MOD)
                 interest(1)
-                prop(YES)
         }
         field(VBAS,DBF_DOUBLE) {
                 prompt("Base Velocity (EGU/s)")
                 promptgroup(GUI_COMMON)
                 special(SPC_MOD)
                 interest(1)
-                prop(YES)
         }
         field(VMAX,DBF_DOUBLE) {
                 prompt("Max. Velocity (EGU/s)")
                 promptgroup(GUI_COMMON)
                 special(SPC_MOD)
                 interest(1)
-                prop(YES)
         }
         field(S,DBF_DOUBLE) {
                 prompt("Speed (revolutions/sec)")
@@ -186,7 +182,6 @@ recordtype(motor) {
                 special(SPC_MOD)
                 interest(1)
                 initial("0.2")
-                prop(YES)
         }
         field(BDST,DBF_DOUBLE) {
                 prompt("BL Distance (EGU)")
@@ -194,14 +189,12 @@ recordtype(motor) {
                 promptgroup(GUI_COMMON)
                 special(SPC_MOD)
                 interest(1)
-                prop(YES)
-        }
+         }
         field(BVEL,DBF_DOUBLE) {
                 prompt("BL Velocity (EGU/s)")
                 promptgroup(GUI_COMMON)
                 special(SPC_MOD)
                 interest(1)
-                prop(YES)
         }
         field(SBAK,DBF_DOUBLE) {
                 prompt("BL Speed (RPS)")
@@ -216,7 +209,6 @@ recordtype(motor) {
                 special(SPC_MOD)
                 interest(1)
                 initial("0.5")
-                prop(YES)
         }
         field(FRAC,DBF_FLOAT) {
                 prompt("Move Fraction")
@@ -283,7 +275,6 @@ recordtype(motor) {
                 special(SPC_MOD)
                 pp(TRUE)
                 interest(1)
-                prop(YES)
         }
         field(ERES,DBF_DOUBLE) {
                 prompt("Encoder Step Size (EGU)")
@@ -291,14 +282,12 @@ recordtype(motor) {
                 special(SPC_MOD)
                 pp(TRUE)
                 interest(1)
-                prop(YES)
         }
         field(RRES,DBF_DOUBLE) {
                 prompt("Readback Step Size (EGU")
                 promptgroup(GUI_COMMON)
                 interest(1)
-                prop(YES)
-        }
+         }
         field(UEIP,DBF_MENU) {
                 prompt("Use Encoder If Present")
                 promptgroup(GUI_COMMON)

After removing prop(YES) from those fields Phoebus still updates those fields when PREC is changed. Here is a screen shot after I changed PREC from 3 to 7.

So I think prop(YES) should be removed from all of the fields except PREC and any others like EGU which Michael's comment would include.

Note that in the screen shot of motorx_all.bob above the DTYP and EGU fields are not displayed correctly. I have made minor modifications to motorx_all.adl, and now the conversion is correct:

I have pushed the .adl changes to Github.

[anjohnson]

If a record type provides a get_units() method in its RSET, that gets called to obtain the string for the units[] member of any of the dbr_gr_* or dbr_ctrl_* CA data types, and so on for the other methods that get other metadata from the record. The core IOC code has no idea where that method will fetch the units string from, it doesn't have to come from a field called EGU. The point of the prop(YES) flag is to tell the IOC code that the field thus marked provides metadata that gets fetched by one or more of those methods.

However there is no point marking fields that will not be accessed by any of those methods, that would just trigger DBE_PROPRETY events unnecessarily. In the case of the motor record, the get_units() method only reads the EGU field, so for that method EGU should be flagged. get_precision() only reads the PREC field, so that should be flagged. Following the same pattern for the remaining 3 methods while they do read from more fields, I don't see SBAS (which was flagged here) being used in any of them, thus I think Mark is right to question all the additions in this PR, there seem to be many more fields marked than you need.

[anjohnson]

Maybe as a reminder for future maintainers, you might add a comment to the DBD file for every prop(YES) line, listing the RSET method(s) that read this field. I note that EGU isn't actually marked but should be, thus I offer this change:

        field(EGU,DBF_STRING) {
                prompt("Engineering Units")
                promptgroup(GUI_COMMON)
                interest(1)
                prop(YES)       # get_units()
                size(16)
        }

[kmpeters]

I created a new pull req: https://github.com/epics-modules/motor/pull/204

@MarkRivers & @anjohnson, does the pull req look OK?

[MarkRivers]

This looks correct to me. Only fields that affect the values returned by the get_units, get_precision, get_control_double, get_graphuic_double, and get_alarm_double functions now have prop(YES).

[anjohnson]

Agreed — that looks better, and I like the annotation. If I had time I'd add that to all the Base record types too. A Codeathon project maybe…

[kmpeters]

Thanks for all the feedback. I really appreciate it.