From c5963fdaa93777f149dc875c7dd4a586da8aba49 Mon Sep 17 00:00:00 2001 From: David Hale Date: Wed, 29 Jul 2026 16:34:46 -0700 Subject: [PATCH 1/6] Sequencer::Sequence::calib_set sets only when needed previously each target set all calib hardware this now stores published tcs, calib, and power telemetry, used to check states before setting. resolves issue 467 --- common/calib_defs.h | 189 +++++++++++++++ common/fits_header_defs.h | 2 + common/message_keys.h | 2 + sequencerd/sequence.cpp | 129 ++++++++-- sequencerd/sequence.h | 14 ++ sequencerd/sequencer_interface.cpp | 21 +- sequencerd/sequencer_interface.h | 375 ++++++++++++++++++++++++++++- sequencerd/sequencerd.cpp | 4 +- tcsd/tcs_interface.cpp | 10 +- tcsd/tcs_interface.h | 13 +- 10 files changed, 720 insertions(+), 39 deletions(-) create mode 100644 common/calib_defs.h diff --git a/common/calib_defs.h b/common/calib_defs.h new file mode 100644 index 00000000..91337de7 --- /dev/null +++ b/common/calib_defs.h @@ -0,0 +1,189 @@ +/** + * @file calib_defs.h + * @brief names of the calibration devices, and their bindings to message keys + * @author David Hale + * + * Three distinct kinds of information are involved in naming a calibration + * device, and they are easy to conflate: + * + * the device name the vocabulary used by the cfg files, by the commands + * sent to the owning daemon, and as the map key in every + * daemon that handles the device + * the message key what the providing daemon publishes it under + * the hardware id how the owning daemon addresses the hardware, e.g. the + * TCS lamp number or the modulator's Arduino channel + * + * The names live in the Lamp namespaces below. The message keys live in + * message_keys.h, where their values are arbitrary identifiers. Several of them + * happen to hold the same string as the device name today, but that is not + * something to rely on, so the Device tables bind the two explicitly rather than + * leaving the binding implicit in a constant's value. + * + * The hardware id is wiring, not vocabulary, so it stays in the owning daemon's + * config file -- LAMPMOD_MOD in calibd.cfg.in remains the place that says which + * Arduino channel a modulator is on. It appears in the tables only because a + * consumer which knows a device by name sometimes has to command it by number. + * + * Everything that names one of these devices derives it from this file: + * TCS::TcsInfo, Sequencer::CalibrationTarget, and the sequencer's telemetry + * caches. + * + */ +#pragma once + +#include +#include + +#include "message_keys.h" + +/***** Lamp *******************************************************************/ +/** + * @namespace Lamp + * @brief names of the calibration lamps and their modulators + * + */ +namespace Lamp { + + /** @brief internal calibration lamps. These are powerd plug names. */ + namespace Cal { + inline const std::string THAR = "LAMPTHAR"; + inline const std::string FEAR = "LAMPFEAR"; + inline const std::string BLUC = "LAMPBLUC"; + inline const std::string REDC = "LAMPREDC"; + } + + /** @brief TCS dome lamps. These are the names tcsd accepts for TCSD_LAMP. */ + namespace Dome { + inline const std::string LO = "LO"; + inline const std::string HI = "HI"; + inline const std::string ARC = "ARC"; + inline const std::string ULTRA = "ULTRA"; + } + + /** @brief lamp modulators. These are the names configured in calibd. */ + namespace Mod { + inline const std::string FEAR = "MODFEAR"; + inline const std::string BLETA = "MODBLETA"; + inline const std::string RDETA = "MODRDETA"; + inline const std::string RDCON = "MODRDCON"; + inline const std::string BLCON = "MODBLCON"; + inline const std::string THAR = "MODTHAR"; + } + +} +/***** Lamp *******************************************************************/ + + +/***** CalibDefs **************************************************************/ +/** + * @namespace CalibDefs + * @brief binds calibration device names to message keys and hardware ids + * + */ +namespace CalibDefs { + + /***** CalibDefs::Device ****************************************************/ + /** + * @struct Device + * @brief binds a device name to the key its provider publishes and to the + * hardware identifier used to command it + * + */ + struct Device { + std::string name; ///< device name used by cfg files, commands and map keys + std::string jkey; ///< key published by the providing daemon + int num; ///< hardware id (TCS lamp number, modulator channel), 0 if unused + }; + /***** CalibDefs::Device ****************************************************/ + + + /***** CalibDefs::domelamps *************************************************/ + /** + * @brief TCS dome lamps + * @details Listed in TCS lamp-number order, which is also the order of the + * dome lamp tokens in the sequencer's CAL_TARGET config lines. + * @return reference to the vector of dome lamp Devices + * + */ + inline const std::vector& domelamps() { + static const std::vector devices = { + { Lamp::Dome::LO, Key::Tcsd::LAMP_LO, 1 }, + { Lamp::Dome::HI, Key::Tcsd::LAMP_HI, 2 }, + { Lamp::Dome::ARC, Key::Tcsd::LAMP_ARC, 3 }, + { Lamp::Dome::ULTRA, Key::Tcsd::LAMP_ULTRA, 4 } + }; + return devices; + } + /***** CalibDefs::domelamps *************************************************/ + + + /***** CalibDefs::callamps **************************************************/ + /** + * @brief internal calibration lamps, which are powerd plugs + * @details Listed in the order of the lamp tokens in the sequencer's + * CAL_TARGET config lines. There is no hardware id here because + * powerd resolves the plug name to a unit and outlet from its own + * NPS_PLUG config. + * @return reference to the vector of calibration lamp Devices + * + */ + inline const std::vector& callamps() { + static const std::vector devices = { + { Lamp::Cal::THAR, Key::Powerd::LAMPTHAR, 0 }, + { Lamp::Cal::FEAR, Key::Powerd::LAMPFEAR, 0 }, + { Lamp::Cal::BLUC, Key::Powerd::LAMPBLUC, 0 }, + { Lamp::Cal::REDC, Key::Powerd::LAMPREDC, 0 } + }; + return devices; + } + /***** CalibDefs::callamps **************************************************/ + + + /***** CalibDefs::modulators ************************************************/ + /** + * @brief lamp modulators + * @details Listed in calibd channel order, which is also the order of the + * modulator tokens in the sequencer's CAL_TARGET config lines. + * This must agree with the LAMPMOD_MOD lines in calibd.cfg.in, + * which is where the channel assignment actually lives. calibd + * publishes each modulator's state under its name, so a consumer + * joins to that telemetry on the name and needs the channel only + * to build a CALIBD_LAMPMOD command. + * Channels 7 and 8 are configured as placeholders and are not + * listed until they are assigned to real modulators. + * @return reference to the vector of modulator Devices + * + */ + inline const std::vector& modulators() { + static const std::vector devices = { + { Lamp::Mod::FEAR, Key::Calibd::MODFEAR, 1 }, + { Lamp::Mod::BLETA, Key::Calibd::MODBLETA, 2 }, + { Lamp::Mod::RDETA, Key::Calibd::MODRDETA, 3 }, + { Lamp::Mod::RDCON, Key::Calibd::MODRDCON, 4 }, + { Lamp::Mod::BLCON, Key::Calibd::MODBLCON, 5 }, + { Lamp::Mod::THAR, Key::Calibd::MODTHAR, 6 } + }; + return devices; + } + /***** CalibDefs::modulators ************************************************/ + + + /***** CalibDefs::find ******************************************************/ + /** + * @brief look up a Device by name + * @param[in] devices one of the vectors defined above + * @param[in] name device name to find + * @return pointer to the Device, or nullptr if name is not defined + * + */ + inline const Device* find( const std::vector &devices, + const std::string &name ) { + for ( const auto &dev : devices ) { + if ( dev.name == name ) return &dev; + } + return nullptr; + } + /***** CalibDefs::find ******************************************************/ + +} +/***** CalibDefs **************************************************************/ diff --git a/common/fits_header_defs.h b/common/fits_header_defs.h index e62fc210..c59ca751 100644 --- a/common/fits_header_defs.h +++ b/common/fits_header_defs.h @@ -38,8 +38,10 @@ namespace FitsHeaderKeys { { Key::Calibd::MODTHAR.c_str(), "", "ThAr lamp modulator pow dut per" }, { Key::Calibd::MODBLCON.c_str(), "", "Blue continuum modulator pow dut per" }, { Key::Calibd::MODBLBYP.c_str(), "", "Blue bypass modulator pow dut per" }, + { Key::Calibd::MODBLETA.c_str(), "", "Blue ETA modulator pow dut per" }, { Key::Calibd::MODRDCON.c_str(), "", "Red continuum modulator pow dut per" }, { Key::Calibd::MODRDBYP.c_str(), "", "Red bypass modulator pow dut per" }, + { Key::Calibd::MODRDETA.c_str(), "", "Red ETA modulator pow dut per" }, { Key::Calibd::CALCOVER.c_str(), "", "calib cover state" }, { Key::Calibd::CALDOOR.c_str(), "", "calib door state" } }; diff --git a/common/message_keys.h b/common/message_keys.h index 4fa79450..a18d5982 100644 --- a/common/message_keys.h +++ b/common/message_keys.h @@ -149,8 +149,10 @@ namespace Key { inline const std::string MODTHAR = "MODTHAR"; inline const std::string MODBLCON = "MODBLCON"; inline const std::string MODBLBYP = "MODBLBYP"; + inline const std::string MODBLETA = "MODBLETA"; inline const std::string MODRDCON = "MODRDCON"; inline const std::string MODRDBYP = "MODRDBYP"; + inline const std::string MODRDETA = "MODRDETA"; inline const std::string CALCOVER = "CALCOVER"; inline const std::string CALDOOR = "CALDOOR"; } diff --git a/sequencerd/sequence.cpp b/sequencerd/sequence.cpp index 8113affb..da4e4d9a 100644 --- a/sequencerd/sequence.cpp +++ b/sequencerd/sequence.cpp @@ -111,7 +111,7 @@ namespace Sequencer { * */ void Sequence::handletopic_tcsd(const nlohmann::json &jmessage) { - // the completed target table contains TCS data now + // load the completed target table with TCS telemetry this->target.column_from_json( "TELRA", Key::Tcsd::TELRA, jmessage ); this->target.column_from_json( "TELDECL", Key::Tcsd::TELDEC, jmessage ); this->target.column_from_json( "ALT", Key::Tcsd::ALT, jmessage ); @@ -119,12 +119,43 @@ namespace Sequencer { this->target.column_from_json( "AIRMASS", Key::Tcsd::AIRMASS, jmessage ); this->target.column_from_json( "CASANGLE", Key::Tcsd::CASANGLE, jmessage ); + // Store under the mutex so the writes are visible to any waiter's predicate + // when it re-acquires tcsd_mtx inside tcsd_cv.wait(). std::lock_guard lock(tcsd_mtx); + this->tcsinfo.store( jmessage ); this->tcsd_cv.notify_all(); } /***** Sequencer::Sequence::handletopic_tcsd *******************************/ + /***** Sequencer::Sequence::handletopic_calibd *****************************/ + /** + * @brief handles Topic::CALIBD telemetry + * @param[in] jmessage subscribed-received JSON message + * + */ + void Sequence::handletopic_calibd(const nlohmann::json &jmessage) { + std::lock_guard lock(calibd_mtx); + this->calibinfo.store( jmessage ); + this->calibd_cv.notify_all(); + } + /***** Sequencer::Sequence::handletopic_calibd *****************************/ + + + /***** Sequencer::Sequence::handletopic_powerd *****************************/ + /** + * @brief handles Topic::POWERD telemetry + * @param[in] jmessage subscribed-received JSON message + * + */ + void Sequence::handletopic_powerd(const nlohmann::json &jmessage) { + std::lock_guard lock(powerd_mtx); + this->powerinfo.store( jmessage ); + this->powerd_cv.notify_all(); + } + /***** Sequencer::Sequence::handletopic_powerd *****************************/ + + /***** Sequencer::Sequence::handletopic_acamd ******************************/ /** * @brief handles Topic::ACAMD telemetry @@ -193,6 +224,8 @@ namespace Sequencer { jmessage[Daemon::SLICECAMD] = true; jmessage[Daemon::SLITD] = true; jmessage[Daemon::TCSD] = true; + jmessage[Daemon::CALIBD] = true; + jmessage[Daemon::POWERD] = true; this->publisher->publish( jmessage, Topic::SNAPSHOT ); } /***** Sequencer::Sequence::request_snapshot *******************************/ @@ -1089,12 +1122,21 @@ namespace Sequencer { this->daemon_manager.clear( Sequencer::DAEMON_POWER ); // powerd not ready + // Discard the cached power state. powerd publishes only when its state + // changes, so nothing else would tell me that what I have is no longer + // current. request_snapshot() below refills it. + { + std::lock_guard lock(this->powerd_mtx); + this->powerinfo.invalidate(); + } + if ( this->reopen_hardware(this->powerd, POWERD_REOPEN, 10000 ) != NO_ERROR ) { logwrite( function, "ERROR initializing power control" ); throw std::runtime_error("could not initialize power control"); } this->daemon_manager.set( Sequencer::DAEMON_POWER ); // powerd ready + this->request_snapshot(); return NO_ERROR; } /***** Sequencer::Sequence::power_init **************************************/ @@ -1504,6 +1546,14 @@ namespace Sequencer { this->daemon_manager.clear( Sequencer::DAEMON_CALIB ); + // Discard the cached calib state. calibd publishes only when its state + // changes, so nothing else would tell me that what I have is no longer + // current. request_snapshot() below refills it. + { + std::lock_guard lock(this->calibd_mtx); + this->calibinfo.invalidate(); + } + ScopedState thr_state( thread_state_manager, Sequencer::THR_CALIB_INIT ); ScopedState wait_state( wait_state_manager, Sequencer::SEQ_WAIT_CALIB ); @@ -1558,6 +1608,7 @@ namespace Sequencer { // calibd is ready this->daemon_manager.set( Sequencer::DAEMON_CALIB ); + this->request_snapshot(); this->thread_error_manager.clear( THR_CALIB_INIT ); @@ -2419,24 +2470,48 @@ namespace Sequencer { this->broadcast.notice( function, "configuring calibrator for "+calname); - // set the calib door and cover + // set the calib door and cover if needed. CALIBD_SET moves both in one + // command, so this can only be skipped when both are already set. // + bool doorcover_isset = false; + { + std::lock_guard lock(this->calibd_mtx); + doorcover_isset = ( this->calibinfo.is_valid() && + this->calibinfo.caldoor == (calinfo.caldoor?1:0) && + this->calibinfo.calcover == (calinfo.calcover?1:0) ); + } + std::stringstream cmd; - cmd.str(""); cmd << CALIBD_SET - << " door=" << ( calinfo.caldoor ? "open" : "close" ) - << " cover=" << ( calinfo.calcover ? "open" : "close" ); + if ( doorcover_isset ) { + logwrite( function, "calib door and cover already set" ); + } + else { + cmd.str(""); cmd << CALIBD_SET + << " door=" << ( calinfo.caldoor ? "open" : "close" ) + << " cover=" << ( calinfo.calcover ? "open" : "close" ); - logwrite( function, "calib: "+cmd.str() ); - if ( !this->cancel_flag.load() && - this->calibd.command_timeout( cmd.str(), CALIBD_SET_TIMEOUT ) != NO_ERROR ) { - this->broadcast.error( function, "moving calib door and/or cover" ); - throw std::runtime_error("moving calib door and/or cover"); + logwrite( function, "calib: "+cmd.str() ); + if ( !this->cancel_flag.load() && + this->calibd.command_timeout( cmd.str(), CALIBD_SET_TIMEOUT ) != NO_ERROR ) { + this->broadcast.error( function, "moving calib door and/or cover" ); + throw std::runtime_error("moving calib door and/or cover"); + } } - // set the internal calibration lamps + // set the internal calibration lamps if needed // for ( const auto &[lamp,state] : calinfo.lamp ) { if ( this->cancel_flag.load() ) break; + // skip if lamp is already in the desired state + { + std::lock_guard lock(this->powerd_mtx); + if (this->powerinfo.is_valid() && + this->powerinfo.plug_state(lamp) == (state?1:0)) { + logwrite(function, "cal lamp "+lamp+" already "+(state?"on":"off")); + continue; + } + } + // otherwise send the command cmd.str(""); cmd << lamp << " " << (state?"on":"off"); message.str(""); message << "power " << cmd.str(); logwrite( function, message.str() ); @@ -2447,10 +2522,20 @@ namespace Sequencer { } } - // set the dome lamps + // set the dome lamps if needed // for ( const auto &[lampname,state] : calinfo.domelamp ) { if ( this->cancel_flag.load() ) break; + // skip if lamp is already in the desired state + { + std::lock_guard lock(this->tcsd_mtx); + if (this->tcsinfo.is_fresh() && + this->tcsinfo.domelamp_state(lampname) == (state?1:0)) { + logwrite(function, "dome lamp "+lampname+" already "+(state?"on":"off")); + continue; + } + } + // otherwise send command to tcsd cmd.str(""); cmd << TCSD_LAMP << " " << lampname << " " << (state==1?"on":"off"); if ( this->tcsd.command( cmd.str() ) != NO_ERROR ) { logwrite(function, "ERROR "+cmd.str()); @@ -2458,11 +2543,27 @@ namespace Sequencer { } } - // set the lamp modulators + // set the lamp modulators if needed // for ( const auto &[mod,state] : calinfo.lampmod ) { if ( this->cancel_flag.load() ) break; - cmd.str(""); cmd << CALIBD_LAMPMOD << " " << mod << " " << (state?1:0) << " 1000"; + // skip if already in the desired state + { + std::lock_guard lock(this->calibd_mtx); + if (this->calibinfo.is_valid() && + this->calibinfo.lampmod_state(mod) == (state?1:0)) { + logwrite(function, "lamp modulator "+mod+" already "+(state?"on":"off")); + continue; + } + } + // otherwise send the command. calibd publishes modulators by name but + // takes the channel number, which comes from the shared table. + const auto *dev = CalibDefs::find( CalibDefs::modulators(), mod ); + if ( dev == nullptr ) { + this->broadcast.error( function, "unknown lamp modulator "+mod ); + throw std::runtime_error("unknown lamp modulator "+mod); + } + cmd.str(""); cmd << CALIBD_LAMPMOD << " " << dev->num << " " << (state?1:0) << " 1000"; if ( this->calibd.command( cmd.str() ) != NO_ERROR ) { this->broadcast.error( function, cmd.str() ); throw std::runtime_error("setting lamp modulator "+cmd.str()); diff --git a/sequencerd/sequence.h b/sequencerd/sequence.h index 30990eb0..6b9dd986 100644 --- a/sequencerd/sequence.h +++ b/sequencerd/sequence.h @@ -369,6 +369,10 @@ namespace Sequencer { [this](const nlohmann::json &msg) { handletopic_slitd(msg); } ) }, { Topic::TCSD, std::function( [this](const nlohmann::json &msg) { handletopic_tcsd(msg); } ) }, + { Topic::CALIBD, std::function( + [this](const nlohmann::json &msg) { handletopic_calibd(msg); } ) }, + { Topic::POWERD, std::function( + [this](const nlohmann::json &msg) { handletopic_powerd(msg); } ) }, { Topic::CAMERAD, std::function( [this](const nlohmann::json &msg) { handletopic_camerad(msg); } ) } }; @@ -423,6 +427,10 @@ namespace Sequencer { std::condition_variable slitd_cv; std::mutex tcsd_mtx; std::condition_variable tcsd_cv; + std::mutex calibd_mtx; + std::condition_variable calibd_cv; + std::mutex powerd_mtx; + std::condition_variable powerd_cv; std::mutex wait_mtx; std::condition_variable cv; std::mutex cv_mutex; @@ -444,6 +452,10 @@ namespace Sequencer { CalibrationTarget caltarget; + TcsInfo tcsinfo; ///< most recent tcsd telemetry, guarded by tcsd_mtx + CalibInfo calibinfo; ///< most recent calibd telemetry, guarded by calibd_mtx + PowerInfo powerinfo; ///< most recent powerd telemetry, guarded by powerd_mtx + std::string single_obsid; ///< obsid for single-target GETONE command std::string prev_single_obsid; ///< the previous single_obsid, used for REPEAT @@ -492,6 +504,8 @@ namespace Sequencer { void handletopic_slicecamd( const nlohmann::json &jmessage ); void handletopic_slitd( const nlohmann::json &jmessage ); void handletopic_tcsd( const nlohmann::json &jmessage ); + void handletopic_calibd( const nlohmann::json &jmessage ); + void handletopic_powerd( const nlohmann::json &jmessage ); void publish_snapshot(); void request_snapshot(); void publish_seqstate(); diff --git a/sequencerd/sequencer_interface.cpp b/sequencerd/sequencer_interface.cpp index a838d282..f04e2da2 100644 --- a/sequencerd/sequencer_interface.cpp +++ b/sequencerd/sequencer_interface.cpp @@ -991,19 +991,22 @@ namespace Sequencer { info.channel_active[chans.at(i)] = on_off(tokens.at(3+i)); } - // tokens 7-10 are lamp states LAMPTHAR, LAMPFEAR, LAMPBLUC, LAMPREDC - for (size_t i=0; i < 4; i++) { - info.lamp[lampnames.at(i)] = on_off(tokens.at(7+i)); - } - - // tokens 11-14 are dome lamps: LO, HI, ARC, ULTRA + // tokens 7-10 are the internal calibration lamp states, and tokens 11-14 + // the dome lamp states, each in the order the shared tables list them + // + const auto &callamps = CalibDefs::callamps(); + const auto &domelamps = CalibDefs::domelamps(); for (size_t i=0; i < 4; i++) { - info.domelamp[domelampnames.at(i)] = on_off(tokens.at(11+i)); + info.lamp[callamps.at(i).name] = on_off(tokens.at(7+i)); + info.domelamp[domelamps.at(i).name] = on_off(tokens.at(11+i)); } - // tokens 15-20 -- modulator numbers are {1:6} + // tokens 15-20 are the modulator states, in modulator channel order. + // Index by name, which is how calibd publishes them. + // + const auto &modulators = CalibDefs::modulators(); for (size_t i=0; i<6; i++) { - info.lampmod[i+1] = on_off(tokens.at(15+i)); + info.lampmod[modulators.at(i).name] = on_off(tokens.at(15+i)); } // token[21] is FITS IMGTYPE diff --git a/sequencerd/sequencer_interface.h b/sequencerd/sequencer_interface.h index 732dc8e7..26e233c9 100644 --- a/sequencerd/sequencer_interface.h +++ b/sequencerd/sequencer_interface.h @@ -23,6 +23,7 @@ #include #include "acam_interface_shared.h" +#include "calib_defs.h" #define ERROR_TARGETLIST_BAD_HEADER 1001 ///< TODO change this @@ -83,6 +84,17 @@ namespace Sequencer { const std::string IMGTYPE_DARK="DARK"; const std::string IMGTYPE_SCI="SCI"; + namespace Chan { + inline const std::string U = "U"; + inline const std::string G = "G"; + inline const std::string R = "R"; + inline const std::string I = "I"; + } + + // The calibration lamp, dome lamp and modulator names are shared with tcsd, + // calibd and powerd, so they live in common/calib_defs.h as Lamp::Cal, + // Lamp::Dome and Lamp::Mod. + /***** Sequencer::PowerSwitch ***********************************************/ /** @@ -132,9 +144,7 @@ namespace Sequencer { class CalibrationTarget { public: CalibrationTarget() : - chans { "U", "G", "R", "I" }, - lampnames { "LAMPTHAR", "LAMPFEAR", "LAMPBLUC", "LAMPREDC" }, - domelampnames { "LO", "HI", "ARC", "ULTRA" } { } // this is how they are ordered in the TCS + chans { Chan::U, Chan::G, Chan::R, Chan::I } { } ///< struct holds all calibration parameters not in the target database typedef struct { @@ -143,9 +153,9 @@ namespace Sequencer { std::map channel_active; // true=on bool caldoor; // true=open bool calcover; // true=open - std::map lamp; // true=on - std::map domelamp; // true=on - std::map lampmod; // true=on + std::map lamp; // true=on, indexed by Lamp::Cal name + std::map domelamp; // true=on, indexed by Lamp::Dome name + std::map lampmod; // true=on, indexed by Lamp::Mod name } calinfo_t; ///< parses config file @@ -166,12 +176,361 @@ namespace Sequencer { private: std::unordered_map calmap; std::vector chans; - std::vector lampnames; - std::vector domelampnames; }; /***** Sequencer::CalibrationTarget *****************************************/ + /***** Sequencer::TcsInfo ***************************************************/ + /** + * @class TcsInfo + * @brief holds the most recently published tcsd telemetry + * + * There is one member here for each key published by + * TCS::Interface::publish_snapshot. Written by the subscriber thread and read + * by the sequence threads; all access is serialized by the caller under + * Sequence::tcsd_mtx. + * + */ + class TcsInfo { + public: + TcsInfo() { + // the lamp names and their message keys are defined in calib_defs.h + // + for ( const auto &dev : CalibDefs::domelamps() ) { + this->domelamp[dev.name] = -1; + } + this->init(); + } + + bool isopen; ///< is tcsd's connection to the TCS open? + std::string tcsname; ///< name of connected TCS { real sim } + std::string motion; ///< one of the TCS_MOTION*_STRs from tcs_constants.h + std::string ha; ///< hh:mm:ss.s + std::string ra_hms; ///< hh:mm:ss.ss + std::string dec_dms; ///< dd:mm:ss.ss + std::string domeshutters; ///< { open closed } + + double ra_h; ///< h.hhhhh (decimal hours) + double dec_d; ///< d.ddddd (decimal degrees) + double azimuth; + double alt; + double zenithangle; + double domeazimuth; + double airmass; + double focus; + double offsetra; + double offsetdec; + double cassangle; + double pa; + + int64_t pubtime; ///< time (usec) that tcsd published this state + + std::map domelamp; ///< 1=on 0=off -1=undefined, indexed by Lamp::Dome name + + /** + * @brief initialize all class member variables to "non-values" + */ + void init() { + isopen=false; + tcsname.clear(); + motion.clear(); + ha.clear(); + ra_hms.clear(); + dec_dms.clear(); + domeshutters.clear(); + ra_h=NAN; + dec_d=NAN; + azimuth=NAN; + alt=NAN; + zenithangle=NAN; + domeazimuth=NAN; + airmass=NAN; + focus=NAN; + offsetra=NAN; + offsetdec=NAN; + cassangle=NAN; + pa=NAN; + pubtime=0; + for ( auto &[name,state] : domelamp ) { state=-1; } + } + + /***** Sequencer::TcsInfo::store ****************************************/ + /** + * @brief store a received tcsd telemetry message + * @details Everything is erased first because a snapshot is all or + * nothing; a key missing from this message must not leave the + * value from an earlier one behind. extract_telemetry_value + * leaves its out-param alone on a missing key, a type + * mismatch, or a null (which is how a NAN arrives), so the + * non-values set by init() survive those cases. + * @param[in] jmessage subscribed-received JSON message + * + */ + void store( const nlohmann::json &jmessage ) { + this->init(); + + Common::extract_telemetry_value( jmessage, Key::Tcsd::ISOPEN, this->isopen ); + Common::extract_telemetry_value( jmessage, Key::Tcsd::TCSNAME, this->tcsname ); + Common::extract_telemetry_value( jmessage, Key::Tcsd::MOTION, this->motion ); + Common::extract_telemetry_value( jmessage, Key::Tcsd::HA, this->ha ); + Common::extract_telemetry_value( jmessage, Key::Tcsd::TELRA, this->ra_hms ); + Common::extract_telemetry_value( jmessage, Key::Tcsd::TELDEC, this->dec_dms ); + Common::extract_telemetry_value( jmessage, Key::Tcsd::DOMESHUT, this->domeshutters ); + Common::extract_telemetry_value( jmessage, Key::Tcsd::TELRA_H, this->ra_h ); + Common::extract_telemetry_value( jmessage, Key::Tcsd::TELDEC_D, this->dec_d ); + Common::extract_telemetry_value( jmessage, Key::Tcsd::AZ, this->azimuth ); + Common::extract_telemetry_value( jmessage, Key::Tcsd::ALT, this->alt ); + Common::extract_telemetry_value( jmessage, Key::Tcsd::ZENANGLE, this->zenithangle ); + Common::extract_telemetry_value( jmessage, Key::Tcsd::DOMEAZ, this->domeazimuth ); + Common::extract_telemetry_value( jmessage, Key::Tcsd::AIRMASS, this->airmass ); + Common::extract_telemetry_value( jmessage, Key::Tcsd::TELFOCUS, this->focus ); + Common::extract_telemetry_value( jmessage, Key::Tcsd::RAOFFSET, this->offsetra ); + Common::extract_telemetry_value( jmessage, Key::Tcsd::DECLOFFS, this->offsetdec ); + Common::extract_telemetry_value( jmessage, Key::Tcsd::CASANGLE, this->cassangle ); + Common::extract_telemetry_value( jmessage, Key::Tcsd::PA, this->pa ); + Common::extract_telemetry_value( jmessage, Key::PUBTIME, this->pubtime ); + + for ( const auto &dev : CalibDefs::domelamps() ) { + Common::extract_telemetry_value( jmessage, dev.jkey, this->domelamp[dev.name] ); + } + } + /***** Sequencer::TcsInfo::store ****************************************/ + + /***** Sequencer::TcsInfo::is_fresh *************************************/ + /** + * @brief is the cached telemetry recent enough to act on? + * @details tcsd publishes a snapshot every second while its connection + * to the TCS is open, so an age check is meaningful here and + * also covers the not-connected case: publishing stops and the + * cache simply ages out. + * @return true|false + * + */ + bool is_fresh() const { + if ( this->pubtime == 0 ) return false; // nothing received yet + return ( get_time_us() - this->pubtime ) <= TCS_STATUS_MAX_AGE_US; + } + /***** Sequencer::TcsInfo::is_fresh *************************************/ + + /** + * @brief cached state of the named dome lamp + * @param[in] name Lamp::Dome name + * @return 1=on 0=off -1=undefined or unknown lamp + */ + int domelamp_state( const std::string &name ) const { + auto it = this->domelamp.find(name); + return ( it == this->domelamp.end() ? -1 : it->second ); + } + + private: + /** @brief how old tcsd telemetry may be and still be acted on */ + static constexpr int64_t TCS_STATUS_MAX_AGE_US = 5'000'000; + }; + /***** Sequencer::TcsInfo ***************************************************/ + + + /***** Sequencer::CalibInfo *************************************************/ + /** + * @class CalibInfo + * @brief holds the most recently published calibd telemetry + * + * Written by the subscriber thread and read by the sequence threads; all + * access is serialized by the caller under Sequence::calibd_mtx. + * + */ + class CalibInfo { + public: + CalibInfo() { + // the modulator names and their message keys are defined in calib_defs.h + // + for ( const auto &dev : CalibDefs::modulators() ) { + this->lampmod[dev.name] = -1; + } + this->init(); + } + + int caldoor; ///< 1=open 0=closed -1=undefined + int calcover; ///< 1=open 0=closed -1=undefined + + std::map lampmod; ///< 1=on 0=off -1=undefined, indexed by Lamp::Mod name + + /** + * @brief initialize all class member variables to "non-values" + */ + void init() { + caldoor=-1; + calcover=-1; + for ( auto &[name,state] : lampmod ) { state=-1; } + } + + /***** Sequencer::CalibInfo::store **************************************/ + /** + * @brief store a received calibd telemetry message + * @details calibd publishes everything as a string: the actuators as a + * position name, and the modulators as " ". + * Both are reduced to a tri-state here so that comparing + * against a desired state is uniform. + * @param[in] jmessage subscribed-received JSON message + * + */ + void store( const nlohmann::json &jmessage ) { + this->init(); + + std::string value; + + value.clear(); + Common::extract_telemetry_value( jmessage, Key::Calibd::CALDOOR, value ); + this->caldoor = open_state( value ); + + value.clear(); + Common::extract_telemetry_value( jmessage, Key::Calibd::CALCOVER, value ); + this->calcover = open_state( value ); + + for ( const auto &dev : CalibDefs::modulators() ) { + value.clear(); + Common::extract_telemetry_value( jmessage, dev.jkey, value ); + this->lampmod[dev.name] = power_state( value ); + } + + this->received=true; + } + /***** Sequencer::CalibInfo::store **************************************/ + + /***** Sequencer::CalibInfo::is_valid ***********************************/ + /** + * @brief is there cached telemetry to act on? + * @details Deliberately not an age check, unlike TcsInfo::is_fresh(). + * calibd publishes only when its state changes, plus a forced + * publish at startup and on request, so telemetry that is + * minutes or hours old is still current and an age check would + * reject all of it. Anything that could invalidate the cache + * without calibd publishing has to clear it explicitly. + * @return true|false + * + */ + bool is_valid() const { return this->received; } + /***** Sequencer::CalibInfo::is_valid ***********************************/ + + /** @brief discard the cache, e.g. when calibd is known to have restarted */ + void invalidate() { this->received=false; this->init(); } + + /** + * @brief cached state of the named modulator + * @param[in] name Lamp::Mod name + * @return 1=on 0=off -1=undefined or unknown modulator + */ + int lampmod_state( const std::string &name ) const { + auto it = this->lampmod.find(name); + return ( it == this->lampmod.end() ? -1 : it->second ); + } + + private: + bool received=false; ///< has any telemetry been received? + + /** @brief actuator position name as 1=open, 0=closed, -1=anything else */ + static int open_state( const std::string &value ) { + if ( value == "open" ) return 1; + if ( value == "close" ) return 0; + return -1; + } + + /** @brief first token of calibd's " " as 1=on, 0=off, -1=other */ + static int power_state( const std::string &value ) { + std::vector tokens; + Tokenize( value, tokens, " " ); + if ( tokens.empty() ) return -1; + if ( tokens.at(0) == "on" ) return 1; + if ( tokens.at(0) == "off" ) return 0; + return -1; + } + }; + /***** Sequencer::CalibInfo *************************************************/ + + + /***** Sequencer::PowerInfo *************************************************/ + /** + * @class PowerInfo + * @brief holds the most recently published powerd telemetry + * + * Only the plugs the sequencer makes decisions about are kept. Written by the + * subscriber thread and read by the sequence threads; all access is + * serialized by the caller under Sequence::powerd_mtx. + * + */ + class PowerInfo { + public: + PowerInfo() { + // the lamp plug names and their message keys are defined in calib_defs.h + // + for ( const auto &dev : CalibDefs::callamps() ) { + this->plug[dev.name] = -1; + } + } + + std::map plug; ///< 1=on 0=off -1=undefined, indexed by plug name + + /** + * @brief initialize all class member variables to "non-values" + */ + void init() { + for ( auto &[name,state] : plug ) { state=-1; } + } + + /***** Sequencer::PowerInfo::store **************************************/ + /** + * @brief store a received powerd telemetry message + * @details powerd publishes a plug as true when it is on. That is + * reduced to a tri-state here so that a plug missing from the + * message is distinguishable from one that is off. + * @param[in] jmessage subscribed-received JSON message + * + */ + void store( const nlohmann::json &jmessage ) { + this->init(); + + for ( const auto &dev : CalibDefs::callamps() ) { + // Leave the plug undefined unless powerd really did report a bool for + // it. Defaulting a missing or malformed value to off would claim + // knowledge I don't have, and a caller could skip turning a lamp on. + // + if ( !jmessage.contains(dev.jkey) || + !jmessage[dev.jkey].is_boolean() ) continue; + bool ison=false; + Common::extract_telemetry_value( jmessage, dev.jkey, ison ); + this->plug[dev.name] = ( ison ? 1 : 0 ); + } + + this->received=true; + } + /***** Sequencer::PowerInfo::store **************************************/ + + /** + * @brief is there cached telemetry to act on? + * @details Not an age check, for the same reason as + * CalibInfo::is_valid() -- powerd publishes only on change. + * @return true|false + */ + bool is_valid() const { return this->received; } + + /** @brief discard the cache, e.g. when powerd is known to have restarted */ + void invalidate() { this->received=false; this->init(); } + + /** + * @brief cached state of the named plug + * @param[in] name plug name + * @return 1=on 0=off -1=undefined or unknown plug + */ + int plug_state( const std::string &name ) const { + auto it = this->plug.find(name); + return ( it == this->plug.end() ? -1 : it->second ); + } + + private: + bool received=false; ///< has any telemetry been received? + }; + /***** Sequencer::PowerInfo *************************************************/ + + /***** Sequencer::TargetInfo ************************************************/ /** * @class TargetInfo diff --git a/sequencerd/sequencerd.cpp b/sequencerd/sequencerd.cpp index 4b9269fc..428c7c24 100644 --- a/sequencerd/sequencerd.cpp +++ b/sequencerd/sequencerd.cpp @@ -133,7 +133,9 @@ int main(int argc, char **argv) { Topic::ACAMD, Topic::SLICECAMD, Topic::SLITD, - Topic::TCSD } ) == ERROR ) { + Topic::TCSD, + Topic::CALIBD, + Topic::POWERD } ) == ERROR ) { logwrite(function, "ERROR initializing publisher-subscriber handler"); sequencerd.exit_cleanly(); } diff --git a/tcsd/tcs_interface.cpp b/tcsd/tcs_interface.cpp index a58a0441..51c756da 100644 --- a/tcsd/tcs_interface.cpp +++ b/tcsd/tcs_interface.cpp @@ -40,6 +40,12 @@ namespace TCS { nlohmann::json jmessage_out; jmessage_out[Key::SOURCE] = Daemon::TCSD; + // Timestamp every snapshot so subscribers can tell how old this state is. + // Publishing stops when the connection to the TCS is closed, so the age is + // also how a subscriber learns that the TCS is no longer being read. + // + jmessage_out[Key::PUBTIME] = get_time_us(); + std::string motion; { std::lock_guard lock(tcs_info_mtx); @@ -1523,7 +1529,9 @@ namespace TCS { retstring="ERR"; return ERROR; } - std::this_thread::sleep_for(std::chrono::milliseconds(250)); // wait for it to turn on|off + // the mechanism takes time to respond and the TCS only checks for + // this command once per second. + std::this_thread::sleep_for(std::chrono::milliseconds(1100)); } // whether or not requesting on|off, check state now diff --git a/tcsd/tcs_interface.h b/tcsd/tcs_interface.h index 3f2397d3..d41a87ce 100644 --- a/tcsd/tcs_interface.h +++ b/tcsd/tcs_interface.h @@ -14,6 +14,7 @@ #include "tcs_constants.h" #include "tcsd_commands.h" #include "message_keys.h" +#include "calib_defs.h" #include #include #include @@ -89,13 +90,13 @@ namespace TCS { TcsInfo() : isopen(false) { - // these map indices are the lamp names accepted by the UI - // this is the only place they appear + // The map indices are the lamp names accepted by the UI. Those names, + // the message keys, and the TCS lamp numbers all come from the shared + // table in calib_defs.h, which is where they are defined. // - lampinfo["LO"] = { Key::Tcsd::LAMP_LO, 1, -1 }; - lampinfo["HI"] = { Key::Tcsd::LAMP_HI, 2, -1 }; - lampinfo["ARC"] = { Key::Tcsd::LAMP_ARC, 3, -1 }; - lampinfo["ULTRA"] = { Key::Tcsd::LAMP_ULTRA, 4, -1 }; + for ( const auto &dev : CalibDefs::domelamps() ) { + lampinfo[dev.name] = { dev.jkey, static_cast(dev.num), -1 }; + } this->init(); } From 7859680d24cd8a06ca3ceb0ce9e4e553eb1af3dd Mon Sep 17 00:00:00 2001 From: David Hale Date: Wed, 29 Jul 2026 18:08:16 -0700 Subject: [PATCH 2/6] tcsd: don't read the TCS on a snapshot request when closed and request_snapshot can work for a single daemon --- common/calib_defs.h | 113 +++++------------------------ sequencerd/sequence.cpp | 44 ++++++----- sequencerd/sequence.h | 2 +- sequencerd/sequencer_interface.cpp | 10 +-- sequencerd/sequencer_interface.h | 92 +++++++++++------------ tcsd/tcs_interface.cpp | 24 ++++-- 6 files changed, 107 insertions(+), 178 deletions(-) diff --git a/common/calib_defs.h b/common/calib_defs.h index 91337de7..4b8bf9a6 100644 --- a/common/calib_defs.h +++ b/common/calib_defs.h @@ -1,32 +1,16 @@ /** * @file calib_defs.h - * @brief names of the calibration devices, and their bindings to message keys + * @brief calibration device names bound to their message keys * @author David Hale * - * Three distinct kinds of information are involved in naming a calibration - * device, and they are easy to conflate: - * - * the device name the vocabulary used by the cfg files, by the commands - * sent to the owning daemon, and as the map key in every - * daemon that handles the device - * the message key what the providing daemon publishes it under - * the hardware id how the owning daemon addresses the hardware, e.g. the - * TCS lamp number or the modulator's Arduino channel - * - * The names live in the Lamp namespaces below. The message keys live in - * message_keys.h, where their values are arbitrary identifiers. Several of them - * happen to hold the same string as the device name today, but that is not - * something to rely on, so the Device tables bind the two explicitly rather than - * leaving the binding implicit in a constant's value. + * The names are the vocabulary used by the cfg files, by the commands sent to + * the owning daemon, and as map keys. Some of them hold the same string as + * their message key, which is not something to rely on, so the Device tables + * bind the two rather than leaving it implicit. * * The hardware id is wiring, not vocabulary, so it stays in the owning daemon's - * config file -- LAMPMOD_MOD in calibd.cfg.in remains the place that says which - * Arduino channel a modulator is on. It appears in the tables only because a - * consumer which knows a device by name sometimes has to command it by number. - * - * Everything that names one of these devices derives it from this file: - * TCS::TcsInfo, Sequencer::CalibrationTarget, and the sequencer's telemetry - * caches. + * config (LAMPMOD_MOD in calibd.cfg.in). It appears here only because a + * consumer that knows a device by name has to command it by number. * */ #pragma once @@ -36,15 +20,9 @@ #include "message_keys.h" -/***** Lamp *******************************************************************/ -/** - * @namespace Lamp - * @brief names of the calibration lamps and their modulators - * - */ namespace Lamp { - /** @brief internal calibration lamps. These are powerd plug names. */ + ///< internal calibration lamps, which are powerd plug names namespace Cal { inline const std::string THAR = "LAMPTHAR"; inline const std::string FEAR = "LAMPFEAR"; @@ -52,7 +30,7 @@ namespace Lamp { inline const std::string REDC = "LAMPREDC"; } - /** @brief TCS dome lamps. These are the names tcsd accepts for TCSD_LAMP. */ + ///< TCS dome lamps, as accepted by tcsd namespace Dome { inline const std::string LO = "LO"; inline const std::string HI = "HI"; @@ -60,7 +38,7 @@ namespace Lamp { inline const std::string ULTRA = "ULTRA"; } - /** @brief lamp modulators. These are the names configured in calibd. */ + ///< lamp modulators, as named in calibd namespace Mod { inline const std::string FEAR = "MODFEAR"; inline const std::string BLETA = "MODBLETA"; @@ -71,40 +49,20 @@ namespace Lamp { } } -/***** Lamp *******************************************************************/ - -/***** CalibDefs **************************************************************/ -/** - * @namespace CalibDefs - * @brief binds calibration device names to message keys and hardware ids - * - */ namespace CalibDefs { - /***** CalibDefs::Device ****************************************************/ /** * @struct Device - * @brief binds a device name to the key its provider publishes and to the - * hardware identifier used to command it - * + * @brief binds a device name to its message key and hardware id */ struct Device { - std::string name; ///< device name used by cfg files, commands and map keys + std::string name; ///< name used by cfg files, commands and map keys std::string jkey; ///< key published by the providing daemon - int num; ///< hardware id (TCS lamp number, modulator channel), 0 if unused + int num; ///< TCS lamp number or modulator channel, 0 if unused }; - /***** CalibDefs::Device ****************************************************/ - - /***** CalibDefs::domelamps *************************************************/ - /** - * @brief TCS dome lamps - * @details Listed in TCS lamp-number order, which is also the order of the - * dome lamp tokens in the sequencer's CAL_TARGET config lines. - * @return reference to the vector of dome lamp Devices - * - */ + ///< dome lamps in TCS lamp-number order, which is the CAL_TARGET token order inline const std::vector& domelamps() { static const std::vector devices = { { Lamp::Dome::LO, Key::Tcsd::LAMP_LO, 1 }, @@ -114,19 +72,9 @@ namespace CalibDefs { }; return devices; } - /***** CalibDefs::domelamps *************************************************/ - - /***** CalibDefs::callamps **************************************************/ - /** - * @brief internal calibration lamps, which are powerd plugs - * @details Listed in the order of the lamp tokens in the sequencer's - * CAL_TARGET config lines. There is no hardware id here because - * powerd resolves the plug name to a unit and outlet from its own - * NPS_PLUG config. - * @return reference to the vector of calibration lamp Devices - * - */ + ///< calibration lamps in CAL_TARGET token order. powerd resolves the plug + ///< name to a unit and outlet itself, so there is no hardware id here. inline const std::vector& callamps() { static const std::vector devices = { { Lamp::Cal::THAR, Key::Powerd::LAMPTHAR, 0 }, @@ -136,24 +84,9 @@ namespace CalibDefs { }; return devices; } - /***** CalibDefs::callamps **************************************************/ - - /***** CalibDefs::modulators ************************************************/ - /** - * @brief lamp modulators - * @details Listed in calibd channel order, which is also the order of the - * modulator tokens in the sequencer's CAL_TARGET config lines. - * This must agree with the LAMPMOD_MOD lines in calibd.cfg.in, - * which is where the channel assignment actually lives. calibd - * publishes each modulator's state under its name, so a consumer - * joins to that telemetry on the name and needs the channel only - * to build a CALIBD_LAMPMOD command. - * Channels 7 and 8 are configured as placeholders and are not - * listed until they are assigned to real modulators. - * @return reference to the vector of modulator Devices - * - */ + ///< modulators in channel order, which is the CAL_TARGET token order. Must + ///< agree with LAMPMOD_MOD in calibd.cfg.in. Channels 7,8 are placeholders. inline const std::vector& modulators() { static const std::vector devices = { { Lamp::Mod::FEAR, Key::Calibd::MODFEAR, 1 }, @@ -165,16 +98,12 @@ namespace CalibDefs { }; return devices; } - /***** CalibDefs::modulators ************************************************/ - - /***** CalibDefs::find ******************************************************/ /** * @brief look up a Device by name - * @param[in] devices one of the vectors defined above + * @param[in] devices one of the vectors above * @param[in] name device name to find - * @return pointer to the Device, or nullptr if name is not defined - * + * @return pointer to the Device, or nullptr if not defined */ inline const Device* find( const std::vector &devices, const std::string &name ) { @@ -183,7 +112,5 @@ namespace CalibDefs { } return nullptr; } - /***** CalibDefs::find ******************************************************/ } -/***** CalibDefs **************************************************************/ diff --git a/sequencerd/sequence.cpp b/sequencerd/sequence.cpp index da4e4d9a..fc1a2e3d 100644 --- a/sequencerd/sequence.cpp +++ b/sequencerd/sequence.cpp @@ -215,17 +215,25 @@ namespace Sequencer { * re-publishing its own state, ensuring the sequencer receives * current telemetry even if the daemon published before the * sequencer subscribed. + * Naming a single daemon asks only that one, which avoids making + * the others refresh their state, some of which costs hardware I/O. + * @param[in] daemon optional single daemon to ask, default all of them * */ - void Sequence::request_snapshot() { + void Sequence::request_snapshot( const std::string &daemon ) { nlohmann::json jmessage; - jmessage[Daemon::CAMERAD] = true; - jmessage[Daemon::ACAMD] = true; - jmessage[Daemon::SLICECAMD] = true; - jmessage[Daemon::SLITD] = true; - jmessage[Daemon::TCSD] = true; - jmessage[Daemon::CALIBD] = true; - jmessage[Daemon::POWERD] = true; + if ( !daemon.empty() ) { + jmessage[daemon] = true; + } + else { + jmessage[Daemon::CAMERAD] = true; + jmessage[Daemon::ACAMD] = true; + jmessage[Daemon::SLICECAMD] = true; + jmessage[Daemon::SLITD] = true; + jmessage[Daemon::TCSD] = true; + jmessage[Daemon::CALIBD] = true; + jmessage[Daemon::POWERD] = true; + } this->publisher->publish( jmessage, Topic::SNAPSHOT ); } /***** Sequencer::Sequence::request_snapshot *******************************/ @@ -1122,9 +1130,8 @@ namespace Sequencer { this->daemon_manager.clear( Sequencer::DAEMON_POWER ); // powerd not ready - // Discard the cached power state. powerd publishes only when its state - // changes, so nothing else would tell me that what I have is no longer - // current. request_snapshot() below refills it. + // Discard the cached power state. powerd publishes only on change, so + // nothing else would tell me it's stale. request_snapshot() below refills it. { std::lock_guard lock(this->powerd_mtx); this->powerinfo.invalidate(); @@ -1136,7 +1143,7 @@ namespace Sequencer { } this->daemon_manager.set( Sequencer::DAEMON_POWER ); // powerd ready - this->request_snapshot(); + this->request_snapshot( Daemon::POWERD ); return NO_ERROR; } /***** Sequencer::Sequence::power_init **************************************/ @@ -1546,9 +1553,8 @@ namespace Sequencer { this->daemon_manager.clear( Sequencer::DAEMON_CALIB ); - // Discard the cached calib state. calibd publishes only when its state - // changes, so nothing else would tell me that what I have is no longer - // current. request_snapshot() below refills it. + // Discard the cached calib state. calibd publishes only on change, so + // nothing else would tell me it's stale. request_snapshot() below refills it. { std::lock_guard lock(this->calibd_mtx); this->calibinfo.invalidate(); @@ -1608,7 +1614,7 @@ namespace Sequencer { // calibd is ready this->daemon_manager.set( Sequencer::DAEMON_CALIB ); - this->request_snapshot(); + this->request_snapshot( Daemon::CALIBD ); this->thread_error_manager.clear( THR_CALIB_INIT ); @@ -2471,7 +2477,7 @@ namespace Sequencer { this->broadcast.notice( function, "configuring calibrator for "+calname); // set the calib door and cover if needed. CALIBD_SET moves both in one - // command, so this can only be skipped when both are already set. + // command so both must already be set to skip it. // bool doorcover_isset = false; { @@ -2556,8 +2562,8 @@ namespace Sequencer { continue; } } - // otherwise send the command. calibd publishes modulators by name but - // takes the channel number, which comes from the shared table. + // otherwise send the command. calibd publishes by name but takes the + // channel number. const auto *dev = CalibDefs::find( CalibDefs::modulators(), mod ); if ( dev == nullptr ) { this->broadcast.error( function, "unknown lamp modulator "+mod ); diff --git a/sequencerd/sequence.h b/sequencerd/sequence.h index 6b9dd986..a64601f2 100644 --- a/sequencerd/sequence.h +++ b/sequencerd/sequence.h @@ -507,7 +507,7 @@ namespace Sequencer { void handletopic_calibd( const nlohmann::json &jmessage ); void handletopic_powerd( const nlohmann::json &jmessage ); void publish_snapshot(); - void request_snapshot(); + void request_snapshot( const std::string &daemon="" ); void publish_seqstate(); void publish_waitstate(); void publish_daemonstate(); diff --git a/sequencerd/sequencer_interface.cpp b/sequencerd/sequencer_interface.cpp index f04e2da2..36c07823 100644 --- a/sequencerd/sequencer_interface.cpp +++ b/sequencerd/sequencer_interface.cpp @@ -991,9 +991,8 @@ namespace Sequencer { info.channel_active[chans.at(i)] = on_off(tokens.at(3+i)); } - // tokens 7-10 are the internal calibration lamp states, and tokens 11-14 - // the dome lamp states, each in the order the shared tables list them - // + // tokens 7-10 are the cal lamps and 11-14 the dome lamps, each in the + // order the tables in calib_defs.h list them const auto &callamps = CalibDefs::callamps(); const auto &domelamps = CalibDefs::domelamps(); for (size_t i=0; i < 4; i++) { @@ -1001,9 +1000,8 @@ namespace Sequencer { info.domelamp[domelamps.at(i).name] = on_off(tokens.at(11+i)); } - // tokens 15-20 are the modulator states, in modulator channel order. - // Index by name, which is how calibd publishes them. - // + // tokens 15-20 are the modulators in channel order, indexed by name + // because that's how calibd publishes them const auto &modulators = CalibDefs::modulators(); for (size_t i=0; i<6; i++) { info.lampmod[modulators.at(i).name] = on_off(tokens.at(15+i)); diff --git a/sequencerd/sequencer_interface.h b/sequencerd/sequencer_interface.h index 26e233c9..9a933798 100644 --- a/sequencerd/sequencer_interface.h +++ b/sequencerd/sequencer_interface.h @@ -91,9 +91,8 @@ namespace Sequencer { inline const std::string I = "I"; } - // The calibration lamp, dome lamp and modulator names are shared with tcsd, - // calibd and powerd, so they live in common/calib_defs.h as Lamp::Cal, - // Lamp::Dome and Lamp::Mod. + // The lamp and modulator names are shared with tcsd, calibd and powerd, so + // they are in common/calib_defs.h as Lamp::Cal, Lamp::Dome and Lamp::Mod. /***** Sequencer::PowerSwitch ***********************************************/ @@ -185,18 +184,15 @@ namespace Sequencer { * @class TcsInfo * @brief holds the most recently published tcsd telemetry * - * There is one member here for each key published by - * TCS::Interface::publish_snapshot. Written by the subscriber thread and read - * by the sequence threads; all access is serialized by the caller under - * Sequence::tcsd_mtx. + * One member for each key published by TCS::Interface::publish_snapshot. + * Written by the subscriber thread, read by the sequence threads; the caller + * serializes all access under Sequence::tcsd_mtx. * */ class TcsInfo { public: TcsInfo() { - // the lamp names and their message keys are defined in calib_defs.h - // - for ( const auto &dev : CalibDefs::domelamps() ) { + for ( const auto &dev : CalibDefs::domelamps() ) { // names from calib_defs.h this->domelamp[dev.name] = -1; } this->init(); @@ -257,12 +253,11 @@ namespace Sequencer { /***** Sequencer::TcsInfo::store ****************************************/ /** * @brief store a received tcsd telemetry message - * @details Everything is erased first because a snapshot is all or - * nothing; a key missing from this message must not leave the - * value from an earlier one behind. extract_telemetry_value - * leaves its out-param alone on a missing key, a type - * mismatch, or a null (which is how a NAN arrives), so the - * non-values set by init() survive those cases. + * @details Erase first, because a snapshot is all or nothing; a key + * missing from this message must not leave the value from an + * earlier one behind. extract_telemetry_value leaves its + * out-param alone on a missing key, wrong type, or null (how + * a NAN arrives), so the init() non-values survive those. * @param[in] jmessage subscribed-received JSON message * */ @@ -298,15 +293,18 @@ namespace Sequencer { /***** Sequencer::TcsInfo::is_fresh *************************************/ /** - * @brief is the cached telemetry recent enough to act on? - * @details tcsd publishes a snapshot every second while its connection - * to the TCS is open, so an age check is meaningful here and - * also covers the not-connected case: publishing stops and the - * cache simply ages out. + * @brief was this read from the TCS recently enough to act on? + * @details Both halves are needed. tcsd publishes every second while + * connected, so the age means something, but it also answers a + * snapshot request while closed and timestamps that message + * like any other. ISOPEN separates the two. + * A poll can still fail while connected, so a caller must + * treat a non-value as unknown no matter what this returns. * @return true|false * */ bool is_fresh() const { + if ( !this->isopen ) return false; // nothing was read from the TCS if ( this->pubtime == 0 ) return false; // nothing received yet return ( get_time_us() - this->pubtime ) <= TCS_STATUS_MAX_AGE_US; } @@ -334,16 +332,14 @@ namespace Sequencer { * @class CalibInfo * @brief holds the most recently published calibd telemetry * - * Written by the subscriber thread and read by the sequence threads; all - * access is serialized by the caller under Sequence::calibd_mtx. + * Written by the subscriber thread, read by the sequence threads; the caller + * serializes all access under Sequence::calibd_mtx. * */ class CalibInfo { public: CalibInfo() { - // the modulator names and their message keys are defined in calib_defs.h - // - for ( const auto &dev : CalibDefs::modulators() ) { + for ( const auto &dev : CalibDefs::modulators() ) { // names from calib_defs.h this->lampmod[dev.name] = -1; } this->init(); @@ -366,10 +362,10 @@ namespace Sequencer { /***** Sequencer::CalibInfo::store **************************************/ /** * @brief store a received calibd telemetry message - * @details calibd publishes everything as a string: the actuators as a - * position name, and the modulators as " ". - * Both are reduced to a tri-state here so that comparing - * against a desired state is uniform. + * @details calibd publishes strings: the actuators as a position name, + * the modulators as " ". Both reduce to a + * tri-state here so comparing against a desired state is + * uniform. * @param[in] jmessage subscribed-received JSON message * */ @@ -399,12 +395,11 @@ namespace Sequencer { /***** Sequencer::CalibInfo::is_valid ***********************************/ /** * @brief is there cached telemetry to act on? - * @details Deliberately not an age check, unlike TcsInfo::is_fresh(). - * calibd publishes only when its state changes, plus a forced - * publish at startup and on request, so telemetry that is - * minutes or hours old is still current and an age check would - * reject all of it. Anything that could invalidate the cache - * without calibd publishing has to clear it explicitly. + * @details Not an age check, unlike TcsInfo::is_fresh(). calibd + * publishes only on change, plus a forced publish at startup + * and on request, so telemetry hours old is still current and + * an age check would reject all of it. Anything that can + * invalidate the cache without calibd publishing must clear it. * @return true|false * */ @@ -452,17 +447,15 @@ namespace Sequencer { * @class PowerInfo * @brief holds the most recently published powerd telemetry * - * Only the plugs the sequencer makes decisions about are kept. Written by the - * subscriber thread and read by the sequence threads; all access is - * serialized by the caller under Sequence::powerd_mtx. + * Keeps only the plugs the sequencer makes decisions about. Written by the + * subscriber thread, read by the sequence threads; the caller serializes all + * access under Sequence::powerd_mtx. * */ class PowerInfo { public: PowerInfo() { - // the lamp plug names and their message keys are defined in calib_defs.h - // - for ( const auto &dev : CalibDefs::callamps() ) { + for ( const auto &dev : CalibDefs::callamps() ) { // names from calib_defs.h this->plug[dev.name] = -1; } } @@ -479,9 +472,9 @@ namespace Sequencer { /***** Sequencer::PowerInfo::store **************************************/ /** * @brief store a received powerd telemetry message - * @details powerd publishes a plug as true when it is on. That is - * reduced to a tri-state here so that a plug missing from the - * message is distinguishable from one that is off. + * @details powerd publishes a plug as true when it is on. That reduces + * to a tri-state here so a plug missing from the message is + * distinguishable from one that is off. * @param[in] jmessage subscribed-received JSON message * */ @@ -489,10 +482,8 @@ namespace Sequencer { this->init(); for ( const auto &dev : CalibDefs::callamps() ) { - // Leave the plug undefined unless powerd really did report a bool for - // it. Defaulting a missing or malformed value to off would claim - // knowledge I don't have, and a caller could skip turning a lamp on. - // + // leave undefined unless powerd really did report a bool. Defaulting + // to off would let a caller skip turning a lamp on. if ( !jmessage.contains(dev.jkey) || !jmessage[dev.jkey].is_boolean() ) continue; bool ison=false; @@ -506,8 +497,7 @@ namespace Sequencer { /** * @brief is there cached telemetry to act on? - * @details Not an age check, for the same reason as - * CalibInfo::is_valid() -- powerd publishes only on change. + * @details Not an age check, same reason as CalibInfo::is_valid() * @return true|false */ bool is_valid() const { return this->received; } diff --git a/tcsd/tcs_interface.cpp b/tcsd/tcs_interface.cpp index 51c756da..7731466c 100644 --- a/tcsd/tcs_interface.cpp +++ b/tcsd/tcs_interface.cpp @@ -33,18 +33,26 @@ namespace TCS { this->publish_snapshot(dontcare); } void Interface::publish_snapshot(std::string &retstring) { - // fill the tcs_info class with current info + // Only read the TCS when there is a connection to read it with, + // and when closed, erase the class. // - this->get_tcs_info(); + bool isopen = false; + { + std::lock_guard lock(tcs_info_mtx); + isopen = this->tcs_info.isopen; + } + + if ( isopen ) { + this->get_tcs_info(); // fill the tcs_info class with current info + } + else { + std::lock_guard lock(tcs_info_mtx); + this->tcs_info.init(); + } nlohmann::json jmessage_out; jmessage_out[Key::SOURCE] = Daemon::TCSD; - - // Timestamp every snapshot so subscribers can tell how old this state is. - // Publishing stops when the connection to the TCS is closed, so the age is - // also how a subscriber learns that the TCS is no longer being read. - // - jmessage_out[Key::PUBTIME] = get_time_us(); + jmessage_out[Key::PUBTIME] = get_time_us(); // so subscribers can age this std::string motion; { From 063942c8c3a6cd4ad98cdeb64cc6ece16df58698 Mon Sep 17 00:00:00 2001 From: David Hale Date: Wed, 29 Jul 2026 18:29:33 -0700 Subject: [PATCH 3/6] keys have no value rather than being omitted --- sequencerd/sequence.cpp | 20 +++++++++++++++++--- slicecamd/slicecam_interface.cpp | 8 ++++++++ 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/sequencerd/sequence.cpp b/sequencerd/sequence.cpp index fc1a2e3d..8ebb9796 100644 --- a/sequencerd/sequence.cpp +++ b/sequencerd/sequence.cpp @@ -3855,8 +3855,8 @@ namespace Sequencer { * @brief publish target info on Topic::TARGETINFO, on change (or force) * @details Builds a JSON message of the current target and publishes it * only when it differs from the last published message, unless - * force is set. The message is empty unless seq state is - * READY or RUNNING. + * force is set. The values are null unless seq state is READY or + * RUNNING; the keys are always present. * @param[in] force optional (default=false) publish irrespective of change * */ @@ -3864,7 +3864,7 @@ namespace Sequencer { nlohmann::json jmessage; jmessage[Key::SOURCE] = Sequencer::DAEMON_NAME; - // fill telemetry only when READY or RUNNING; otherwise an empty (no-target) message + // fill telemetry only when READY or RUNNING // if ( this->seq_state_manager.are_any_set( Sequencer::SEQ_READY, Sequencer::SEQ_RUNNING ) ) { // unconfigured values are stored as NAN @@ -3878,6 +3878,20 @@ namespace Sequencer { jmessage[Key::TargetInfo::RA] = this->target.ra_hms; jmessage[Key::TargetInfo::DECL] = this->target.dec_dms; } + else { + // Send the keys with no value rather than omitting them. Having no target + // is not the same as having forgotten to send a key, and a subscriber can + // only tell the difference if I say which one this is. + // + jmessage[Key::TargetInfo::OBS_ID] = nullptr; + jmessage[Key::TargetInfo::NAME] = nullptr; + jmessage[Key::TargetInfo::SLITA] = nullptr; + jmessage[Key::TargetInfo::BINSPECT] = nullptr; + jmessage[Key::TargetInfo::BINSPAT] = nullptr; + jmessage[Key::TargetInfo::POINTMODE] = nullptr; + jmessage[Key::TargetInfo::RA] = nullptr; + jmessage[Key::TargetInfo::DECL] = nullptr; + } // unless forced, only publish if the target info changed // diff --git a/slicecamd/slicecam_interface.cpp b/slicecamd/slicecam_interface.cpp index 0db1a62c..ec21083e 100644 --- a/slicecamd/slicecam_interface.cpp +++ b/slicecamd/slicecam_interface.cpp @@ -1013,6 +1013,14 @@ namespace Slicecam { Common::extract_telemetry_value( jmessage, Key::TargetInfo::RA, ra_hms ); Common::extract_telemetry_value( jmessage, Key::TargetInfo::DECL, dec_dms ); + // no target is normal, so don't get here by way of an exception + // + if ( ra_hms.empty() || dec_dms.empty() ) { + this->targetinfo_ra_deg.store(NAN); + this->targetinfo_dec_deg.store(NAN); + return; + } + try { this->targetinfo_ra_deg.store( radec_to_decimal( ra_hms ) * TO_DEGREES ); this->targetinfo_dec_deg.store( radec_to_decimal( dec_dms ) ); From 47bf5daf9c9eda5e602e4ad1748bd4fe94f13991 Mon Sep 17 00:00:00 2001 From: David Hale Date: Wed, 29 Jul 2026 19:24:47 -0700 Subject: [PATCH 4/6] increase tcs lamp power-on delay empirically --- tcsd/tcs_interface.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tcsd/tcs_interface.cpp b/tcsd/tcs_interface.cpp index 7731466c..451de246 100644 --- a/tcsd/tcs_interface.cpp +++ b/tcsd/tcs_interface.cpp @@ -1537,9 +1537,6 @@ namespace TCS { retstring="ERR"; return ERROR; } - // the mechanism takes time to respond and the TCS only checks for - // this command once per second. - std::this_thread::sleep_for(std::chrono::milliseconds(1100)); } // whether or not requesting on|off, check state now @@ -1578,6 +1575,12 @@ namespace TCS { std::string reply; long error = this->send_command( cmd.str(), reply, TCS::FAST_RESPONSE ); + // the mechanism takes time to respond and the TCS only checks for + // this command once per second. + if (error==NO_ERROR) { + std::this_thread::sleep_for(std::chrono::milliseconds(1600)); + } + return error; } /***** TCS::Interface::set_lamp *********************************************/ From b3e75db397de553effd48bf1ab951843a53c5cd1 Mon Sep 17 00:00:00 2001 From: David Hale Date: Wed, 29 Jul 2026 19:37:08 -0700 Subject: [PATCH 5/6] calib_set log when set instead of when not set --- sequencerd/sequence.cpp | 12 +++++------- tcsd/tcs_interface.cpp | 2 +- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/sequencerd/sequence.cpp b/sequencerd/sequence.cpp index 8ebb9796..00e1b2b6 100644 --- a/sequencerd/sequence.cpp +++ b/sequencerd/sequence.cpp @@ -2488,10 +2488,7 @@ namespace Sequencer { } std::stringstream cmd; - if ( doorcover_isset ) { - logwrite( function, "calib door and cover already set" ); - } - else { + if ( !doorcover_isset ) { cmd.str(""); cmd << CALIBD_SET << " door=" << ( calinfo.caldoor ? "open" : "close" ) << " cover=" << ( calinfo.calcover ? "open" : "close" ); @@ -2502,6 +2499,7 @@ namespace Sequencer { this->broadcast.error( function, "moving calib door and/or cover" ); throw std::runtime_error("moving calib door and/or cover"); } + logwrite( function, "calib door and cover set" ); } // set the internal calibration lamps if needed @@ -2513,7 +2511,6 @@ namespace Sequencer { std::lock_guard lock(this->powerd_mtx); if (this->powerinfo.is_valid() && this->powerinfo.plug_state(lamp) == (state?1:0)) { - logwrite(function, "cal lamp "+lamp+" already "+(state?"on":"off")); continue; } } @@ -2526,6 +2523,7 @@ namespace Sequencer { this->broadcast.error( function, message.str() ); throw std::runtime_error("setting lamp "+message.str()); } + logwrite(function, "cal lamp "+lamp+" "+(state?"on":"off")); } // set the dome lamps if needed @@ -2537,7 +2535,6 @@ namespace Sequencer { std::lock_guard lock(this->tcsd_mtx); if (this->tcsinfo.is_fresh() && this->tcsinfo.domelamp_state(lampname) == (state?1:0)) { - logwrite(function, "dome lamp "+lampname+" already "+(state?"on":"off")); continue; } } @@ -2547,6 +2544,7 @@ namespace Sequencer { logwrite(function, "ERROR "+cmd.str()); throw std::runtime_error("setting TCS "+cmd.str()); } + logwrite(function, "dome lamp "+lampname+" "+(state?"on":"off")); } // set the lamp modulators if needed @@ -2558,7 +2556,6 @@ namespace Sequencer { std::lock_guard lock(this->calibd_mtx); if (this->calibinfo.is_valid() && this->calibinfo.lampmod_state(mod) == (state?1:0)) { - logwrite(function, "lamp modulator "+mod+" already "+(state?"on":"off")); continue; } } @@ -2574,6 +2571,7 @@ namespace Sequencer { this->broadcast.error( function, cmd.str() ); throw std::runtime_error("setting lamp modulator "+cmd.str()); } + logwrite(function, "lamp modulator "+mod+" "+(state?"on":"off")); } if ( this->cancel_flag.load() ) { diff --git a/tcsd/tcs_interface.cpp b/tcsd/tcs_interface.cpp index 451de246..b35eb2a2b 100644 --- a/tcsd/tcs_interface.cpp +++ b/tcsd/tcs_interface.cpp @@ -1578,7 +1578,7 @@ namespace TCS { // the mechanism takes time to respond and the TCS only checks for // this command once per second. if (error==NO_ERROR) { - std::this_thread::sleep_for(std::chrono::milliseconds(1600)); + std::this_thread::sleep_for(std::chrono::milliseconds(2000)); } return error; From 3a7ee3b189f095af2e0d7379c0c1dbafa1fca8a7 Mon Sep 17 00:00:00 2001 From: David Hale Date: Thu, 30 Jul 2026 11:43:23 -0700 Subject: [PATCH 6/6] fixes snapshot requests that were not being published (needed a force) --- calibd/calib_interface.cpp | 11 ++++++++--- flexured/flexure_interface.cpp | 4 ++-- focusd/focus_interface.cpp | 10 +++++++++- tcsd/tcs_interface.cpp | 3 +-- 4 files changed, 20 insertions(+), 8 deletions(-) diff --git a/calibd/calib_interface.cpp b/calibd/calib_interface.cpp index 6a40a628..2e9eaf11 100644 --- a/calibd/calib_interface.cpp +++ b/calibd/calib_interface.cpp @@ -776,13 +776,18 @@ namespace Calib { /***** Calib::Interface::publish_status *************************************/ + /***** Calib::Interface::handletopic_snapshot *******************************/ + /** + * @brief If my topic is in the jmessage then force-publish my status + * @param[in] jmessage ref to incomming message + * + */ void Interface::handletopic_snapshot( const nlohmann::json &jmessage ) { - // If my topic is in the jmessage then publish my status - // if ( jmessage.contains( Topic::CALIBD ) ) { - this->publish_status(); + this->publish_status(true); } } + /***** Calib::Interface::handletopic_snapshot *******************************/ /***** Calib::Modulator::configure_host *************************************/ diff --git a/flexured/flexure_interface.cpp b/flexured/flexure_interface.cpp index 176feabb..e6a2df44 100644 --- a/flexured/flexure_interface.cpp +++ b/flexured/flexure_interface.cpp @@ -624,13 +624,13 @@ namespace Flexure { /***** Flexure::Interface::handletopic_snapshot ****************************/ /** - * @brief respond to a snapshot request by publishing my status + * @brief If my topic is in the jmessage then force-publish my status * @param[in] jmessage subscribed-received JSON message * */ void Interface::handletopic_snapshot( const nlohmann::json &jmessage ) { if ( jmessage.contains( Topic::FLEXURED ) ) { - this->publish_status(); + this->publish_status(true); } } /***** Flexure::Interface::handletopic_snapshot ****************************/ diff --git a/focusd/focus_interface.cpp b/focusd/focus_interface.cpp index 67c48e05..87abae67 100644 --- a/focusd/focus_interface.cpp +++ b/focusd/focus_interface.cpp @@ -597,11 +597,19 @@ namespace Focus { /***** Focus::Interface::publish_status ***********************************/ + /***** Focus::Interface::handletopic_snapshot ******************************/ + /** + * @brief If my topic is in the jmessage then force-publish my status + * @param[in] jmessage subscribed-received JSON message + * + */ void Interface::handletopic_snapshot( const nlohmann::json &jmessage ) { if ( jmessage.contains( Topic::FOCUSD ) ) { - this->publish_status(); + this->publish_status(true); } } + /***** Focus::Interface::handletopic_snapshot ******************************/ + /***** Focus::Interface::test ***********************************************/ /** diff --git a/tcsd/tcs_interface.cpp b/tcsd/tcs_interface.cpp index b35eb2a2b..1fe8bee7 100644 --- a/tcsd/tcs_interface.cpp +++ b/tcsd/tcs_interface.cpp @@ -1515,7 +1515,6 @@ namespace TCS { if ( caseCompareString( state, "off" ) ) req_state = 0; // no default lamp, must be specified - // { std::lock_guard lock(tcs_info_mtx); auto lamploc = this->tcs_info.lampinfo.find(which); @@ -1576,7 +1575,7 @@ namespace TCS { long error = this->send_command( cmd.str(), reply, TCS::FAST_RESPONSE ); // the mechanism takes time to respond and the TCS only checks for - // this command once per second. + // this command once per second. yes it really takes this long. if (error==NO_ERROR) { std::this_thread::sleep_for(std::chrono::milliseconds(2000)); }