From 239adf531f7f0d32a5e2147482d69ac2cd9ae5fb Mon Sep 17 00:00:00 2001 From: Christoffer Fremling Date: Thu, 5 Nov 2026 10:33:23 -0800 Subject: [PATCH 1/3] sequencerd: set the flexure compensation for the pointing being exposed flexure_set ran in parallel with the slew, so flexured computed the compensation from telescope telemetry read mid-slew, for the previous pointing, and it was never refreshed during an NEXP>1 series or before a REPEAT. For a target the telescope slews to, the compensator is now set after acquisition and the target offset, in parallel with the slit move, and joined before the exposure. When the telescope stays where it is (a calibration, or a repeat of the same pointing) it is set with the other workers, as before. During an NEXP>1 series it is re-applied in the readout between sub-exposures, only while that readout is running and the next shutter has not opened, with fresh, finite TCS telemetry and no thread error; a failure is reported and otherwise ignored. A REPEAT sets it once camera_set has seen camerad ready. Readouts are counted from camerad's shutter reports and PIXELCOUNT messages, so the count cannot run ahead of the shutter. abort waits up to 20 s (was 5 s) for the sequence to exit, since the readout wait may be inside a flexured call. Co-Authored-By: Claude Opus 5.5 --- sequencerd/sequence.cpp | 256 ++++++++++++++++++++++++++++++++++++---- sequencerd/sequence.h | 16 +++ 2 files changed, 252 insertions(+), 20 deletions(-) diff --git a/sequencerd/sequence.cpp b/sequencerd/sequence.cpp index 338b6077..37552c68 100644 --- a/sequencerd/sequence.cpp +++ b/sequencerd/sequence.cpp @@ -52,6 +52,18 @@ namespace Sequencer { int isready = jmessage[Key::Camerad::READY].get(); this->can_expose.store(isready); } + // Readout counting, step 1 of 2 (see the bookkeeping notes in sequence.h): + // the shutter report going from open to closed, in a report that is not + // PAUSED (camerad closes the shutter to pause, and says so), means an + // integration has ended. Open or paused reports disarm. + if ( jmessage.contains(Key::Camerad::SHUTTEROPEN) && jmessage[Key::Camerad::SHUTTEROPEN].is_boolean() ) { + const bool open = jmessage[Key::Camerad::SHUTTEROPEN].get(); + const bool paused = jmessage.contains(Key::Camerad::PAUSED) && jmessage[Key::Camerad::PAUSED].is_boolean() + && jmessage[Key::Camerad::PAUSED].get(); + if ( open || paused ) this->readout_armed = false; + else if ( this->last_shutteropen ) this->readout_armed = true; + this->last_shutteropen = open; + } this->camerad_cv.notify_all(); } /***** Sequencer::Sequence::handletopic_camerad ****************************/ @@ -512,10 +524,47 @@ namespace Sequencer { // Set READOUT flag and clear EXPOSE flag when pixels start coming in // ------------------------------------------------------------------ // - if ( seq.arm_readout_flag && statstr.compare( 0, 10, "PIXELCOUNT" )==0 ) { // async message tag PIXELCOUNT - seq.arm_readout_flag = false; - seq.wait_state_manager.set_and_clear( {Sequencer::SEQ_WAIT_READOUT}, // set READOUT - {Sequencer::SEQ_WAIT_EXPOSE} ); // clear EXPOSE + if ( statstr.compare( 0, 10, "PIXELCOUNT" )==0 ) { // async message tag PIXELCOUNT + // How far into its readout this device is: "PIXELCOUNT_: + // " (the first two numbers after the colon are + // used; -1 if they cannot be read). + double readout_fraction = -1.0; + const auto colon = statstr.find( ':' ); + if ( colon != std::string::npos ) { + std::istringstream fields( statstr.substr( colon+1 ) ); + std::string token; + std::vector numbers; + while ( numbers.size() < 2 && fields >> token ) { + try { + std::size_t used = 0; + const double value = std::stod( token, &used ); + if ( used == token.size() ) numbers.push_back( value ); + } + catch ( const std::exception & ) { } + } + if ( numbers.size() == 2 && numbers[1] > 0.0 ) readout_fraction = numbers[0] / numbers[1]; + } + // The first PIXELCOUNT of a readout ends the EXPOSE wait -- but not one + // from the second half of a readout: that is the tail of the previous + // target's readout, arriving after the next expose command. + if ( seq.arm_readout_flag && !( readout_fraction >= 0.5 ) ) { + seq.arm_readout_flag = false; + seq.wait_state_manager.set_and_clear( {Sequencer::SEQ_WAIT_READOUT}, // set READOUT + {Sequencer::SEQ_WAIT_EXPOSE} ); // clear EXPOSE + } + // Readout counting, step 2 of 2 (step 1 in handletopic_camerad): an + // armed count is completed by a PIXELCOUNT from the first half of a + // readout. A message from the tail of an earlier readout, or one that + // cannot be read, completes nothing; a count completed here leaves at + // least half a readout before camerad can open the shutter again. + { + std::lock_guard lock(seq.camerad_mtx); + if ( seq.readout_armed && readout_fraction >= 0.0 && readout_fraction < 0.5 ) { + seq.readout_armed = false; + seq.readouts_started++; + seq.camerad_cv.notify_all(); + } + } } // --------------------- @@ -717,6 +766,14 @@ namespace Sequencer { this->is_repeat_target.store( this->repeat_target() ); if ( this->is_repeat_target.load() ) this->broadcast.notice( function, "repeat target" ); + // Flexure compensation is computed from the telescope position at the + // moment flexured applies it. When the telescope stays where it is (a + // calibration, or a repeat of the same pointing) it is set with the other + // workers, in parallel, as it always was. When the telescope slews, that + // would compensate for the PREVIOUS pointing, so it is set after + // acquisition instead, just before the exposure. + const bool flexure_after_acquisition = !this->target.iscal && !this->is_repeat_target.load(); + // threads to start, pair their ThreadStatusBit with the function to call std::vector>> worker_threads; @@ -742,12 +799,14 @@ namespace Sequencer { worker_threads = { { THR_MOVE_TO_TARGET, std::bind(&Sequence::move_to_target, this) }, { THR_CAMERA_SET, std::bind(&Sequence::camera_set, this) }, { THR_FOCUS_SET, std::bind(&Sequence::focus_set, this) }, - { THR_FLEXURE_SET, std::bind(&Sequence::flexure_set, this) }, { THR_CALIB_SET, std::bind(&Sequence::calib_set, this) }, // for CAL targets, slit comes from database, otherwise use VSM acquire position { THR_SLIT_SET, std::bind(&Sequence::slit_set, this, this->target.iscal ? Sequencer::VSM_EXPOSE : Sequencer::VSM_ACQUIRE) } }; + if ( !flexure_after_acquisition ) { // telescope does not move: set it now, in parallel + worker_threads.push_back( { THR_FLEXURE_SET, std::bind(&Sequence::flexure_set, this) } ); + } } // pair their ThreadStatusBit with their future @@ -830,6 +889,14 @@ namespace Sequencer { } } + // For a target the telescope slewed to (flexure_after_acquisition), the + // flexure compensation is set here -- telescope on target, acquisition + // and target offset done -- in parallel with the slit move and the + // offset settle, and joined before the expose command. For NEXP>1 it is + // re-applied during each readout (flexure_reapply, in the readout wait). + // + std::future flexure_future; + if ( !this->target.iscal ) { // send offsets only on fineacquire, otherwise user needs to fix things if ( !this->is_repeat_target.load() && @@ -840,6 +907,9 @@ namespace Sequencer { return; } } + if ( flexure_after_acquisition ) { + flexure_future = std::async( std::launch::async, &Sequence::flexure_set, this ); + } // ensure slit offset is in "expose" position when needed try { error |= this->slit_set(Sequencer::VSM_EXPOSE); @@ -861,6 +931,23 @@ namespace Sequencer { std::this_thread::sleep_for( std::chrono::duration( this->offset_settle_sec ) ); } + // The compensator must be set before the shutter opens. A failure keeps + // the semantics flexure_set has in the worker set: the sequence stops. + // + if ( flexure_future.valid() ) { + try { + long flexure_error = flexure_future.get(); + error |= flexure_error; + if ( flexure_error != NO_ERROR ) { + this->broadcast.error( function, Sequencer::thread_names.at(THR_FLEXURE_SET)+" failed" ); + } + } + catch (const std::exception& e) { + this->broadcast.error( function, Sequencer::thread_names.at(THR_FLEXURE_SET)+" : "+std::string(e.what()) ); + return; + } + } + logwrite( function, "starting exposure" ); ///< TODO @todo log to telemetry! // Start the exposure in a thread... @@ -907,14 +994,37 @@ namespace Sequencer { // of the last exposure finishes — the correct completion signal for both // single and multi-exposure sequences. Skip the wait for frame transfer. // + // While waiting, every counted sub-exposure readout (readouts_started; see + // the bookkeeping notes in sequence.h) is acted on once: a readout that more sub-exposures + // follow gets the flexure compensation re-applied during it. + // if (!this->is_science_frame_transfer) { logwrite( function, "waiting for readout" ); + int readouts_handled = 0; std::unique_lock lock(this->camerad_mtx); - while ( !this->camerad_cv.wait_for( lock, std::chrono::seconds(30), - [this]() { return this->can_expose.load() || this->cancel_flag.load(); } ) ) { - logwrite( function, "waiting for readout — requesting snapshot" ); + while ( true ) { + const bool signalled = this->camerad_cv.wait_for( lock, std::chrono::seconds(30), + [this,&readouts_handled]() { return this->can_expose.load() || this->cancel_flag.load() || + this->readouts_started > readouts_handled; } ); + if ( this->can_expose.load() || this->cancel_flag.load() ) break; + if ( !signalled ) { + logwrite( function, "waiting for readout — requesting snapshot" ); + lock.unlock(); + this->request_snapshot(); + lock.lock(); + continue; + } + const int sub = ++readouts_handled; // readout of sub-exposure number sub (1-based) has begun + const int expected = this->readouts_expected; // the N sent to camerad, not the live row lock.unlock(); - this->request_snapshot(); + if ( sub < expected ) { + this->flexure_reapply( sub, expected ); + } + else if ( sub > expected ) { + message.str(""); message << "readout " << sub << " seen for a " << expected + << "-exposure command: camerad is running more sub-exposures than were sent"; + this->broadcast.warning( function, message.str() ); + } lock.lock(); } this->broadcast.notice( function, "done waiting for readout" ); @@ -2452,31 +2562,110 @@ namespace Sequencer { return NO_ERROR; } - // build up list of activate chans + // the compensate command names this target's active channels + const std::string comp = this->flexure_compensate_command(); + const std::string activechans = comp.substr( std::string(FLEXURED_COMPENSATE).size() ); + + if ( !this->cancel_flag.load() && + this->flexured.command( comp ) != NO_ERROR ) { + this->broadcast.error( function, "setting flexure compensator" ); + this->thread_error_manager.set( THR_FLEXURE_SET ); + throw std::runtime_error("setting flexure compensator"); + } + + this->is_flexure_default.store(false); + + this->broadcast.notice( function, "flexure compensator set for "+activechans); + + return NO_ERROR; + } + /***** Sequencer::Sequence::flexure_set *************************************/ + + + /***** Sequencer::Sequence::flexure_compensate_command **********************/ + /** + * @brief the FLEXURED_COMPENSATE command for the current target + * @return "compensate [ ...]" with the channels active for + * the target's calibration entry ("SCIENCE" for science targets) + */ + std::string Sequence::flexure_compensate_command() { std::ostringstream activechans; const std::string calname = std::string(this->target.iscal ? this->target.name : "SCIENCE"); const auto &calinfo = this->caltarget.get_info(calname); for (const auto &[chan,active] : calinfo.channel_active) { if (active) activechans << " " << chan; } + return std::string(FLEXURED_COMPENSATE) + activechans.str(); + } + /***** Sequencer::Sequence::flexure_compensate_command **********************/ - std::ostringstream comp; - comp << FLEXURED_COMPENSATE << activechans.str(); - if ( !this->cancel_flag.load() && - this->flexured.command( comp.str() ) != NO_ERROR ) { - this->broadcast.error( function, "setting flexure compensator" ); - this->thread_error_manager.set( THR_FLEXURE_SET ); - throw std::runtime_error("setting flexure compensator"); + /***** Sequencer::Sequence::flexure_reapply *********************************/ + /** + * @brief re-issue the flexure compensation between two sub-exposures + * @param[in] sub the sub-exposure (1-based) whose readout is in progress + * @return ERROR | NO_ERROR (informational only) + * + * Called by sequence_start during the readout of sub-exposure sub when + * sub-exposure sub+1 of expected follows. flexured recomputes from the + * telescope position at this moment, so the next sub-exposure starts + * corrected. Skipped (the compensation set at the start of the series + * stays) unless that readout is still running and the next shutter has not + * opened, the TCS telemetry is fresh and finite (flexured itself does not + * check the values), and no thread error is set. A failure is reported and + * otherwise ignored: nothing may disturb a running exposure command. + */ + long Sequence::flexure_reapply( int sub, int expected ) { + const std::string function("Sequencer::Sequence::flexure_reapply"); + std::stringstream message; + + if ( !this->should_flexcomp.load() || this->cancel_flag.load() ) return NO_ERROR; + + auto skipped = [&]( const std::string &why ) { + message.str(""); message << "flexure compensation NOT re-applied before exposure " << sub+1 + << " of " << expected << ": " << why; + this->broadcast.warning( function, message.str() ); + return ERROR; + }; + auto next_started = [this, sub]() { + std::lock_guard lock( this->camerad_mtx ); + return ( this->last_shutteropen || this->readouts_started != sub ); + }; + + if ( this->thread_error_manager.are_any_set() ) return skipped( "a thread error is set" ); + { + std::lock_guard lock( this->tcsd_mtx ); + if ( !this->daemon_manager.is_set( Sequencer::DAEMON_TCS ) || !this->tcsinfo.is_fresh() || + !std::isfinite( this->tcsinfo.zenithangle ) || !std::isfinite( this->tcsinfo.cassangle ) || + !std::isfinite( this->tcsinfo.pa ) ) { + return skipped( "TCS telemetry not fresh or not valid" ); } + } + if ( next_started() ) return skipped( "the next exposure has already started" ); + + const std::string comp = this->flexure_compensate_command(); + + message.str(""); message << "re-applying flexure compensation during readout " << sub << " of " + << expected << " for " << this->target.name; + logwrite( function, message.str() ); + + if ( this->flexured.command( comp ) != NO_ERROR ) return skipped( "flexured returned an error" ); this->is_flexure_default.store(false); - this->broadcast.notice( function, "flexure compensator set for "+activechans.str()); + if ( next_started() ) { + message.str(""); message << "flexure compensator re-applied, but exposure " << sub+1 << " of " << expected + << " had already started: the actuators moved during its first seconds"; + this->broadcast.warning( function, message.str() ); + return ERROR; + } + message.str(""); message << "flexure compensator re-applied before exposure " << sub+1 + << " of " << expected; + this->broadcast.notice( function, message.str() ); return NO_ERROR; } - /***** Sequencer::Sequence::flexure_set *************************************/ + /***** Sequencer::Sequence::flexure_reapply *********************************/ /***** Sequencer::Sequence::calib_set ***************************************/ @@ -2688,7 +2877,9 @@ namespace Sequencer { // so this loop exits after at most a few iterations. // if ( abort_during_run ) { - const auto drain_timeout = std::chrono::steady_clock::now() + std::chrono::seconds(5); + // 20 s: the readout wait may be inside a flexured re-apply (client + // timeout 6 s + 2 s, flexured itself up to ~11 s) when the cancel lands + const auto drain_timeout = std::chrono::steady_clock::now() + std::chrono::seconds(20); while ( this->thread_state_manager.is_set( Sequencer::THR_SEQUENCE_START ) && std::chrono::steady_clock::now() < drain_timeout ) { std::this_thread::sleep_for( std::chrono::milliseconds(20) ); @@ -2840,6 +3031,24 @@ namespace Sequencer { return NO_ERROR; } + // Flexure for the telescope position now. Only after camera_set has seen + // camerad ready: a REPEAT is accepted while the previous one is still + // integrating, and the actuators must not move under it. + // + try { + if ( this->flexure_set() != NO_ERROR ) { + this->broadcast.error( function, Sequencer::thread_names.at(THR_FLEXURE_SET)+" failed" ); + } + } + catch (const std::exception& e) { + this->broadcast.error( function, Sequencer::thread_names.at(THR_FLEXURE_SET)+" exception: "+std::string(e.what()) ); + return ERROR; + } + if ( this->cancel_flag.load() ) { + this->broadcast.notice(function, "cancelled repeat exposure"); + return NO_ERROR; + } + // Start the exposure in a thread... // auto start_exposure = std::async(std::launch::async, &Sequence::trigger_exposure, this); @@ -2890,6 +3099,13 @@ namespace Sequencer { // if ( this->tcs_preauth_time > 0 ) this->notify_tcs_next_target = true; else this->notify_tcs_next_target = false; + { + std::lock_guard lock(this->camerad_mtx); + this->last_shutteropen = false; // readout bookkeeping starts fresh with this expose command; + this->readout_armed = false; // the shutter is closed until camerad opens it for this one + this->readouts_started = 0; + this->readouts_expected = this->target.nexp; // the N sent below, fixed for this command + } this->arm_readout_flag = true; // enables the async_listener to look for the readout and clear the EXPOSE bit this->set_imgtype(); diff --git a/sequencerd/sequence.h b/sequencerd/sequence.h index 8df8f33b..e6a82a0c 100644 --- a/sequencerd/sequence.h +++ b/sequencerd/sequence.h @@ -431,6 +431,20 @@ namespace Sequencer { std::condition_variable acam_cv; std::mutex camerad_mtx; std::condition_variable camerad_cv; + // Readout bookkeeping for the current expose command, guarded by camerad_mtx + // and reset in trigger_exposure(). A sub-exposure's readout is counted in two + // steps. (1) camerad's shutter report goes from open to closed in a report + // that is not PAUSED: that integration has ended, so the count is armed + // (handletopic_camerad). (2) A PIXELCOUNT from the first half of a readout + // completes it (async listener). Any open or paused report disarms. + // camerad's reports arrive in the order it published them, so a late or + // repeated report cannot arm twice for one close, and a late PIXELCOUNT + // cannot count a sub-exposure that is integrating. The count never exceeds + // the integrations that have ended; a lost report only leaves one uncounted. + bool last_shutteropen = false; ///< SHUTTEROPEN in the last camerad report since the expose command + bool readout_armed = false; ///< an integration ended (open -> closed, not paused), its readout not yet counted + int readouts_started = 0; ///< sub-exposure readouts counted since the expose command + int readouts_expected = 0; ///< N of that expose command (this->target can be rewritten by manual commands) std::mutex slitd_mtx; std::condition_variable slitd_cv; std::mutex tcsd_mtx; @@ -619,6 +633,8 @@ namespace Sequencer { long camera_set(); ///< sets camera according to target entry params long slit_set(VirtualSlitMode mode=VSM_DATABASE); ///< sets slit according to target entry params and mode long move_to_target(); ///< sends request to TCS to move to target coords + long flexure_reapply( int sub, int expected ); ///< re-issues the flexure compensation between sub-exposures (never fatal) + std::string flexure_compensate_command(); ///< FLEXURED_COMPENSATE plus the active channels for the current target static void dothread_notify_tcs( Sequencer::Sequence &seq ); ///< like move_to_target but for preauth only long focus_set(); long flexure_set(); From 7f37a08c5fdcef64d8f0c61e44282bc4571553b0 Mon Sep 17 00:00:00 2001 From: Christoffer Fremling Date: Thu, 5 Nov 2026 10:48:42 -0800 Subject: [PATCH 2/3] sequencerd: pick the next target when the last readout starts When the last sub-exposure of a science target starts reading out, the target is completed and the next one is picked through the normal path (get_next, activate, publish, slew, acquire) while camerad reads out. A readout gate holds every camerad command for the new target (camera settings, expose) until camerad reports READY. There is no early pick in single-step or single-target mode, after a cancel, with a thread error set, or when the current row is a calibration. Readouts are counted from camerad's PIXELCOUNT messages, so the count cannot run ahead of the shutter; a PIXELCOUNT from the tail of the previous readout cannot end the next target's EXPOSE wait. camera_set runs in parallel with the slew and acquisition behind the readout gate and is joined just before the exposure; a target is never exposed unless its own camera settings were applied. camerad: READY is published when the last frame has arrived, a few ms before its FITS write ends, so a camera setting sent right after READY was refused (write in progress). A binning change or DMA remap now waits up to 2 s for a finishing write before refusing. Co-Authored-By: Claude Opus 5.5 --- camerad/astrocam.cpp | 10 +-- camerad/astrocam.h | 20 ++++++ sequencerd/sequence.cpp | 149 ++++++++++++++++++++++++++++++++++++++-- sequencerd/sequence.h | 8 +++ 4 files changed, 178 insertions(+), 9 deletions(-) diff --git a/camerad/astrocam.cpp b/camerad/astrocam.cpp index 6a5792d1..91d46f1d 100644 --- a/camerad/astrocam.cpp +++ b/camerad/astrocam.cpp @@ -1019,9 +1019,10 @@ namespace AstroCam { return ERROR; } - // Don't make any changes while frames are still being written. + // Don't make any changes while frames are still being written: give a + // write that is finishing (ms after READY) up to 2 s, then refuse. // - if ( this->any_writes_pending() ) { + if ( !this->wait_writes_done( 2000 ) ) { logwrite( function, "ERROR: cannot change binning while frame write is in progress" ); retstring="write_in_progress"; return ERROR; @@ -3602,9 +3603,10 @@ for ( const auto &dev : selectdev ) { return( ERROR ); } - // Don't remap the DMA buffer while frames are still being written from it. + // Don't remap the DMA buffer while frames are still being written from it: + // give a write that is finishing (ms after READY) up to 2 s, then refuse. // - if ( this->any_writes_pending() ) { + if ( !this->wait_writes_done( 2000 ) ) { logwrite( function, "ERROR: cannot remap DMA buffer while frame write is in progress" ); retstring="write_in_progress"; return ERROR; diff --git a/camerad/astrocam.h b/camerad/astrocam.h index 0901b248..12d29af5 100644 --- a/camerad/astrocam.h +++ b/camerad/astrocam.h @@ -964,6 +964,26 @@ std::vector> fitsinfo; /***** Interface::any_writes_pending ************************************/ + /***** Interface::wait_writes_done **************************************/ + /** + * @brief wait up to timeout_ms for every frame write to finish + * @return true if no writes are pending, false on timeout + * @details READY is published when the last frame has arrived, a few ms + * before its FITS write ends, so a command sent right after READY + * can find a write still in progress. + */ + inline bool wait_writes_done( int timeout_ms ) { + std::unique_lock lock( this->write_lock ); + return this->write_condition.wait_for( lock, std::chrono::milliseconds( timeout_ms ), [this]() { + for ( const auto &pending : this->writes_pending ) { + if ( !pending.empty() ) return false; + } + return true; + } ); + } + /***** Interface::wait_writes_done **************************************/ + + /***** Interface::write_pending *****************************************/ /** * @brief Set or clear the write pending state for a given exposure diff --git a/sequencerd/sequence.cpp b/sequencerd/sequence.cpp index 37552c68..717c6b0e 100644 --- a/sequencerd/sequence.cpp +++ b/sequencerd/sequence.cpp @@ -51,6 +51,13 @@ namespace Sequencer { if (jmessage.contains(Key::Camerad::READY)) { int isready = jmessage[Key::Camerad::READY].get(); this->can_expose.store(isready); + // The first READY after the next target was picked during a readout is + // the end of that readout (camerad reported READY=false when the + // exposure started and publishes in order): open the readout gate. + if ( isready && this->readout_gate_armed ) { + this->readout_gate_armed = false; + this->wait_state_manager.clear( Sequencer::SEQ_WAIT_READOUT ); + } } // Readout counting, step 1 of 2 (see the bookkeeping notes in sequence.h): // the shutter report going from open to closed, in a report that is not @@ -689,6 +696,16 @@ namespace Sequencer { this->is_ontarget.store(false); this->is_usercontinue.store(false); + // On every way out of this function except a cancel, do not report the + // sequence finished while camerad is still reading out a target that was + // completed at the start of its last readout. Declared after the + // ScopedStates above, so it runs before they set READY. + // + struct ReadoutGateOnExit { + Sequence &seq; + ~ReadoutGateOnExit() { try { seq.wait_for_readout_gate( "ending the sequence" ); } catch (...) { } } + } readout_gate_on_exit{ *this }; + // This is the main loop which runs as long as there are targets in the database, // or until cancelled. Or, this will run once in the case of single-target-mode, // which is invoked by storing an OBSID in the class variable single_obsid. @@ -745,6 +762,23 @@ namespace Sequencer { } this->publish_targetinfo(); // publish the now-active target + + // A target picked while the previous one is still reading out starts + // at once only if the telescope has to slew to it: a science row with + // coordinates and SLIT pointing. Anything else -- a calibration, a row + // without coordinates, ACAM pointing -- starts when that readout has + // finished, exactly as before. It is already 'active' while it waits, + // like every picked row. + // + { + const bool slews = !this->target.iscal + && !( this->target.ra_hms.empty() && this->target.dec_dms.empty() ) + && !( this->target.pointmode == Acam::POINTMODE_ACAM ); + if ( !slews && !this->wait_for_readout_gate( "starting "+std::string(this->target.name) ) ) { + this->broadcast.notice( function, "sequence cancelled" ); + return; + } + } } else // targetstate not TARGET_FOUND if ( targetstate == TargetInfo::TARGET_NOT_FOUND ) { // no target found is an automatic stop @@ -796,8 +830,9 @@ namespace Sequencer { // threads to start, pair their ThreadStatusBit with the function to call // + // camera_set is not here: it talks to camerad, so it runs just before + // the exposure, behind the readout gate (see "starting exposure"). worker_threads = { { THR_MOVE_TO_TARGET, std::bind(&Sequence::move_to_target, this) }, - { THR_CAMERA_SET, std::bind(&Sequence::camera_set, this) }, { THR_FOCUS_SET, std::bind(&Sequence::focus_set, this) }, { THR_CALIB_SET, std::bind(&Sequence::calib_set, this) }, // for CAL targets, slit comes from database, otherwise use VSM acquire position @@ -818,6 +853,19 @@ namespace Sequencer { worker_futures.emplace_back( thr, std::async(std::launch::async, func) ); } + // camera_set talks to camerad, so it waits for the readout gate, then runs + // in parallel with everything else (a channel power-up takes seconds) and + // is joined just before the exposure. Not for ACAM pointing, which never + // exposes here. + // + std::future camera_future; + if ( !( this->target.pointmode == Acam::POINTMODE_ACAM ) ) { + camera_future = std::async( std::launch::async, [this]() -> long { + if ( !this->wait_for_readout_gate( "setting the camera for "+std::string(this->target.name) ) ) return ABORT; + return this->camera_set(); + } ); + } + // wait for the threads to complete. these can be cancelled. // for ( auto &[thr, future] : worker_futures) { @@ -948,6 +996,36 @@ namespace Sequencer { } } + // Nothing reaches camerad before the previous target's readout has + // finished: when this target was picked at the start of that readout, + // the camera settings (camera_future) and the expose wait for READY. + // + if ( !this->wait_for_readout_gate( "exposing "+std::string(this->target.name) ) ) { + this->broadcast.notice( function, "sequence cancelled" ); + return; + } + if ( camera_future.valid() ) { + try { + long camera_error = camera_future.get(); + error |= camera_error; + // never expose unless this target's camera settings were applied + if ( camera_error != NO_ERROR ) { + this->broadcast.error( function, Sequencer::thread_names.at(THR_CAMERA_SET) + +( camera_error==ABORT ? " cancelled" : " failed" )+": not exposing" ); + return; + } + this->broadcast.notice( function, Sequencer::thread_names.at(THR_CAMERA_SET)+" ready" ); + } + catch (const std::exception& e) { + this->broadcast.error( function, Sequencer::thread_names.at(THR_CAMERA_SET)+" : "+std::string(e.what()) ); + return; + } + } + if ( this->cancel_flag.load() ) { + this->broadcast.notice( function, "sequence cancelled" ); + return; + } + logwrite( function, "starting exposure" ); ///< TODO @todo log to telemetry! // Start the exposure in a thread... @@ -996,8 +1074,15 @@ namespace Sequencer { // // While waiting, every counted sub-exposure readout (readouts_started; see // the bookkeeping notes in sequence.h) is acted on once: a readout that more sub-exposures - // follow gets the flexure compensation re-applied during it. + // follow gets the flexure compensation re-applied during it. When the + // LAST one starts reading out, this target is complete -- its + // integration has ended, and camerad took its header telemetry at + // exposure start and AIRMASS1/MJD1 at shutter close -- so the loop + // completes it and picks the next target now instead of after READY, + // unless single-step, a single target, a cancel or a thread error says + // otherwise. The readout gate keeps camerad untouched until READY. // + bool picked_during_readout = false; if (!this->is_science_frame_transfer) { logwrite( function, "waiting for readout" ); int readouts_handled = 0; @@ -1016,6 +1101,12 @@ namespace Sequencer { } const int sub = ++readouts_handled; // readout of sub-exposure number sub (1-based) has begun const int expected = this->readouts_expected; // the N sent to camerad, not the live row + if ( sub == expected && !this->target.iscal && !this->do_once.load() && this->single_obsid.empty() && + !this->cancel_flag.load() && !this->thread_error_manager.are_any_set() ) { + this->readout_gate_armed = true; // under camerad_mtx: no READY can slip in between + picked_during_readout = true; + break; + } lock.unlock(); if ( sub < expected ) { this->flexure_reapply( sub, expected ); @@ -1027,9 +1118,16 @@ namespace Sequencer { } lock.lock(); } - this->broadcast.notice( function, "done waiting for readout" ); + if ( picked_during_readout ) { + this->broadcast.notice( function, "last readout of "+std::string(this->target.name) + +" under way: completing it and picking the next target" ); + } + else { + this->broadcast.notice( function, "done waiting for readout" ); + } } - this->wait_state_manager.clear( Sequencer::SEQ_WAIT_READOUT ); + // after a pick the READOUT bit stays set until camerad's READY opens the gate + if ( !picked_during_readout ) this->wait_state_manager.clear( Sequencer::SEQ_WAIT_READOUT ); // Now that we're done waiting, check for errors or abort // @@ -1106,9 +1204,13 @@ namespace Sequencer { if (this->cancel_flag.load()) { logwrite(function, "sequence cancelled"); - return NO_ERROR; + return ABORT; // nothing was set: the caller must not expose } } + // camerad_mtx guards can_expose only; holding it across the camerad + // commands below would block the ZMQ subscriber thread (every topic) for + // their duration -- a channel power-up takes seconds + lock.unlock(); logwrite( function, "setting camera parameters"); @@ -2434,6 +2536,41 @@ namespace Sequencer { /***** Sequencer::Sequence::move_to_target **********************************/ + /***** Sequencer::Sequence::wait_for_readout_gate ***************************/ + /** + * @brief wait until the previous target's readout has finished + * @param[in] before what is waiting, for the log + * @return true to proceed, false if cancelled while waiting + * + * The gate is armed when the next target is picked at the start of the + * previous target's last readout (sequence_start) and opened by the first + * camerad READY report after that (handletopic_camerad). Returns at once + * when it is not armed, which is every case but that one. + */ + bool Sequence::wait_for_readout_gate( const std::string &before ) { + const std::string function("Sequencer::Sequence::wait_for_readout_gate"); + std::unique_lock lock( this->camerad_mtx ); + if ( !this->readout_gate_armed ) return true; + logwrite( function, "previous target still reading out: waiting before "+before ); + while ( this->readout_gate_armed && !this->cancel_flag.load() ) { + if ( !this->camerad_cv.wait_for( lock, std::chrono::seconds(30), + [this]() { return !this->readout_gate_armed || this->cancel_flag.load(); } ) ) { + logwrite( function, "still waiting for the previous readout -- requesting snapshot" ); + lock.unlock(); + this->request_snapshot(); + lock.lock(); + } + } + if ( this->readout_gate_armed ) { + logwrite( function, "cancelled while waiting for the previous readout" ); + return false; + } + logwrite( function, "previous readout finished" ); + return true; + } + /***** Sequencer::Sequence::wait_for_readout_gate ***************************/ + + /***** Sequencer::Sequence::dothread_notify_tcs *****************************/ /** * @brief notify TCS operator of next target coords for preauthorization, no move @@ -2988,6 +3125,8 @@ namespace Sequencer { this->is_ontarget.store(false); this->is_usercontinue.store(false); + if ( !this->wait_for_readout_gate( "repeating the exposure" ) ) return ERROR; + std::string targetstatus; this->target.get_specified_target( this->prev_single_obsid, targetstatus ); diff --git a/sequencerd/sequence.h b/sequencerd/sequence.h index e6a82a0c..e68ccf8c 100644 --- a/sequencerd/sequence.h +++ b/sequencerd/sequence.h @@ -445,6 +445,13 @@ namespace Sequencer { bool readout_armed = false; ///< an integration ended (open -> closed, not paused), its readout not yet counted int readouts_started = 0; ///< sub-exposure readouts counted since the expose command int readouts_expected = 0; ///< N of that expose command (this->target can be rewritten by manual commands) + // The readout gate, guarded by camerad_mtx. When the last readout of a + // target starts, the loop completes that target and picks the next one + // at once; nothing may then reach camerad (camera settings, an expose) + // until camerad reports READY again. Armed at that pick, opened by the + // first READY=true report after it (handletopic_camerad), and held + // across a stop or restart until then. + bool readout_gate_armed = false; std::mutex slitd_mtx; std::condition_variable slitd_cv; std::mutex tcsd_mtx; @@ -633,6 +640,7 @@ namespace Sequencer { long camera_set(); ///< sets camera according to target entry params long slit_set(VirtualSlitMode mode=VSM_DATABASE); ///< sets slit according to target entry params and mode long move_to_target(); ///< sends request to TCS to move to target coords + bool wait_for_readout_gate( const std::string &before ); ///< wait for the previous target's readout to finish; false if cancelled long flexure_reapply( int sub, int expected ); ///< re-issues the flexure compensation between sub-exposures (never fatal) std::string flexure_compensate_command(); ///< FLEXURED_COMPENSATE plus the active channels for the current target static void dothread_notify_tcs( Sequencer::Sequence &seq ); ///< like move_to_target but for preauth only From f51c3b5bc2756f4743250af54ce174bf6180c05b Mon Sep 17 00:00:00 2001 From: Christoffer Fremling Date: Thu, 5 Nov 2026 10:48:43 -0800 Subject: [PATCH 3/3] camerad, sequencerd: stop and abort an NEXP>1 series stop: the sequencer accepts it for the whole series of the current target (the EXPOSE wait covers only the first sub-exposure; camerad not READY covers the rest, except the previous target's last readout after a pick) and no longer clears a cancel. camerad ends the series after the frame in progress, cut short if it is integrating with 5 s or more left, instead of starting the next sub-exposure. abort no longer starts the next sub-exposure either. A cancelled exposure's remaining PIXELCOUNT messages cannot set the READOUT wait later. Co-Authored-By: Claude Opus 5.5 --- camerad/astrocam.cpp | 34 ++++++++++++++++++++++++++++------ camerad/astrocam.h | 2 +- sequencerd/sequence.cpp | 18 ++++++++++++++---- 3 files changed, 43 insertions(+), 11 deletions(-) diff --git a/camerad/astrocam.cpp b/camerad/astrocam.cpp index 91d46f1d..13a12dd8 100644 --- a/camerad/astrocam.cpp +++ b/camerad/astrocam.cpp @@ -957,6 +957,10 @@ namespace AstroCam { long Interface::do_abort() { std::string function = "AstroCam::Interface::do_abort"; std::stringstream message; + // No sub-exposure follows an abort: clearing exposure_pending below wakes + // dothread_monitor_exposure_pending, which starts the next sub-exposure + // whenever nexp is still > 0 after its decrement. + this->nexp.store(0); // int this_expbuf = this->get_expbuf(); for ( const auto &dev : this->active_devnums ) { this->exposure_pending( dev, false ); @@ -2714,7 +2718,7 @@ namespace AstroCam { if ( interface.nexp.load() > 0 ) { message.str(""); message << "starting next exposure. " << interface.nexp << " remaining"; interface.broadcast.notice( function, message.str() ); - interface.do_expose(interface.nexp); + interface.do_expose(interface.nexp, true); // continuation: nexp already holds what is left } else { interface.status.can_expose.store(true); @@ -2769,7 +2773,7 @@ namespace AstroCam { * @return ERROR or NO_ERROR * */ - long Interface::do_expose(int nexp_in) { + long Interface::do_expose(int nexp_in, bool continuation) { const std::string function("AstroCam::Interface::do_expose"); std::stringstream message; std::string devstr; @@ -2793,8 +2797,6 @@ namespace AstroCam { } } - this->nexp.store(nexp_in); - // Different controllers may have different rules about when to set and clear their // exposure_pending flag (depending on whether or not they support frame transfer), but // if an exposure is pending for any controller then do not start a new exposure. @@ -2808,6 +2810,11 @@ namespace AstroCam { return(ERROR); } + // Only now, and only for a new expose command: a refused expose must not + // change how many sub-exposures the running series has left, and a + // continuation must not undo a stop that lowered nexp since it was read. + if ( !continuation ) this->nexp.store(nexp_in); + // When using the internal ARC shutter control you cannot overlap exposures // with readout because it would require sending a command which cannot // be handled during readout. @@ -4429,9 +4436,18 @@ logwrite(function, message.str()); const std::string function("AstroCam::Interface::stop_exposure"); std::stringstream message; - // nothing to do if not exposing - // + // An NEXP>1 series ends after the frame in progress, whatever that frame + // is doing -- integrating, reading out, or about to start -- so no further + // sub-exposure begins. That frame is also cut short below when it is + // integrating with >= 5 s left. + const bool ends_series = ( this->nexp.load() > 1 ); + if ( ends_series ) this->nexp.store(1); if ( ! this->exposure_pending() ) { + if ( ends_series ) { + logwrite( function, "between sub-exposures: the series ends after the next frame" ); + retstring="series_ends"; + return NO_ERROR; + } logwrite( function, "not exposing" ); return NOTHING; } @@ -4442,6 +4458,12 @@ logwrite(function, message.str()); // long remaining_time = this->camera.shutter_timer.get_remaining(); + if ( remaining_time < 5000 && ends_series ) { + logwrite( function, "too late to cut this frame short: the series ends after it" ); + retstring="series_ends"; + return NO_ERROR; + } + if ( remaining_time < 5000 ) { message << "ERROR cannot stop exposure time with less than 5 s exptime remaining" << " (" << remaining_time << ")"; diff --git a/camerad/astrocam.h b/camerad/astrocam.h index 12d29af5..f225ab9a 100644 --- a/camerad/astrocam.h +++ b/camerad/astrocam.h @@ -1260,7 +1260,7 @@ std::vector> fitsinfo; void set_imagesize(int rowsin, int colsin, int* status); long expose(std::string nexp_in); - long do_expose(int nexp_in); + long do_expose(int nexp_in, bool continuation=false); ///< continuation: the next sub-exposure of a running series double get_live_airmass(); ///< latest airmass from cached tcsd telemetry, or NAN void get_live_provenance( std::string &githash, std::string &buildtime ); ///< build provenance of all running daemons long native(std::string cmdstr); diff --git a/sequencerd/sequence.cpp b/sequencerd/sequence.cpp index 717c6b0e..8376d860 100644 --- a/sequencerd/sequence.cpp +++ b/sequencerd/sequence.cpp @@ -1054,6 +1054,7 @@ namespace Sequencer { // if ( this->cancel_flag.load() ) { this->broadcast.notice( function, "exposure cancelled" ); + this->arm_readout_flag = false; // the stopped frame's PIXELCOUNTs must not set READOUT later error = this->target.update_state( Sequencer::TARGET_UNASSIGNED ); message.str(""); message << ( error==NO_ERROR ? "" : "ERROR " ) << "marking target " << this->target.name << " id " << this->target.obsid << " order " << this->target.obsorder @@ -3060,15 +3061,24 @@ namespace Sequencer { ScopedState thr_state( this->thread_state_manager, Sequencer::THR_STOP_EXPOSURE ); - // This function is only used while exposing + // Only while an exposure command of the current target is running. For + // NEXP>1 that is the whole series: the EXPOSE bit covers only the first + // sub-exposure; after it, camerad not READY (and not the previous + // target's last readout after a pick, which has nothing left to stop) + // covers the rest. A stale READOUT bit is not evidence. camerad ends the + // series after the frame in progress, cut short if it is integrating + // with >= 5 s left. A stop never clears a cancel. // - if ( ! this->wait_state_manager.is_set( Sequencer::SEQ_WAIT_EXPOSE ) ) { + bool series_running = false; + { + std::lock_guard lock( this->camerad_mtx ); + series_running = !this->can_expose.load() && !this->readout_gate_armed; + } + if ( ! this->wait_state_manager.is_set( Sequencer::SEQ_WAIT_EXPOSE ) && !series_running ) { this->broadcast.notice(function, "not currently exposing"); return; } - this->cancel_flag.store(false); - // Send command to the camera to stop the exposure. // std::string reply;