sequencerd: expose once acamd has made the target offset

Config/acamd.cfg.in

@@ -157,10 +157,10 @@ MOTOR_POS="cover 1 1 504 close"
 #
 ACQUIRE_TIMEOUT=90            # seconds before ACAM acquisition sequence aborts on failure to acquire (REQUIRED!)
 ACQUIRE_RETRYS=5              # max number of retrys before acquisition fails (optional, can be left blank to disable)
-ACQUIRE_OFFSET_THRESHOLD=0.5  # computed offset below this threshold (in arcsec) defines successful acquisition
+ACQUIRE_OFFSET_THRESHOLD=1.0  # computed offset below this threshold (in arcsec) defines successful acquisition
 ACQUIRE_MIN_REPEAT=2          # minimum number of sequential successful acquires
 ACQUIRE_TCS_MAX_OFFSET=60     # max offset (arcsec) for an ordinary guiding correction sent to the TCS
-ACQUIRE_TCS_MAX_PUTONSLIT_OFFSET=300  # max offset (arcsec) for a deliberate goal offset (put-on-slit, offset-star, end-of-fineacquire, pyGUI 'Offset') applied while guiding
+ACQUIRE_TCS_MAX_PUTONSLIT_OFFSET=300  # max offset (arcsec) for the one correction that makes a deliberate goal offset (put-on-slit, offset-star, end-of-fineacquire, pyGUI 'Offset'); guide and fine-acquire corrections keep ACQUIRE_TCS_MAX_OFFSET
 
 # SkySimulator options:
 #   SKYSIM_IMAGE_SIZE=<is>   where <is> is integer

Config/sequencerd.cfg.in

@@ -162,7 +162,7 @@ ACQUIRE_TIMEOUT=90                   # seconds before ACAM acquisition sequence
 ACQUIRE_RETRYS=5                     # max number of retrys before acquisition fails (optional, can be left blank to disable)
 ACQUIRE_OFFSET_THRESHOLD=0.5         # computed offset below this threshold (in arcsec) defines successful acquisition
 ACQUIRE_MIN_REPEAT=2                 # minimum number of sequential successful acquires
-OFFSET_SETTLE_SEC=3                  # sec to wait after an offset-star target offset before exposing (0 disables; only delays targets with a non-zero offset)
+OFFSET_SETTLE_SEC=0                  # extra sec to wait before exposing once acamd has made an offset-star target offset (the sequencer waits for the move itself; 0 = no extra wait)
 ACQUIRE_TCS_MAX_OFFSET=60            # the maximum allowable offset sent to the TCS, in arcsec
 
 # Calibration Settings

Config/slicecamd.cfg.in

@@ -86,9 +86,10 @@ FINE_ACQUIRE_MIN_SAMPLES=3
 
 # FINE_ACQUIRE_SETTLE_FRAMES=<n>
 # <n> frames discarded after each telescope move to allow settling before
-# centroid measurements resume.
+# centroid measurements resume. The wall-clock settle (FINE_ACQUIRE_SETTLE_SEC,
+# applied after acamd confirms the goal change) covers the move itself.
 #
-FINE_ACQUIRE_SETTLE_FRAMES=2
+FINE_ACQUIRE_SETTLE_FRAMES=0
 
 # FINE_ACQUIRE_SETTLE_SEC=<sec>
 # Time-based settle <sec> applied after each move, in addition to SETTLE_FRAMES,
@@ -98,6 +99,13 @@ FINE_ACQUIRE_SETTLE_FRAMES=2
 #
 FINE_ACQUIRE_SETTLE_SEC=3.0
 
+# FINE_ACQUIRE_MOVE_TIMEOUT=<sec>
+# Max time <sec> to wait for acamd to report a requested goal change applied
+# before the settle begins. On timeout the settle proceeds anyway. Set to 0
+# to disable the wait.
+#
+FINE_ACQUIRE_MOVE_TIMEOUT=60.0
+
 # FINE_ACQUIRE_GAIN=<g>
 # Proportional gain <g> = {0..1} applied to the commanded offset when the residual
 # is at or below FINE_ACQUIRE_GAIN_THRESHOLD arcsec.

acamd/acam_interface.h

