Merge pull request #2206 from jluhrs/feature/FR-72718

FR-72718 Fixed some bugs in target filter control.
gemini-hlsw · Jul 6, 2022 · 9ba0baa · 9ba0baa
2 parents 69c156d + bbece1e
commit 9ba0baa
Show file tree

Hide file tree

Showing 6 changed files with 317 additions and 60 deletions.
diff --git a/modules/server/src/main/scala/seqexec/server/altair/AltairController.scala b/modules/server/src/main/scala/seqexec/server/altair/AltairController.scala
@@ -56,14 +56,13 @@ object AltairController {
   implicit val showAltairConfig: Show[AltairConfig] = Show.fromToString[AltairConfig]
 
   sealed case class AltairPauseResume[F[_]](
-    pause:              Option[F[Unit]],
-    guideWhilePaused:   GuideCapabilities,
-    pauseTargetFilter:  Boolean,
-    resume:             Option[F[Unit]],
-    restoreOnResume:    GuideCapabilities,
-    resumeTargetFilter: Boolean,
-    config:             Option[F[Unit]],
-    forceFreeze:        Boolean
+    pause:             Option[F[Unit]],
+    guideWhilePaused:  GuideCapabilities,
+    pauseTargetFilter: Boolean,
+    resume:            Option[F[Unit]],
+    restoreOnResume:   GuideCapabilities,
+    config:            Option[F[Unit]],
+    forceFreeze:       Boolean
   )
 
 }
diff --git a/modules/server/src/main/scala/seqexec/server/altair/AltairControllerDisabled.scala b/modules/server/src/main/scala/seqexec/server/altair/AltairControllerDisabled.scala
@@ -27,7 +27,6 @@ class AltairControllerDisabled[F[_]: Logger: Applicative] extends AltairControll
       pauseTargetFilter = false,
       overrideLogMessage("Altair", "resume AO loops").some,
       GuideCapabilities(canGuideM2 = false, canGuideM1 = false),
-      resumeTargetFilter = false,
       none,
       forceFreeze = true
     ).pure[F]

