From 4e25b8018a204671552888be86e88da2ba8fb463 Mon Sep 17 00:00:00 2001 From: Christoffer Fremling Date: Thu, 29 Oct 2026 16:09:28 +0200 Subject: [PATCH] acamd: classify guide, fine-acquire and deliberate-offset corrections A deliberate goal offset (the star-to-target offset after fine acquisition, put-on-slit, the GUI offset) armed a one-shot allowance that any sent correction cleared. A guide correction whose blocking pt_offset was in flight when the offset arrived cleared it as it completed, and the next frame's correction to the new goal was refused at the guiding cap, so the exposure was taken on the offset star. The cap selection also required acquire_mode == TARGET_GUIDE, which is a race of its own. Goal changes now carry a generation, and each correction is classified by the goal it was computed from: guide, fine-acquire, or deliberate offset. Only a deliberate-offset correction may use the larger ACQUIRE_TCS_MAX_PUTONSLIT_OFFSET cap; once sent it has made the change and clears goalshift_pending, unless a newer change arrived meanwhile. A correction computed from an older goal can neither use nor spend a newer goal's allowance. acamd.cfg.in: ACQUIRE_OFFSET_THRESHOLD=1.0, the value the deployed configuration uses. Co-Authored-By: Claude Opus 5.5 --- Config/acamd.cfg.in | 4 +- acamd/acam_interface.cpp | 178 ++++++++++++++++++++++++++------------- acamd/acam_interface.h | 78 ++++++++++++++++- 3 files changed, 196 insertions(+), 64 deletions(-) diff --git a/Config/acamd.cfg.in b/Config/acamd.cfg.in index 619954a2..9069af75 100644 --- a/Config/acamd.cfg.in +++ b/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= where is integer diff --git a/acamd/acam_interface.cpp b/acamd/acam_interface.cpp index 36dcbdec..7e0b47c9 100644 --- a/acamd/acam_interface.cpp +++ b/acamd/acam_interface.cpp @@ -3225,13 +3225,15 @@ logwrite( function, "[DEBUG] acquire here" ); } this->astrometry.solve( last_imagename, this->target.ext_solver_args ); - this->astrometry.get_solution( result, this->target.acam_goal.ra, this->target.acam_goal.dec, this->target.acam_goal.angle, matches, rmsarcsec ); + double solved_ra=NAN, solved_dec=NAN, solved_angle=NAN; + this->astrometry.get_solution( result, solved_ra, solved_dec, solved_angle, matches, rmsarcsec ); + this->target.set_goal( solved_ra, solved_dec, solved_angle ); // under goal_mtx: the guide loop may be reading it if ( result=="NOISY" || result=="GOOD" ) { goodsolution=true; - message.str(""); message << "setting goals from solver: " << this->target.acam_goal.ra << " " - << this->target.acam_goal.dec << " " - << this->target.acam_goal.angle; + message.str(""); message << "setting goals from solver: " << solved_ra << " " + << solved_dec << " " + << solved_angle; logwrite( function, message.str() ); break; } @@ -3311,11 +3313,13 @@ logwrite( function, message.str() ); message.str(""); message << "[DEBUG] input coords_slit.ra="<< this->target.coords_slit.ra << " .dec=" << this->target.coords_slit.dec; logwrite( function, message.str() ); + double goal_ra=NAN, goal_dec=NAN, goal_angle=NAN; this->fpoffsets.compute_offset_last_angle( this->target.get_pointmode(), "ACAM", this->target.coords_slit.ra, this->target.coords_slit.dec, - this->target.acam_goal.ra, this->target.acam_goal.dec, this->target.acam_goal.angle ); + goal_ra, goal_dec, goal_angle ); + this->target.set_goal( goal_ra, goal_dec, goal_angle ); // under goal_mtx: the guide loop may be reading it -message.str(""); message << "[DEBUG] goals: " << this->target.acam_goal.ra << " " << this->target.acam_goal.dec << " " << this->target.acam_goal.angle; +message.str(""); message << "[DEBUG] goals: " << goal_ra << " " << goal_dec << " " << goal_angle; logwrite( function, message.str() ); if ( error == NO_ERROR ) { @@ -3396,7 +3400,10 @@ logwrite( function, message.str() ); this->nacquired = 0; this->attempts = 0; this->sequential_failures = 0; - this->allow_large_offset.store(false); // no stale deliberate-offset allowance + { + std::lock_guard lock(this->goal_mtx); + this->deliberate_gen = 0; // no stale deliberate-offset allowance + } this->is_acquired.store( false, std::memory_order_release ); // Start the timeout clock, initialized as the time now plus the @@ -3470,6 +3477,7 @@ logwrite( function, message.str() ); // an attempt and will keep trying until max attempts. // long error = NO_ERROR; + bool frame_superseded = false; // the goal changed while this frame's correction was computed if ( this->acquire_mode == Acam::TARGET_ACQUIRE || this->acquire_mode == Acam::TARGET_ACQUIRE_HERE || @@ -3525,8 +3533,11 @@ message.str(""); message << "[DEBUG] ACQUIRE_HERE using goals: " << this->acam_g logwrite( function, message.str() ); } ******/ -message.str(""); message << "[DEBUG] using goals: " << this->acam_goal.ra << " " << this->acam_goal.dec << " " << this->acam_goal.angle; +{ +const goal_snapshot_t logged = this->snapshot_goal(); +message.str(""); message << "[DEBUG] using goals: " << logged.ra << " " << logged.dec << " " << logged.angle; logwrite( function, message.str() ); +} message.str(""); message << "[DEBUG] dRA=" << iface->target.dRA << " dDEC=" << iface->target.dDEC; logwrite( function, message.str() ); @@ -3590,7 +3601,8 @@ logwrite( function, message.str() ); // // Offsets calculated as difference between acam goals and the solution from // solve. ACAM goals are the SLIT->ACAM translated coords from the DB and - // are where the ACAM needs to be pointed. Goals never change. + // are where the ACAM needs to be pointed. Goals change only through + // offset_goal and a new acquisition (set_goal); see goal_mtx. // double ra_off, dec_off; // calculated offsets will be in degrees @@ -3613,16 +3625,31 @@ logwrite( function, message.str() ); offset = angular_separation( acam_goal.ra, acam_goal.dec, acam_ra, acam_dec ); } *****/ - const bool goal_change_at_compute = this->goalshift_pending.load(std::memory_order_acquire); - const double goal_ra_used = this->acam_goal.ra; - const double goal_dec_used = this->acam_goal.dec; + // Every correction is classified by the goal it is computed against, read + // together with that goal's generation (PAL-171; see goal_mtx): + // + // - guide correction: the loop has already moved to this goal. Capped at + // ACQUIRE_TCS_MAX_OFFSET and, while guiding, median-filtered. It makes + // no goal change and spends nothing that belongs to one. + // - goal-change correction: offset_goal changed the goal and no correction + // computed against it has been sent. A fine-acquire change + // ('fineguiding') keeps the ACQUIRE_TCS_MAX_OFFSET cap; a deliberate + // change among those still to be made allows + // ACQUIRE_TCS_MAX_PUTONSLIT_OFFSET. Sent at once and alone, never + // through the median filter. Once sent it has made the change(s) and + // clears goalshift_pending, unless a newer change arrived meanwhile. + // + // So a guide correction in flight when a deliberate offset arrives can + // neither use nor spend the deliberate cap. + // + const goal_snapshot_t goal = this->snapshot_goal(); if ( iface->fpoffsets.solve_offset( acam_ra, acam_dec, - goal_ra_used, goal_dec_used, + goal.ra, goal.dec, ra_off, dec_off ) == ERROR ) break; - offset = angular_separation( goal_ra_used, goal_dec_used, acam_ra, acam_dec ); + offset = angular_separation( goal.ra, goal.dec, acam_ra, acam_dec ); message.str(""); message << "[DEBUG] acam_ra=" << acam_ra << " acam_dec=" << acam_dec << " acam_goal.ra=" - << acam_goal.ra << " .dec=" << this->acam_goal.dec << " .ang=" << this->acam_goal.angle; + << goal.ra << " .dec=" << goal.dec << " .ang=" << goal.angle; logwrite( function, message.str() ); message.str(""); message << "[DEBUG] calculated RAoffset=" << ra_off << " DECLoffset=" << dec_off; logwrite( function, message.str() ); @@ -3641,18 +3668,18 @@ logwrite( function, message.str() ); // There is a maximum offset we send to the TCS. This is not a TCS limit // (theirs is very large); it is our safety limit so that a bad solution - // can't move us far off the slit. Ordinary guiding corrections use the - // normal tcs_max_offset (ACQUIRE_TCS_MAX_OFFSET). A deliberate goal offset - // applied while guiding -- via offset_goal, which covers put-on-slit, - // offset-star acquisition, the end-of-fineacquire target offset, and the - // pyGUI 'Offset' button -- is intentionally larger, so for the one - // correction that consumes it we allow up to tcs_max_putonslit_offset - // (ACQUIRE_TCS_MAX_PUTONSLIT_OFFSET). The ACQUIRE path uses tcs_max_offset - // and is unaffected. + // can't move us far off the slit. Guide and fine-acquire corrections use + // tcs_max_offset (ACQUIRE_TCS_MAX_OFFSET). Only the correction that makes + // a deliberate goal offset -- put-on-slit, offset-star acquisition, the + // end-of-fineacquire target offset, the pyGUI 'Offset' button -- may move + // up to tcs_max_putonslit_offset (ACQUIRE_TCS_MAX_PUTONSLIT_OFFSET). // - double maxoffset = this->tcs_max_offset; - if ( this->acquire_mode == Acam::TARGET_GUIDE && this->allow_large_offset.load() ) { - maxoffset = this->tcs_max_putonslit_offset; + const double maxoffset = goal.deliberate ? this->tcs_max_putonslit_offset : this->tcs_max_offset; + + if ( goal.goal_change ) { + message.str(""); message << "NOTICE correction for goal change " << goal.gen << ": offset=" << offset + << " arcsec, cap " << maxoffset << " arcsec"; + logwrite( function, message.str() ); } // Check the requested offset against the applicable max allowed offset @@ -3661,13 +3688,14 @@ logwrite( function, message.str() ); message.str(""); message << "[WARNING] calculated offset " << offset << " not below max " << maxoffset << " and will not be sent to the TCS"; logwrite( function, message.str() ); - this->allow_large_offset.store(false); // deliberate-offset allowance consumed even when rejected + // A refused correction has made no goal change: a deliberate change + // keeps its cap for the next frame. // Match found but failure to send an offset is considered an attempt // so attempts is incremented. // if ( attempts >= this->max_attempts ) { - message.str(""); message << "ERROR: failed to find offset below " << this->tcs_max_offset << " within max number of attempts"; + message.str(""); message << "ERROR: failed to find offset below " << maxoffset << " within max number of attempts"; iface->async.enqueue_and_log( function, message.str() ); error = ERROR; break; @@ -3685,7 +3713,29 @@ logwrite( function, message.str() ); bool should_offset=false; - // While guiding, run the offsets through a median filter over a + // A correction computed against a goal that has changed since is + // neither sent nor filtered: the next frame is computed against the new + // goal. + // + if ( !this->goal_unchanged_since( goal.gen ) ) { + // Measured against a goal that no longer exists: not sent, and the + // frame counts as neither an acquisition nor a failure. + logwrite( function, "NOTICE goal changed while this correction was computed: not sent" ); + attempts = 0; + frame_superseded = true; + break; + } + else + // A goal-change correction is sent at once and alone. It never enters + // the median filter, and the samples there, computed against the old + // goal, are discarded. + // + if ( goal.goal_change ) { + this->reset_offset_params(); + should_offset = true; + } + else + // While guiding, run guide corrections through a median filter over a // specified time period, and send the offsets only at the rate // determined by that period. This returns true and modifies the // values ra_off,dec_off when it's time to offset. @@ -3698,12 +3748,10 @@ logwrite( function, message.str() ); if ( should_offset ) { // send offset to TCS here (returns when offset is complete) if ( iface->tcsd.pt_offset( ra_off*3600., dec_off*3600., OFFSETRATE )==ERROR) break; - this->allow_large_offset.store(false); // deliberate-offset allowance consumed - if ( goal_change_at_compute && - goal_ra_used == this->acam_goal.ra && - goal_dec_used == this->acam_goal.dec ) { - this->goalshift_pending.store( false, std::memory_order_release ); - } + // A goal-change correction has now made its change(s). A guide + // correction makes none: a goal change that arrived while it was in + // flight keeps its own cap and stays pending. + if ( goal.goal_change ) this->goal_change_made( goal.gen ); std::this_thread::sleep_for( std::chrono::seconds(1) ); } @@ -3739,7 +3787,7 @@ logwrite( function, message.str() ); } while ( false ); // the do-loop is executed only once. used to allow breaks. } // end of acquisition sequence - if ( this->nacquired == 0 && this->max_attempts > 0 ) { + if ( !frame_superseded && this->nacquired == 0 && this->max_attempts > 0 ) { logwrite(function,"[DEBUG] nacquired="+std::to_string(nacquired)); if ( ++this->sequential_failures >= this->max_attempts ) { logwrite( function, "ERROR sequential failures "+std::to_string(this->sequential_failures) @@ -5486,8 +5534,9 @@ logwrite( function, message.str() ); /***** Acam::Interface::offset_goal *****************************************/ /** * @brief sets the dRA,dDEC offsets to apply to goal - * @details The offset is not applied here, this only stores the offsets - * in the class. Offsets are applied only while guiding. + * @details This changes the goal only. The acquisition or guide loop + * moves the telescope with its next correction computed against + * the new goal (see goal_mtx in Target). * @param[in] args input string expected " " * @param[out] retstring return string * @return ERROR | NO_ERROR @@ -5509,7 +5558,7 @@ logwrite( function, message.str() ); retstring = ACAMD_OFFSETGOAL; retstring.append( " [ [ fineguiding ] ]\n" ); retstring.append( " Apply offsets to the ACAM goal coordinates.\n" ); - retstring.append( " These offsets are applied only while guiding. If omitted,\n" ); + retstring.append( " The acquisition or guide loop then moves to the new goal. If omitted,\n" ); retstring.append( " the current offsets are returned. Units are in degrees.\n" ); retstring.append( " 'fineguiding' marks a slicecam fine-guiding correction (keeps the 60\" cap).\n" ); return HELP; @@ -5524,29 +5573,48 @@ logwrite( function, message.str() ); retstring="invalid_argument"; return ERROR; } - this->target.dRA = dRA; - this->target.dDEC = dDEC; - - // optional fineguiding flag: marks a slicecam fine-guiding correction, which + // optional fineguiding flag: marks a slicecam fine-acquire correction, which // must stay within the ordinary 60" guiding cap. Its absence means a // deliberate goal offset (put-on-slit, offset-star, end-of-fineacquire, // pyGUI 'Offset') that may legitimately be larger. std::string flag; const bool is_fineguiding = (iss >> flag && flag == "fineguiding"); - // Apply any dRA, dDEC goal offsets from the "put on slit" action to - // acam_ra_goal, acam_dec_goal. These dRA,dDEC offsets can come from - // either the ACAM or slicecam GUIs and are stored in the Target class. - // - // The supplied acam_ra_goal,acam_dec_goal are modified by dRA,dDEC + // Apply the dRA,dDEC offsets to the goal and record the change with its + // kind, in one step under goal_mtx: the guide loop never reads half a + // change, and two offsets arriving on different connections are both kept. + // Only a correction the guide loop computes against this goal (or a later + // one) can make the change; for a deliberate offset that correction alone + // may move up to ACQUIRE_TCS_MAX_PUTONSLIT_OFFSET, whatever mode the loop is + // in. A correction already in flight can neither use nor spend it (PAL-171). // - this->fpoffsets.apply_offset( this->target.acam_goal.ra, this->target.dRA, - this->target.acam_goal.dec, this->target.dDEC ); - this->target.goalshift_pending.store( true, std::memory_order_release ); + unsigned long long gen=0; + { + std::lock_guard lock( this->target.goal_mtx ); + double goal_ra = this->target.acam_goal.ra; + double goal_dec = this->target.acam_goal.dec; + if ( this->fpoffsets.apply_offset( goal_ra, dRA, goal_dec, dDEC ) == ERROR ) { + logwrite( function, "ERROR applying the offset to the goal: goal unchanged" ); + retstring="goal_unchanged"; + return ERROR; + } + this->target.acam_goal.ra = goal_ra; + this->target.acam_goal.dec = goal_dec; + this->target.dRA = dRA; + this->target.dDEC = dDEC; + gen = ++this->target.goal_gen; + if ( !is_fineguiding && ( dRA != 0.0 || dDEC != 0.0 ) ) { + this->target.deliberate_gen = gen; // a zero offset changes nothing, so it raises no cap + } + this->target.goalshift_pending.store( true, std::memory_order_release ); + } this->publish_status(true); + message.str(""); message << "goal change " << gen << " (" << ( is_fineguiding ? "fine-acquire" : "deliberate offset" ) + << "): dRA=" << dRA*3600. << " dDEC=" << dDEC*3600. << " arcsec"; + logwrite( function, message.str() ); - message.str(""); message << this->target.dRA << " " << this->target.dDEC; + message.str(""); message << dRA << " " << dDEC; retstring = message.str(); // Applying an offset to the goal while guiding must never drop out of @@ -5554,14 +5622,8 @@ logwrite( function, message.str() ); // TARGET_GUIDE and reset the offset filter so the new goal takes effect // quickly. This covers GUI "put on slit" and sequencer target offsets. // - // This is a deliberate offset (put-on-slit, offset-star, end-of-fineacquire, - // or the pyGUI 'Offset' button) and may exceed the normal guiding cap, so - // allow the next correction up to ACQUIRE_TCS_MAX_PUTONSLIT_OFFSET. The - // allowance is one-shot: do_acquire consumes it when the offset is sent. - // if ( this->target.acquire_mode == Acam::TARGET_GUIDE ) { this->target.reset_offset_params(); - if ( !is_fineguiding ) this->target.allow_large_offset.store(true); } this->publish_status(); diff --git a/acamd/acam_interface.h b/acamd/acam_interface.h index 06467a0d..4648bffa 100644 --- a/acamd/acam_interface.h +++ b/acamd/acam_interface.h @@ -352,7 +352,7 @@ namespace Acam { std::atomic 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; @@ -387,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 lock(this->offset_params_mtx); + this->ra_offs.clear(); + this->dec_offs.clear(); + this->time_offs.clear(); + } inline double get_timeout() { return this->timeout; } @@ -484,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 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 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 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 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 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),