@@ -349,9 +349,10 @@ namespace Acam {
 
       std::atomic<bool> is_acquired;       ///< set if target acquired successfully
       std::atomic<bool> stop_acquisition;  ///< set if the acquisition sequence should stop
+      std::atomic<bool> goalshift_pending{false};  ///< a requested goal change awaits execution by the guide loop
 
       double tcs_max_offset;
-      double tcs_max_putonslit_offset{300.};  ///< max offset (arcsec) for a deliberate goal offset (put-on-slit etc.) applied while guiding; defaults 300 if ACQUIRE_TCS_MAX_PUTONSLIT_OFFSET absent
+      double tcs_max_putonslit_offset{300.};  ///< max offset (arcsec) for the correction that makes a deliberate goal offset (put-on-slit etc.); defaults 300 if ACQUIRE_TCS_MAX_PUTONSLIT_OFFSET absent
 
       double offset_cal_offset, offset_cal_raoff, offset_cal_decoff;
 
@@ -386,7 +387,14 @@ namespace Acam {
         this->time_offs.clear();
         this->tcs_offset_period = val;
       }
-      inline void reset_offset_params() { this->reset_offset_params(this->tcs_offset_period); }
+      /// clear the filter only: never rewrite tcs_offset_period, which a concurrent
+      /// offsetperiod command may be setting
+      inline void reset_offset_params() {
+        std::lock_guard<std::mutex> lock(this->offset_params_mtx);
+        this->ra_offs.clear();
+        this->dec_offs.clear();
+        this->time_offs.clear();
+      }
 
       inline double get_timeout() { return this->timeout; }
 
@@ -483,9 +491,72 @@ namespace Acam {
         double ra;
         double dec;
         double angle;
-      } acam_goal;
+      } acam_goal;       ///< guarded by goal_mtx (snapshot_goal, set_goal, offset_goal)
 
-      std::atomic<bool> allow_large_offset{false};  ///< one-shot: allow the next guiding correction up to tcs_max_putonslit_offset
+      // Goal changes and the corrections that make them (PAL-171).
+      //
+      // offset_goal changes the goal; the guide loop moves the telescope. Every
+      // correction is classified by the goal it was computed against:
+      //   guide correction       -- a goal the loop has already moved to;
+      //   goal-change correction -- a goal changed since: a fine-acquire
+      //                             correction ('fineguiding') or a deliberate
+      //                             offset (untagged: target offset, offset star,
+      //                             put-on-slit, pyGUI 'Offset').
+      // goal_gen counts goal changes (offset_goal) and replacements (set_goal,
+      // which counts as made). goal_gen_applied is the newest change that a SENT
+      // correction was computed against. deliberate_gen is the newest
+      // deliberate change (0 = none); it is still to be made while
+      // deliberate_gen > goal_gen_applied, and only a correction computed against
+      // it (or a later goal) can make it -- so a correction already in flight can
+      // neither use nor spend its cap. All guarded by goal_mtx.
+      //
+      std::mutex goal_mtx;
+      unsigned long long goal_gen{0};
+      unsigned long long goal_gen_applied{0};
+      unsigned long long deliberate_gen{0};
+
+      struct goal_snapshot_t {
+        double ra, dec, angle;
+        unsigned long long gen;  ///< goal generation the correction is computed against
+        bool goal_change;        ///< includes a goal change not yet made
+        bool deliberate;         ///< includes a deliberate goal change not yet made
+      };
+
+      /// the goal, its generation and its classification, read together
+      inline goal_snapshot_t snapshot_goal() {
+        std::lock_guard<std::mutex> lock(this->goal_mtx);
+        return { this->acam_goal.ra, this->acam_goal.dec, this->acam_goal.angle, this->goal_gen,
+                 this->goal_gen > this->goal_gen_applied,
+                 this->deliberate_gen > this->goal_gen_applied };
+      }
+
+      /// true if no goal change has arrived since the snapshot of generation gen
+      inline bool goal_unchanged_since( unsigned long long gen ) {
+        std::lock_guard<std::mutex> lock(this->goal_mtx);
+        return this->goal_gen == gen;
+      }
+
+      /// a correction computed against generation gen has been sent: the goal
+      /// changes up to gen are made. goalshift_pending stays set if a newer one arrived.
+      inline void goal_change_made( unsigned long long gen ) {
+        std::lock_guard<std::mutex> lock(this->goal_mtx);
+        if ( gen > this->goal_gen_applied ) this->goal_gen_applied = gen;
+        this->goalshift_pending.store( this->goal_gen > this->goal_gen_applied, std::memory_order_release );
+      }
+
+      /// a new acquisition replaces the goal (from the target coordinates or a
+      /// solve): every earlier change is void and nothing is pending. The
+      /// generation moves on, so a correction computed against the replaced goal
+      /// is dropped; the new goal counts as made (the acquisition moves to it).
+      inline void set_goal( const double ra, const double dec, const double angle ) {
+        std::lock_guard<std::mutex> lock(this->goal_mtx);
+        this->acam_goal.ra     = ra;
+        this->acam_goal.dec    = dec;
+        this->acam_goal.angle  = angle;
+        this->goal_gen_applied = ++this->goal_gen;
+        this->deliberate_gen   = 0;
+        this->goalshift_pending.store( false, std::memory_order_release );
+      }
 
       Target() : iface(nullptr), timeout(10), max_attempts(-1), min_repeat(1),
                  is_acquired(false),