diff --git a/modules/server/src/main/scala/seqexec/server/altair/AltairControllerEpics.scala b/modules/server/src/main/scala/seqexec/server/altair/AltairControllerEpics.scala
@@ -96,18 +96,20 @@ object AltairControllerEpics {
 
     private def pauseNgsMode(
       position:     (Length, Length),
-      currCfg:      EpicsAltairConfig
+      currCfg:      EpicsAltairConfig,
+      instrument:   Instrument
     )(pauseReasons: PauseConditionSet): PauseReturn[F] = {
       // There are two reasons to stop NGS:
       // 1. This is an unguided step
       // 2. The current control matrix will not be valid for the end position after applying an offset (i.e. the offset
       // is too big).
 
-      val guidedStep   = !pauseReasons.contains(PauseCondition.GaosGuideOff)
-      val currMatrixOk = validateCurrentControlMatrix(currCfg, position)
-      val prepMatrixOk = validatePreparedControlMatrix(currCfg, position)
+      val guidedStep    = !pauseReasons.contains(PauseCondition.GaosGuideOff)
+      val currMatrixOk  = validateCurrentControlMatrix(currCfg, position)
+      val prepMatrixOk  = validatePreparedControlMatrix(currCfg, position)
+      val isSmallOffset = pauseReasons.offsetO.forall(canGuideWhileOffseting(_, instrument))
 
-      val needsToStop = !guidedStep || !currMatrixOk
+      val needsToStop = !guidedStep || !currMatrixOk || !isSmallOffset
 
       val mustPrepareMatrix = (!currCfg.aoLoop || needsToStop) && guidedStep && !prepMatrixOk
 
@@ -126,17 +128,19 @@ object AltairControllerEpics {
         PauseReturn[F](
           wasPaused = true,
           pauseAction.some,
-          pauseTargetFilter = true,
+          pauseTargetFilter =
+            pauseReasons.offsetO.nonEmpty,                                        // if not guiding, pause target filter for offsets
           GuideCapabilities(canGuideM2 = false, canGuideM1 = false),
           configActions
         )
       else
         PauseReturn[F](
           wasPaused = false,
           L.debug(
-            s"Skipped pausing Altair NGS guiding because guidedStep=$guidedStep, needsToStop = $needsToStop, currMatrixOk=$currMatrixOk"
+            s"Skipped pausing Altair NGS guiding because currCfg.aoLoop=${currCfg.aoLoop}, guidedStep=$guidedStep, isSmallOffset = $isSmallOffset, currMatrixOk=$currMatrixOk"
           ).some,
-          pauseTargetFilter = false,
+          pauseTargetFilter =
+            (!currCfg.aoLoop && pauseReasons.offsetO.nonEmpty) || !isSmallOffset, // pause target filter for unguided offset
           GuideCapabilities(canGuideM2 = currCfg.aoLoop, canGuideM1 = currCfg.aoLoop),
           configActions
         )
@@ -154,7 +158,8 @@ object AltairControllerEpics {
     private def pauseResumeNgsMode(
       startPos:     (Length, Length),
       currCfg:      EpicsAltairConfig,
-      currOffset:   FocalPlaneOffset
+      currOffset:   FocalPlaneOffset,
+      instrument:   Instrument
     )(pauseReasons: PauseConditionSet, resumeReasons: ResumeConditionSet): AltairPauseResume[F] = {
       val newPos                = pauseReasons.offsetO
         .map(x => newPosition(startPos)(x.to))
@@ -165,7 +170,7 @@ object AltairControllerEpics {
       val adjustedResumeReasons =
         forceFreeze.fold(resumeReasons - ResumeCondition.GaosGuideOn, resumeReasons)
 
-      val pauseResult = pauseNgsMode(newPos, currCfg)(adjustedPauseReasons)
+      val pauseResult = pauseNgsMode(newPos, currCfg, instrument)(adjustedPauseReasons)
       val resume      = resumeNgsMode(
         currCfg.aoLoop,
         currCfg.aoLoop && pauseResult.wasPaused,
@@ -178,7 +183,6 @@ object AltairControllerEpics {
         pauseResult.pauseTargetFilter,
         resume.resumeAction,
         resume.keepGuiding,
-        resume.resumeTargetFilter,
         pauseResult.config,
         forceFreeze
       )
@@ -234,7 +238,6 @@ object AltairControllerEpics {
 
       ResumeReturn(
         action.some,
-        !((aoOn && !wasPaused) || !guidedStep),
         GuideCapabilities(canGuideM2 = reasons.contains(ResumeCondition.GaosGuideOn),
                           canGuideM1 = reasons.contains(ResumeCondition.GaosGuideOn)
         )
@@ -350,7 +353,6 @@ object AltairControllerEpics {
         pause.pauseTargetFilter,
         resume.resumeAction,
         resume.keepGuiding,
-        resume.resumeTargetFilter,
         none,
         forceFreeze
       )
@@ -365,26 +367,29 @@ object AltairControllerEpics {
       val guidedStep    = !reasons.contains(PauseCondition.GaosGuideOff)
       val isSmallOffset = reasons.offsetO.forall(canGuideWhileOffseting(_, instrument))
       val mustPauseNGS  = !(guidedStep && isSmallOffset) && (strap || sfo)
+      val usingNGS      = currCfg.sfoLoop === LgsSfoControl.Enable || currCfg.strapLoop
 
       val pauseAction = L.debug(
         s"Pausing Altair LGS(strap = $strap, sfo = $sfo) guiding because guidedStep=$guidedStep, isSmallOffset=$isSmallOffset"
       ) *>
         ttgsOff(currCfg) *>
         L.debug(s"Altair LGS(strap = $strap, sfo = $sfo) guiding paused")
 
-      if ((currCfg.sfoLoop === LgsSfoControl.Enable || currCfg.strapLoop) && mustPauseNGS)
+      if (usingNGS && mustPauseNGS)
         PauseReturn(
           wasPaused = true,
           pauseAction.some,
-          pauseTargetFilter = true,
+          pauseTargetFilter = reasons.offsetO.nonEmpty,
           GuideCapabilities(canGuideM2 = false, canGuideM1 = true),
           none
         )
       else
         PauseReturn(
           wasPaused = false,
-          L.debug(s"Skipped pausing Altair LGS(strap = $strap, sfo = $sfo) guiding").some,
-          pauseTargetFilter = false,
+          L.debug(
+            s"Skipped pausing Altair LGS(strap = $strap, sfo = $sfo) guiding, guidedStep=$guidedStep, isSmallOffset=$isSmallOffset"
+          ).some,
+          pauseTargetFilter = (!usingNGS && reasons.offsetO.nonEmpty) || !isSmallOffset,
           GuideCapabilities(canGuideM2 =
                               currCfg.sfoLoop === LgsSfoControl.Enable || currCfg.strapLoop,
                             canGuideM1 = true
@@ -415,7 +420,6 @@ object AltairControllerEpics {
 
       ResumeReturn(
         action.some,
-        !alreadyThere && guidedStep,
         GuideCapabilities(
           canGuideM2 = (strap || sfo) && reasons.contains(ResumeCondition.GaosGuideOn),
           canGuideM1 = true
@@ -439,7 +443,6 @@ object AltairControllerEpics {
         pauseTargetFilter = true,
         none,
         GuideCapabilities(canGuideM2 = false, canGuideM1 = false),
-        resumeTargetFilter = false,
         none,
         forceFreeze = true
       )
@@ -454,15 +457,16 @@ object AltairControllerEpics {
       retrieveConfig.map { currCfg =>
         cfg match {
           case Ngs(_, starPos)      =>
-            pauseResumeNgsMode(starPos, currCfg, currentOffset)(pauseReasons, resumeReasons)
+            pauseResumeNgsMode(starPos, currCfg, currentOffset, instrument)(pauseReasons,
+                                                                            resumeReasons
+            )
           case Lgs(false, false, _) =>
             AltairPauseResume(
               none,
               GuideCapabilities(canGuideM2 = false, canGuideM1 = true),
               pauseTargetFilter = true,
               none,
               GuideCapabilities(canGuideM2 = false, canGuideM1 = true),
-              resumeTargetFilter = false,
               none,
               forceFreeze = true
             )
@@ -477,7 +481,6 @@ object AltairControllerEpics {
               pauseTargetFilter = false,
               none,
               GuideCapabilities(resumeReasons.fixed.contains(P1On), canGuideM1 = true),
-              resumeTargetFilter = false,
               none,
               forceFreeze = true
             )
@@ -488,7 +491,6 @@ object AltairControllerEpics {
               pauseTargetFilter = false,
               none,
               GuideCapabilities(resumeReasons.fixed.contains(OiOn), canGuideM1 = true),
-              resumeTargetFilter = false,
               none,
               forceFreeze = true
             )
@@ -565,9 +567,8 @@ object AltairControllerEpics {
 
   // Auxiliary class that contains all the information from a resume calculation
   private sealed case class ResumeReturn[F[_]](
-    resumeAction:       Option[F[Unit]],  // The resume action
-    resumeTargetFilter: Boolean,          // Does the target filter need to be enabled ? (info for TCS configuration)
-    keepGuiding:        GuideCapabilities // What guiding to enable after resume
+    resumeAction: Option[F[Unit]],  // The resume action
+    keepGuiding:  GuideCapabilities // What guiding to enable after resume
   )
 
   private val DefaultTimeout = FiniteDuration(10, SECONDS)

diff --git a/modules/server/src/main/scala/seqexec/server/altair/AltairControllerSim.scala b/modules/server/src/main/scala/seqexec/server/altair/AltairControllerSim.scala
@@ -28,7 +28,6 @@ object AltairControllerSim {
         pauseTargetFilter = false,
         L.info(s"Simulate restoring Altair configuration $cfg because of $resumeReasons").some,
         GuideCapabilities(canGuideM2 = false, canGuideM1 = false),
-        resumeTargetFilter = false,
         none,
         forceFreeze = true
       ).pure[F]

diff --git a/modules/server/src/main/scala/seqexec/server/tcs/TcsNorthControllerEpicsAo.scala b/modules/server/src/main/scala/seqexec/server/tcs/TcsNorthControllerEpicsAo.scala
@@ -123,11 +123,10 @@ object TcsNorthControllerEpicsAo {
           L.debug(s"Target filtering ${filterEnabled.fold("activated", "deactivated")}.")
 
       def sysConfig(
-        current:            EpicsTcsAoConfig,
-        demand:             TcsNorthAoConfig,
-        pauseTargetFilter:  Boolean,
-        resumeTargetFilter: Boolean,
-        aoConfigO:          Option[F[Unit]]
+        current:           EpicsTcsAoConfig,
+        demand:            TcsNorthAoConfig,
+        pauseTargetFilter: Boolean,
+        aoConfigO:         Option[F[Unit]]
       ): F[EpicsTcsAoConfig] = {
         val mountConfigParams   =
           commonController.configMountPos(subsystems, current, demand.tc, EpicsTcsAoConfig.base)
@@ -163,7 +162,7 @@ object TcsNorthControllerEpicsAo {
                  )
                    epicsSys.waitAGInPosition(agTimeout) *> L.debug("AG inposition")
                  else Applicative[F].unit
-            _ <- executeTargetFilterConf(true).whenA(resumeTargetFilter)
+            _ <- executeTargetFilterConf(true).whenA(pauseTargetFilter && mountMoves)
             _ <- L.debug("Completed TCS configuration")
           } yield s
         } else
@@ -199,7 +198,6 @@ object TcsNorthControllerEpicsAo {
                            s1,
                            adjustedDemand,
                            pr.pauseTargetFilter,
-                           pr.resumeTargetFilter,
                            pr.config
                          )
         _             <- guideOn(subsystems, s2, adjustedDemand, pr.restoreOnResume)