epics-modules / motor

APS BCDA synApps module: motor
https://epics-modules.github.io/motor/
20 stars 47 forks source link

Add raw limits so that soft limits are synced with motor resolution change #193

Closed rerpha closed 1 year ago

rerpha commented 1 year ago

Fixes #191

rerpha commented 1 year ago

hi, was just wondering if you had a chance to review this? Cheers

kmpeters commented 1 year ago

I plan to test this soon. I want to include it in the next motor release.

kmpeters commented 1 year ago

I'm testing this pull request with simulated motors. When I change a dial limit, a camonitor shows the dial limit updates twice (both times with the same value) and the raw limits don't update.

kmpeters commented 1 year ago

The double-camonitor-entries-for-the-limit-being-changed behavior is apparently normal behavior that is also present on the master branch.

I see the RHLM and RLLM values change in dbpr output from iocsh, but ca clients only get the correct value at the time they connect.

kmpeters commented 1 year ago

Marking the raw limit fields isn't enough to solve the problem.

diff --git a/motorApp/MotorSrc/motorRecord.cc b/motorApp/MotorSrc/motorRecord.cc
index 77658993..91bd4928 100644
--- a/motorApp/MotorSrc/motorRecord.cc
+++ b/motorApp/MotorSrc/motorRecord.cc
@@ -3982,11 +3982,13 @@ static void set_user_highlimit(motorRecord* pmr, struct motor_dset* pdset)
         {
             pmr->dhlm = tmp_limit;
             pmr->rhlm = tmp_raw;
+            MARK_AUX(M_RHLM);
         }
         else
         {
             pmr->dllm = tmp_limit;
-            pmr->rllm = tmp_limit;
+            pmr->rllm = tmp_raw;
+            MARK_AUX(M_RLLM);
         }
     }
     MARK(M_HLM);
@@ -4052,11 +4054,13 @@ static void set_user_lowlimit(motorRecord* pmr, struct motor_dset* pdset)
         if (dir_positive) {
             pmr->dllm = tmp_limit;
             pmr->rllm = tmp_raw;
+            MARK_AUX(M_RLLM);
         }
         else
         {
             pmr->dhlm = tmp_limit;
             pmr->rhlm = tmp_raw;
+            MARK_AUX(M_RHLM);
         }

     }
@@ -4082,6 +4086,7 @@ static void set_dial_highlimit(motorRecord *pmr, struct motor_dset *pdset)
     tmp_raw = pmr->dhlm / pmr->mres;
     // set the raw high limit
     pmr->rhlm = tmp_raw;
+    MARK_AUX(M_RHLM);

     INIT_MSG();
     if (pmr->mres < 0) {
@@ -4125,7 +4130,7 @@ static void set_dial_lowlimit(motorRecord *pmr, struct motor_dset *pdset)
     tmp_raw = pmr->dllm / pmr->mres;
     // set the raw low limit
     pmr->rllm = tmp_raw;
-
+    MARK_AUX(M_RLLM);

     INIT_MSG();
     if (pmr->mres < 0) {
kmpeters commented 1 year ago

Adding these lines fixes the ca-clients-failing-to-update problem:

diff --git a/motorApp/MotorSrc/motorRecord.cc b/motorApp/MotorSrc/motorRecord.cc
index 77658993..e4a106b8 100644
--- a/motorApp/MotorSrc/motorRecord.cc
+++ b/motorApp/MotorSrc/motorRecord.cc
@@ -3527,6 +3527,10 @@ static void monitor(motorRecord * pmr)
         db_post_events(pmr, &pmr->homf, local_mask);
     if ((local_mask = monitor_mask | (MARKED_AUX(M_HOMR) ? DBE_VAL_LOG : 0)))
         db_post_events(pmr, &pmr->homr, local_mask);
+    if ((local_mask = monitor_mask | (MARKED_AUX(M_RHLM) ? DBE_VAL_LOG : 0)))
+        db_post_events(pmr, &pmr->rhlm, local_mask);
+    if ((local_mask = monitor_mask | (MARKED_AUX(M_RLLM) ? DBE_VAL_LOG : 0)))
+        db_post_events(pmr, &pmr->rllm, local_mask);
kmpeters commented 1 year ago

Commit 99d0c414157a9dcfcfc90cc196d8155df477ce12 fixed updating RHLM/RLLM when HLM/DHLM/LLM/DHLM change, but not when the resolution changes.

kmpeters commented 1 year ago

Commit 99d0c41 fixed updating RHLM/RLLM when HLM/DHLM/LLM/DHLM change, but not when the resolution changes.

RHLM/RLLM shouldn't change when MRES changes.