@@ -530,6 +601,7 @@ namespace Acam {
         int         attempts     = 0;
         std::string filter       = "";
         std::string cover        = "";
+        bool        goalshift_pending = false;
       } last_status;
 
     public:

common/message_keys.h

@@ -106,6 +106,7 @@ namespace Key {
     inline const std::string ATTEMPTS     = "attempts";
     inline const std::string SEEING       = "seeing";
     inline const std::string BACKGROUND   = "background";
+    inline const std::string GOALSHIFT_PENDING = "goalshift_pending";  ///< a requested goal change awaits execution by the guide loop
   }
 
   namespace Slicecamd {

sequencerd/sequence.cpp

@@ -197,13 +197,26 @@ namespace Sequencer {
     int64_t pubtime=0;
     Common::extract_telemetry_value( jmessage, Key::PUBTIME, pubtime );
 
+    // acamd's goal change still to be made: raised by offsetgoal, cleared once
+    // the correction computed against the new goal has been sent and the TCS
+    // has answered it. wait_for_target_offset waits on it before exposing.
+    const bool has_goalshift = jmessage.contains( Key::Acamd::GOALSHIFT_PENDING );
+    bool goalshift = false;
+    if ( has_goalshift ) Common::extract_telemetry_value( jmessage, Key::Acamd::GOALSHIFT_PENDING, goalshift );
+
     // Store under the mutex so the writes are visible to any waiter's predicate
     // when it re-acquires acam_mtx inside acam_cv.wait().
     std::lock_guard<std::mutex> lock(this->acam_mtx);
     this->is_acam_guiding.store( acquired );
     if ( has_mode ) {
       this->is_acam_acquiring.store( acquiring );
     }
+    // recorded per status, so a wait that starts after the change was already
+    // made still sees that it was raised and then cleared
+    if ( has_goalshift ) {
+      if ( goalshift ) this->acam_goalshift_raised_us.store( pubtime );
+      else             this->acam_goalshift_cleared_us.store( pubtime );
+    }
     this->acam_pubtime.store( pubtime );
     this->acam_cv.notify_all();
   }
@@ -944,12 +957,13 @@ namespace Sequencer {
       // re-applied during each readout (flexure_reapply, in the readout wait).
       //
       std::future<long> flexure_future;
+      int64_t offset_sent_us = 0;  // set when the target offset went to acamd as a goal change
 
       if ( !this->target.iscal ) {
         // send offsets only on fineacquire, otherwise user needs to fix things
         if ( !this->is_repeat_target.load()      &&
               this->is_fineacquire_locked.load() &&
-              this->target_offset() == ERROR ) {
+              this->target_offset( &offset_sent_us ) == ERROR ) {
           if (this->wait_for_user()==ABORT) {
             this->broadcast.notice( function, "cancelled" );
             return;
@@ -968,9 +982,25 @@ namespace Sequencer {
         }
       }
 
-      // The offset-star offset is applied asynchronously by the ACAM guide loop;
-      // ACAMD_OFFSETGOAL returns before the move lands. Wait so the target is on
-      // the slit before exposing. Skipped when the target has no offset.
+      // A target offset sent while ACAM guides only changes acamd's goal. The
+      // guide loop makes the move with its next correction, whose pt_offset
+      // returns when the TCS answers the move. Expose once acamd reports that
+      // move made; the wait overlaps the slit move above and is bounded. If the
+      // move is not reported in time, expose anyway with a warning -- what the
+      // fixed settle always did -- so this can never hold the sequence.
+      if ( offset_sent_us > 0 ) {
+        const long offset_made = this->wait_for_target_offset( offset_sent_us );
+        if ( offset_made == ABORT ) {
+          this->broadcast.notice( function, "cancelled" );
+          return;
+        }
+        if ( offset_made != NO_ERROR ) {
+          this->broadcast.warning( function, "target offset not confirmed by acamd in time: exposing anyway" );
+        }
+      }
+
+      // Optional extra settle once the offset has been made (OFFSET_SETTLE_SEC,
+      // 0 = none). Skipped when the target has no offset.
       if ( !this->target.iscal && this->is_fineacquire_locked.load() &&
            ( this->target.offset_ra != 0.0 || this->target.offset_dec != 0.0 ) &&
            this->offset_settle_sec > 0.0 ) {
@@ -4354,9 +4384,11 @@ namespace Sequencer {
    * @return     ERROR or NO_ERROR
    *
    */
-  long Sequence::target_offset() {
+  long Sequence::target_offset( int64_t *acam_sent_us ) {
     const std::string function("Sequencer::Sequence::target_offset");
 
+    if ( acam_sent_us ) *acam_sent_us = 0;
+
     // nothing to do if both ra and dec offsets are zero
     if (this->target.offset_ra  == 0.0 &&
         this->target.offset_dec == 0.0) return NO_ERROR;
@@ -4371,7 +4403,11 @@ namespace Sequencer {
       const double ddec_deg = this->target.offset_dec / 3600.0;
       std::stringstream cmd;
       cmd << ACAMD_OFFSETGOAL << " " << std::fixed << std::setprecision(6) << dra_deg << " " << ddec_deg;
+      // taken before the send, so acamd's publish of the raised goal change is
+      // always later (no guard band needed, unlike do_acam_acquire)
+      const int64_t sent_us = get_time_us();
       error = this->acamd.command( cmd.str() );
+      if ( error == NO_ERROR && acam_sent_us ) *acam_sent_us = sent_us;
     }
     else
     // if ACAM is not guiding then I send the offsets directly to the TCS
@@ -4390,6 +4426,61 @@ namespace Sequencer {
   /***** Sequencer::Sequence::target_offset ***********************************/
 
 
+  /***** Sequencer::Sequence::wait_for_target_offset **************************/
+  /**
+   * @brief      wait until acamd has made a target offset sent as a goal change
+   * @details    ACAMD_OFFSETGOAL only changes the guide goal. acamd's guide loop
+   *             makes the move with its next correction, and that correction's
+   *             pt_offset returns when the TCS answers the move (later for
+   *             bigger moves). acamd raises GOALSHIFT_PENDING with the goal
+   *             change and clears it once that correction has been sent and
+   *             answered. The move is made once a status published after the
+   *             send has shown it raised and a later one has shown it cleared.
+   *             handletopic_acamd records both publish times as statuses
+   *             arrive, so this holds even if the move was made before this
+   *             wait began (the wait follows the slit move). Abort wakes acam_cv.
+   * @param[in]  sent_us  get_time_us() taken just before the offset was sent
+   * @return     NO_ERROR | ERROR (not reported within TIMEOUT_SEC) | ABORT (cancelled)
+   *
+   */
+  long Sequence::wait_for_target_offset( const int64_t sent_us ) {
+    const std::string function("Sequencer::Sequence::wait_for_target_offset");
+
+    // The move is made at acamd's next ACAM frame and answered by the TCS
+    // within seconds (measured: <= 4.3 s up to 147"); this covers a second
+    // frame and a 300" move. Bounded: the caller exposes either way.
+    //
+    constexpr double TIMEOUT_SEC = 30.0;
+
+    logwrite( function, "NOTICE: waiting for acamd to make the target offset before exposing" );
+
+    const auto deadline = std::chrono::steady_clock::now() + std::chrono::duration<double>( TIMEOUT_SEC );
+    bool made = false;
+    {
+      std::unique_lock<std::mutex> lock( this->acam_mtx );
+      made = this->acam_cv.wait_until( lock, deadline, [&]() {
+          if ( this->cancel_flag.load() ) return true;
+          const int64_t raised = this->acam_goalshift_raised_us.load();
+          return ( raised > sent_us && this->acam_goalshift_cleared_us.load() > raised );
+        } );
+    }
+
+    if ( this->cancel_flag.load() ) return ABORT;
+
+    std::ostringstream oss;
+    if ( !made ) {
+      oss << "ERROR acamd did not make the target offset within " << TIMEOUT_SEC << " s";
+      logwrite( function, oss.str() );
+      return ERROR;
+    }
+    oss << "target offset made by acamd " << std::fixed << std::setprecision(1)
+        << ( get_time_us() - sent_us ) / 1.0e6 << " s after it was sent";
+    logwrite( function, oss.str() );
+    return NO_ERROR;
+  }
+  /***** Sequencer::Sequence::wait_for_target_offset **************************/
+
+
   /***** Sequencer::Sequence::publish_targetinfo *****************************/
   /**
    * @brief      publish target info on Topic::TARGETINFO, on change (or force)

sequencerd/sequence.h

@@ -305,6 +305,8 @@ namespace Sequencer {
       std::atomic<bool> is_acam_guiding{false};    ///< is acam guiding (IS_ACQUIRED)?
       std::atomic<bool> is_acam_acquiring{false};  ///< is acam in an acquire mode?
       std::atomic<int64_t> acam_pubtime{0};      ///< publish time (us) of latest received acamd status
+      std::atomic<int64_t> acam_goalshift_raised_us{0};   ///< publish time (us) of the latest acamd status showing GOALSHIFT_PENDING set
+      std::atomic<int64_t> acam_goalshift_cleared_us{0};  ///< publish time (us) of the latest acamd status showing GOALSHIFT_PENDING clear
 
       /** @brief guard-band (us) subtracted from the acquire-command send time
        *         when computing the freshness boundary in do_acam_acquire. Tolerates
@@ -350,7 +352,7 @@ namespace Sequencer {
           tcs_settle_stable(1),
           tcs_domeazi_ready(1),
           tcs_preauth_time(0),
-          offset_settle_sec(3),
+          offset_settle_sec(0),
           do_once(false),
           tcs_which("real"),
           tcs_name("offline"),
@@ -428,7 +430,7 @@ namespace Sequencer {
       double tcs_settle_stable;   ///< time that TCS must report TRACKING before it is really tracking
       double tcs_domeazi_ready;   ///< max degrees azimuth that dome and telescope can differ before ready to observe
       double tcs_preauth_time;    ///< seconds before end of exposure to notify TCS of next target's coords (0 to disable)
-      double offset_settle_sec;   ///< sec to wait after a target offset before exposing (config OFFSET_SETTLE_SEC; 0 disables)
+      double offset_settle_sec;   ///< extra sec to wait after the target offset has been made, before exposing (config OFFSET_SETTLE_SEC; 0 = none)
       bool   tcs_auto_ontarget = true;     ///< TCS ?ONTARGET may satisfy the TCSOP wait (config TCS_AUTO_ONTARGET; default yes)
       double tcs_auto_ontarget_sep = 5.0;  ///< max |telescope-target| arcsec for auto-ontarget (config TCS_AUTO_ONTARGET_SEP)
       double tcs_auto_ontarget_cass = 0.05; ///< max |TCS cass angle - commanded ring angle| deg for auto-ontarget (config TCS_AUTO_ONTARGET_CASS)
@@ -621,7 +623,8 @@ namespace Sequencer {
       long get_tcs_weather_coords( double &ra_h, double &dec_d );              ///< read the current TCS ra,dec in decimal hr,deg
       long get_tcs_coords( double &ra_h, double &dec_d );        ///< read the current TCS ra,dec,cass in decimal hr,deg
       long get_tcs_cass( double &cass );
-      long target_offset();
+      long target_offset( int64_t *acam_sent_us = nullptr );  ///< acam_sent_us: send time if the offset went to acamd as a goal change, else 0
+      long wait_for_target_offset( const int64_t sent_us );   ///< wait until acamd has made that goal change
 
       void publish_targetinfo( bool force=false );                  ///< publish target info on change (or force)