Fixed RVEL bug

[StefanMathis]

This PR fixes https://epics.anl.gov/tech-talk/2026/msg00212.php by writing the motor velocity into status_.velocity, where it is later read from when the motorRecord fields are populated.

[tboegi]

While the fix looks correct as such, it may be useful to separate the actual velocity
of the motor from the commanded velocity.
We do read back the "commanded velocity" from the controller,
which may slightly differ from what the IOC wants.
Having the actual velocity in a different asyn parameter would all
to both look at the actual, measured velocity and the commended in the MCU

[kmpeters]

@tboegi, the data structure only has one velocity element, which is clearly intended for the readback velocity:

motor/motorApp/MotorSrc/asynMotorController.h

Lines 105 to 110 in 0cb7d76

	typedef struct MotorStatus {
	double position; /**< Commanded motor position */
	double encoderPosition; /**< Actual encoder position */
	double velocity; /**< Actual velocity */
	epicsUInt32 status; /**< Word containing status bits (motion done, limits, etc.) */
	} MotorStatus;

This PR looks good to me.

[tboegi]

@kmpeters : Isnt pC_->motorVelocity_ the setpoint for the move ?

getDoubleParam(axis, motorVelBase_, &baseVelocity);
getDoubleParam(axis, motorVelocity_, &velocity);
getDoubleParam(axis, motorAccel_, &acceleration);
status = pAxis->move(value, 1, baseVelocity, velocity, acceleration);

When we mix setpoint with readback, the following can happen:
The user writes to the VAL field of the motorRecord.
The drivers "sends" 4 "messages" to the model 3 dirver:

1.        WRITE_MSG(SET_VELOCITY, &velocity);
   

2.       WRITE_MSG(SET_VEL_BASE, &vbase);
   

3.       WRITE_MSG(SET_ACCEL, &accel);
   

4.       WRITE_MSG(MOVE_ABS/REL, &position);
   

5.        WRITE_MSG(GO, NULL);
   

If the driver/poller writes 0.0 to motorVelocity_ between 1) and 2) we have a problem:
The move() will be called with velocity == 0.0

[MarkRivers]

@tboegi I understand your concern. However, motorPosition_ and motorEncoderPosition_ are treated identically to how motorVelocity_ is treated in this PR. They are used to SET the position and encoder position here:

motor/motorApp/MotorSrc/asynMotorController.cpp

Line 359 in 32fe1b1

} else if (function == motorPosition_) {

and here:

motor/motorApp/MotorSrc/asynMotorController.cpp

Line 366 in 32fe1b1

} else if (function == motorEncoderPosition_) {

But they are also used to read back the actual positions, for example here:
https://github.com/epics-motor/motorAcsMotion/blob/0217e915adfafeb7983d3ef4529f7a77fd8fedb0/acsMotionApp/src/SPiiPlusDriver.cpp#L759
and here:
https://github.com/epics-motor/motorAcsMotion/blob/0217e915adfafeb7983d3ef4529f7a77fd8fedb0/acsMotionApp/src/SPiiPlusDriver.cpp#L764

How is motorVelocity_ different from this?

I agree that it would be cleaner to have separate parameters for the "set" values and the "readback" values. But to do that for motorPosition_ and motorEncoderPosition_ I believe would break backwards compatibility for all model 3 drivers.

[StefanMathis]

@tboegi's comment is absolutely valid and I did think about introducing a motorSetVelocity_ entry - but did refrain from that due to the backwards compatibility issues @MarkRivers mentioned. In my tests, I had no issues because I only write to motorVelocity_ when the motor is already moving, therefore avoiding the race condition.

To mitigate this issue, maybe we could document somewhere that the field should only be written to when the motor is moving (e.g. in the header of asynMotorController.h next to the index declaration)? In general, it would probably be helpful to document for asynMotor which paramLib entry interacts with which field(s). For example, what is the difference between motorResolution_ and motorRecResolution_ and which one is MRES?

[tboegi]

I think that motorSetVelocity_ would be the same as motorVelocity_ : it is the setpoint.
We can introduce motorActVelocity_ for this (and fiddle that into status.velocity)

[tboegi]

@kmpeters @MarkRivers @StefanMathis
Please have a look at
#238