Update position set function for Power PMAC Controllers

[jmagithub]

Adding Power PMAC position updating command for save/restore function to work properly.

[LeandroMartinsdS]

Hi @jmagithub

I've made some tests, and for me it seems that it will only work if the previous HomePos=0, that shouldn't be the case for the most part of applications, since it's set by the homing routines or by the reading of an absolute encoder.

For the Power PMAC, the queried position is calculated by the difference Motor[x].ActPos-Motor[x].HomePos. For changing the current value, as you suggested, we should change change the HomePos, but the ActPos need to be considered.

Also, in my opinion, scaling by 32 shouldn't be present for the PowerPMAC, since it makes sense just for TurboPMAC/PMAC2.

Said that, I suggest something like:

epicsInt32 position = (epicsInt32) floor(value  / pAxis->scale_ + 0.5);

if (cid_ == PMAC_CID_POWER_) {
  sprintf(command, "#%dK Motor[%d].HomePos=Motor[%d].ActPos-%d",
             pAxis->axisNo_,
             pAxis->axisNo_, pAxis->axisNo_, position);     
}
else{
  sprintf(command, "#%dK M%d61=%d*32*I%d08 M%d62=%d*32*I%d08",
          pAxis->axisNo_,
          pAxis->axisNo_, position, pAxis->axisNo_,
          pAxis->axisNo_, position, pAxis->axisNo_);
}

[JamesOHeaDLS]

Hi @jmagithub

Please could you give some more details about the problem you are seeing, and the steps needed to reproduce it?

Thanks,

James

[jmagithub]

Hi @LeandroMartinsdS

I did some tests with Motor[x].ActPos-Motor[x].HomePos, I couldn't set the .VAL field of the motor record to a value that less than the .OFF field value with .FOFF in frozen state. It seems no such restriction with only -Motor[x].HomePos, I can change .VAL .DVAL and .RVAL fields to the full range, not sure what causing that now.

I agree that the 32 scale should not be used in Power PMAC, the only reason it's used is because I'd like to make minimum changes to the source, since there is a 32 scale up in the position calculation epicsInt32 position = (epicsInt32) floor(value * 32 / pAxis->scale_ + 0.5); and that need to be scaled down in PPMAC.

Hi @jmagithub

I've made some tests, and for me it seems that it will only work if the previous HomePos=0, that shouldn't be the case for the most part of applications, since it's set by the homing routines or by the reading of an absolute encoder.

For the Power PMAC, the queried position is calculated by the difference Motor[x].ActPos-Motor[x].HomePos. For changing the current value, as you suggested, we should change change the HomePos, but the ActPos need to be considered.

Also, in my opinion, scaling by 32 shouldn't be present for the PowerPMAC, since it makes sense just for TurboPMAC/PMAC2.

Said that, I suggest something like:

epicsInt32 position = (epicsInt32) floor(value  / pAxis->scale_ + 0.5);

if (cid_ == PMAC_CID_POWER_) {
  sprintf(command, "#%dK Motor[%d].HomePos=Motor[%d].ActPos-%d",
             pAxis->axisNo_,
             pAxis->axisNo_, pAxis->axisNo_, position);     
}
else{
  sprintf(command, "#%dK M%d61=%d*32*I%d08 M%d62=%d*32*I%d08",
          pAxis->axisNo_,
          pAxis->axisNo_, position, pAxis->axisNo_,
          pAxis->axisNo_, position, pAxis->axisNo_);
}

[jmagithub]

Hi @JamesOHeaDLS ,

The issue I saw before was when Power PMAC controller power cycled and softioc restarted, the .VAL field was set to the .OFF field. I could not set the .VAL .DVAL and .RVAL fields with .SET field to "set" and .FOFF field to "frozen". There is no problem with previous Turbo PMAC controllers.

Hi @jmagithub

Please could you give some more details about the problem you are seeing, and the steps needed to reproduce it?

Thanks,

James

[LeandroMartinsdS]

@jmagithub

My first tests was just on the controller level, now I have tested further trying to replicate the issues that you're facing.

I had the impression of seeing similar problems when I forgot to add one more pAxis->axisNo_ to sprintf arguments.

I couldn't set the .VAL field of the motor record to a value that less than the .OFF field value with .FOFF in frozen state.

I spotted my mistake setting the DEBUG_LEVEL to 16, and then DEBUG_EXECUTE.PROC to 1. Maybe that helps to understand.

Testing Motor[%d].HomePos=-%d and initial conditions

Motor[1].ActPos = 10
Motor[1].HomePos = 0
VAL/DVAL/RVAL = 10
OFF = 0 

I got this:

BL45P-MO-TEST:M1.SET Use -> Set
BL45P-MO-TEST:M1.FOFF Variable -> Frozen
BL45P-MO-TEST:M1.VAL 10 -> 110
BL45P-MO-IOC-99 ->dbgf BL45P-MO-TEST:M1.VAL
DBR_DOUBLE:         120                 
BL45P-MO-IOC-99 ->dbgf BL45P-MO-TEST:M1.DVAL
DBR_DOUBLE:         120                 
BL45P-MO-IOC-99 ->dbgf BL45P-MO-TEST:M1.RVAL
DBR_LONG:           120

and in the controller:

Motor[1].ActPos
Motor[1].ActPos=10
Motor[1].HomePos
Motor[1].HomePos=-110

From my understanding, that happens because the HomePos is set to -VAL, but the RVAL that is read from the controller is ActPos-HomePos, which updated the VAL field

Testing with Motor[%d].HomePos=Motor[%d].ActPos-%d and the same initial conditions I got the following behaviour:

BL45P-MO-TEST:M1.FOFF Variable -> Frozen
BL45P-MO-TEST:M1.SET Use -> Set
BL45P-MO-TEST:M1.VAL 10 -> 110
dbgf BL45P-MO-TEST:M1.VAL
DBR_DOUBLE:         110                 
BL45P-MO-IOC-99 ->dbgf BL45P-MO-TEST:M1.DVAL
DBR_DOUBLE:         110                 
BL45P-MO-IOC-99 ->dbgf BL45P-MO-TEST:M1.RVAL
DBR_LONG:           110       0x6e    

[EDIT: Fix typo in HomePos]

Motor[1].ActPos
Motor[1].ActPos=10
Motor[1].HomePos
Motor[1].HomePos=-100

[jmagithub]

@LeandroMartinsdS

Thanks for the detailed test.

My testing result aligns with yours when test with Motor[%d].HomePos=-%d. There is a slight difference when I tested with Motor[%d].HomePos=Motor[%d].ActPos-%d. I got HomePos=-100 instead of HomePos=-110, that's probably due to a typo.

I agree that ActPos should be included in the formula. () is needed for the position value to avoid problem when it's negative, the modified command line will be Motor[%d].HomePos=Motor[%d].ActPos-(%d), this also explains the problem I had before.

I make another commit to the pull request, please review.

[LeandroMartinsdS]

@JamesOHeaDLS @ajgdls,

Anything to add before we proceed with merging this pull request?