[kstars] kstars/ekos/guide/externalguide: PHD2: honor the "Receive external guide frames" setting. Also, only log event messages from PHD2 when verbose logging is enabled

Jasem Mutlaq null at kde.org
Sun Jul 29 09:16:12 BST 2018


Git commit 9be2169dc508d6b52dcb1a82b478ea98afa109b6 by Jasem Mutlaq, on behalf of Andy Galasso.
Committed on 29/07/2018 at 08:07.
Pushed by mutlaqja into branch 'master'.

PHD2: honor the "Receive external guide frames" setting. Also, only log event messages from PHD2 when verbose logging is enabled
PHD2: only request a new star image if there is not already an outstanding requests to get the star image
PHD2: additional communication reliablity work

    - prevent problem with intermittent dropped messages by only sending a single JSONRPC request at a time
    - added more debug logging
    - use the documented set of phd2 states; remove custom states and use new state variables for dither state
    - fix error that aborts guiding when a dither is started before the initial settling has completed
    - fix dither timeouts being caused by the dither timer expiring sooner than the timeout sent to PHD2
PHD2: fix error loading PHD2 star image

    In every other frame the temporary .fit file was getting deleted before it could be displayed.

    Also, fix a possible file descriptor leak that could occur if a fitsio function call failed.

PHD2: do not process partial JSON responses, wait until the full resonse (terminated by newline) is received

    The existing code could prematurely process incomplete chunks of phd2 response messages. Now we will
    wait for a full newline-terminated message before processing the response. This also allows us to remove
    a bunch of workaround code that was trying to deal with chopped-up partial messages.

CCMAIL:kstars-devel at kde.org
CCMAIL:rlancaster at gmail.com
BUGS:392392
FIXED-IN:3.0.0

M  +294  -193  kstars/ekos/guide/externalguide/phd2.cpp
M  +44   -31   kstars/ekos/guide/externalguide/phd2.h

https://commits.kde.org/kstars/9be2169dc508d6b52dcb1a82b478ea98afa109b6

diff --git a/kstars/ekos/guide/externalguide/phd2.cpp b/kstars/ekos/guide/externalguide/phd2.cpp
index 21e041eff..b60a8c79d 100644
--- a/kstars/ekos/guide/externalguide/phd2.cpp
+++ b/kstars/ekos/guide/externalguide/phd2.cpp
@@ -14,13 +14,14 @@
 #include "kstars.h"
 
 #include "ekos/ekosmanager.h"
+#include "fitsviewer/fitsdata.h"
 
+#include <cassert>
 #include <fitsio.h>
 #include <KMessageBox>
 #include <QImage>
 
 #include <QJsonDocument>
-#include <QJsonObject>
 #include <QtNetwork/QNetworkReply>
 
 #include <ekos_guide_debug.h>
@@ -117,16 +118,18 @@ PHD2::PHD2()
     ditherTimer = new QTimer(this);
     connect(ditherTimer, &QTimer::timeout, this, [=]
     {
+        qCDebug(KSTARS_EKOS_GUIDE) << "ditherTimer expired, state" << state << "dithering" << isDitherActive << "settling" << isSettling;
         ditherTimer->stop();
+        isDitherActive = false;
+        isSettling = false;
         if (Options::ditherFailAbortsAutoGuide())
         {
-            state = DITHER_FAILED;
+            abort();
             emit newStatus(GUIDE_DITHERING_ERROR);
         }
         else
         {
             emit newLog(i18n("PHD2: There was no dithering response from PHD2, but continue guiding."));
-            state = GUIDING;
             emit newStatus(Ekos::GUIDE_DITHERING_SUCCESS);
         }
     });
@@ -135,6 +138,7 @@ PHD2::PHD2()
 PHD2::~PHD2()
 {
     delete abortTimer;
+    delete ditherTimer;
 }
 
 bool PHD2::Connect()
@@ -149,12 +153,42 @@ bool PHD2::Connect()
     return true;
 }
 
+void PHD2::ResetConnectionState()
+{
+    connection = DISCONNECTED;
+
+    // clear the outstanding and queued RPC requests
+    pendingRpcResultType = NO_RESULT;
+    rpcRequestQueue.clear();
+
+    starImageRequested = false;
+    isSettling = false;
+    isDitherActive = false;
+
+    ditherTimer->stop();
+    abortTimer->stop();
+}
+
 bool PHD2::Disconnect()
 {
     if (connection == EQUIPMENT_CONNECTED)
+    {
+        // !FIXME-ag this is not reliable because we are just sending
+        // a blind request to disconnect equipment then close the
+        // socket. PHD2 will refuse to disconnect the equipment if it
+        // is still looping exposures
+        //
+        // to be reliable it should:
+        //    call stop_capture
+        //    wait for the stop to complete by polling get_app_state
+        //    disconnect equipment
+        //    disconnect from phd2
+
         connectEquipment(false);
+    }
+
+    ResetConnectionState();
 
-    connection = DISCONNECTED;
     tcpSocket->disconnectFromHost();
 
     emit newStatus(GUIDE_DISCONNECTED);
@@ -181,59 +215,47 @@ void PHD2::displayError(QAbstractSocket::SocketError socketError)
             emit newLog(i18n("The following error occurred: %1.", tcpSocket->errorString()));
     }
 
-    connection = DISCONNECTED;
+    ResetConnectionState();
 }
 
 void PHD2::readPHD2()
 {
-    QTextStream stream(tcpSocket);
-
-    QJsonParseError qjsonError;
-
-    while (stream.atEnd() == false)
+    while (!tcpSocket->atEnd() && tcpSocket->canReadLine())
     {
-        QString rawString = stream.readLine();
-
-        if (rawString.isEmpty())
+        QByteArray line = tcpSocket->readLine();
+        if (line.isEmpty())
             continue;
 
-        QJsonDocument jdoc = QJsonDocument::fromJson(rawString.toLatin1(), &qjsonError);
+        QJsonParseError qjsonError;
+
+        QJsonDocument jdoc = QJsonDocument::fromJson(line, &qjsonError);
 
         if (qjsonError.error != QJsonParseError::NoError)
         {
-            //This will remove a method that had parsing errors from the request list.
-            removeBrokenRequestFromList(rawString);
-
-            //So we don't spam the error log with image frames that accidentally get broken up.
-            if(rawString.contains("frame"))     //This prevents it from printing the first line of the error.
-                blockLine2=true;                //This will set it to watch for the second line to cause an error.
-            else if(blockLine2)                 //This will prevent it from printing the second line of the error.
-                blockLine2=false;               //After avoiding printing the error message, this will set it to look for errors again.
-            else
-            {
-                //This will still print other parsing errors that don't involve image frames.
-                emit newLog(rawString);
-                emit newLog(qjsonError.errorString());
-            }
+            emit newLog(i18n("PHD2: invalid response received: %1", QString(line)));
+            emit newLog(i18n("PHD2: JSON error: %1", qjsonError.errorString()));
             continue;
         }
 
         QJsonObject jsonObj = jdoc.object();
 
         if (jsonObj.contains("Event"))
-            processPHD2Event(jsonObj);
+            processPHD2Event(jsonObj, line);
         else if (jsonObj.contains("error"))
-            processPHD2Error(jsonObj);
+            processPHD2Error(jsonObj, line);
         else if (jsonObj.contains("result"))
-            processPHD2Result(jsonObj, rawString);
+            processPHD2Result(jsonObj, line);
     }
 }
 
-void PHD2::processPHD2Event(const QJsonObject &jsonEvent)
+void PHD2::processPHD2Event(const QJsonObject &jsonEvent, const QByteArray &line)
 {
+    if (Options::verboseLogging())
+        qCDebug(KSTARS_EKOS_GUIDE) << "PHD2: event:" << line;
+
     QString eventName = jsonEvent["Event"].toString();
 
-    if (events.contains(eventName) == false)
+    if (!events.contains(eventName))
     {
         emit newLog(i18n("Unknown PHD2 event: %1", eventName));
         return;
@@ -246,23 +268,15 @@ void PHD2::processPHD2Event(const QJsonObject &jsonEvent)
         case Version:
             emit newLog(i18n("PHD2: Version %1", jsonEvent["PHDVersion"].toString()));
             connection = CONNECTED;
-            //This will give the other initial messages from PHD2 a chance to come in first.
-            //And then if the equipment is not already connected, then try to connect it.
-            QTimer::singleShot(100, [=]{
-                if(connection != EQUIPMENT_CONNECTED)
-                    connectEquipment(true);
-            });
             break;
 
         case CalibrationComplete:
-            state = GUIDING;
             setEquipmentConnected();
             emit newLog(i18n("PHD2: Calibration Complete."));
             emit newStatus(Ekos::GUIDE_CALIBRATION_SUCESS);
             break;
 
         case StartGuiding:
-            state = GUIDING;
             setEquipmentConnected();
             updateGuideParameters();
             emit newLog(i18n("PHD2: Guiding Started."));
@@ -282,11 +296,14 @@ void PHD2::processPHD2Event(const QJsonObject &jsonEvent)
             break;
 
         case AppState:
+            // AppState is the last of the initial messages received when we first connect to PHD2
             processPHD2State(jsonEvent["State"].toString());
+            // if the equipment is not already connected, then try to connect it.
+            if (connection != EQUIPMENT_CONNECTED)
+                connectEquipment(true);
             break;
 
         case CalibrationFailed:
-            state = CALIBRATION_FAILED;
             emit newLog(i18n("PHD2: Calibration Failed (%1).", jsonEvent["Reason"].toString()));
             emit newStatus(Ekos::GUIDE_CALIBRATION_ERROR);
             break;
@@ -296,10 +313,12 @@ void PHD2::processPHD2Event(const QJsonObject &jsonEvent)
             break;
 
         case LoopingExposures:
+            state = LOOPING;
             //emit newLog(i18n("PHD2: Looping Exposures."));
             break;
 
         case LoopingExposuresStopped:
+            state = STOPPED;
             emit newLog(i18n("PHD2: Looping Exposures Stopped."));
             break;
 
@@ -318,29 +337,36 @@ void PHD2::processPHD2Event(const QJsonObject &jsonEvent)
                 emit newLog(i18n("PHD2: Settling failed (%1).", jsonEvent["Error"].toString()));
             }
 
-            if (state == GUIDING)
-            {
-                if (error)
-                    state = STOPPED;
-                else
-                    emit newLog(i18n("PHD2: Settling complete, Guiding Started."));
-            }
-            else if (state == DITHERING)
+            bool wasDithering = isDitherActive;
+
+            isDitherActive = false;
+            isSettling = false;
+
+            if (wasDithering)
             {
                 ditherTimer->stop();
                 if (error && Options::ditherFailAbortsAutoGuide())
                 {
-                    state = DITHER_FAILED;
+                    abort();
                     emit newStatus(GUIDE_DITHERING_ERROR);
                 }
                 else
                 {
-                    if(error)
-                         emit newLog(i18n("PHD2: There was a dithering error, but continue guiding."));
-                    state = GUIDING;
+                    if (error)
+                        emit newLog(i18n("PHD2: There was a dithering error, but continue guiding."));
+
                     emit newStatus(Ekos::GUIDE_DITHERING_SUCCESS);
                 }
             }
+            else
+            {
+                // settle completed after "guide" command
+
+                if (!error)
+                {
+                    emit newLog(i18n("PHD2: Settling complete, Guiding Started."));
+                }
+            }
         }
         break;
 
@@ -350,7 +376,7 @@ void PHD2::processPHD2Event(const QJsonObject &jsonEvent)
 
         case StarLost:
             emit newLog(i18n("PHD2: Star Lost. Trying to reacquire."));
-            if(state != LOSTLOCK)
+            if (state != LOSTLOCK)
             {
                 state = LOSTLOCK;
                 abortTimer->start(starReAcquisitionTime);
@@ -358,33 +384,32 @@ void PHD2::processPHD2Event(const QJsonObject &jsonEvent)
             break;
 
         case GuidingStopped:
-            emit newLog(i18n("PHD2: Guiding Stopped."));
             state = STOPPED;
+            emit newLog(i18n("PHD2: Guiding Stopped."));
             emit newStatus(Ekos::GUIDE_IDLE);
             break;
 
         case Resumed:
             emit newLog(i18n("PHD2: Guiding Resumed."));
             emit newStatus(Ekos::GUIDE_GUIDING);
-            state = GUIDING;
             break;
 
         case GuideStep:
         {
-            if(state == DITHERING)
-                    return;
-            if( state == LOSTLOCK)
+            if (state == LOSTLOCK)
             {
                 emit newLog(i18n("PHD2: Star found, guiding resumed."));
                 abortTimer->stop();  
                 state = GUIDING;
             }
-            else if(state != GUIDING)
+            else if (state != GUIDING)
             {
                 emit newLog(i18n("PHD2: Guiding started up again."));
                 emit newStatus(Ekos::GUIDE_GUIDING);
                 state = GUIDING;
             }
+            if (isDitherActive)
+                return;
             double diff_ra_pixels, diff_de_pixels, diff_ra_arcsecs, diff_de_arcsecs, pulse_ra, pulse_dec;
             QString RADirection, DECDirection;
             diff_ra_pixels = jsonEvent["RADistanceRaw"].toDouble();
@@ -394,13 +419,13 @@ void PHD2::processPHD2Event(const QJsonObject &jsonEvent)
             RADirection = jsonEvent["RADirection"].toString();
             DECDirection = jsonEvent["DECDirection"].toString();
 
-            if(RADirection == "East")
+            if (RADirection == "East")
                 pulse_ra = -pulse_ra;  //West Direction is Positive, East is Negative
-            if(DECDirection == "South")
+            if (DECDirection == "South")
                 pulse_dec = -pulse_dec; //South Direction is Negative, North is Positive
 
             //If the pixelScale is properly set from PHD2, the second block of code is not needed, but if not, we will attempt to calculate the ra and dec error without it.
-            if(pixelScale!=0)
+            if (pixelScale != 0)
             {
                 diff_ra_arcsecs = diff_ra_pixels * pixelScale;
                 diff_de_arcsecs = diff_de_pixels * pixelScale;
@@ -438,11 +463,6 @@ void PHD2::processPHD2Event(const QJsonObject &jsonEvent)
         break;
 
         case GuidingDithered:
-            if(state == GUIDING)
-            {
-                state = DITHERING;
-                emit newStatus(Ekos::GUIDE_DITHERING);
-            }
             break;
 
         case LockPositionSet:
@@ -481,7 +501,7 @@ void PHD2::processPHD2State(const QString &phd2State)
         state = SELECTED;
     else if (phd2State == "Calibrating")
         state = CALIBRATING;
-    else if (phd2State == "GUIDING")
+    else if (phd2State == "Guiding")
         state = GUIDING;
     else if (phd2State == "LostLock")
         state = LOSTLOCK;
@@ -491,14 +511,16 @@ void PHD2::processPHD2State(const QString &phd2State)
         state = LOOPING;
 }
 
-void PHD2::processPHD2Result(const QJsonObject &jsonObj, QString rawString)
+void PHD2::processPHD2Result(const QJsonObject &jsonObj, const QByteArray &line)
 {
-    PHD2ResultType resultRequest = takeRequestFromList(jsonObj);
+    PHD2ResultType resultType = takeRequestFromList(jsonObj);
 
-    if(resultRequest != STAR_IMAGE)  //This is so we don't spam the log with Image Data.
-        qCDebug(KSTARS_EKOS_GUIDE) << rawString;
+    if (resultType == STAR_IMAGE)
+        qCDebug(KSTARS_EKOS_GUIDE) << "PHD2: received star image response, id" << jsonObj["id"].toInt();   // don't spam the log with image data
+    else
+        qCDebug(KSTARS_EKOS_GUIDE) << "PHD2: response:" << line;
 
-    switch (resultRequest)
+    switch (resultType)
     {
         case NO_RESULT:
             //Ekos didn't ask for this result?
@@ -509,7 +531,6 @@ void PHD2::processPHD2Result(const QJsonObject &jsonObj, QString rawString)
             break;
 
         case DITHER_COMMAND_RECEIVED:               //dither
-            state = DITHERING;
             emit newStatus(Ekos::GUIDE_DITHERING);
             break;
 
@@ -576,7 +597,7 @@ void PHD2::processPHD2Result(const QJsonObject &jsonObj, QString rawString)
 
         case PIXEL_SCALE:                           //get_pixel_scale
             pixelScale=jsonObj["result"].toDouble();
-            if(pixelScale == 0 )
+            if (pixelScale == 0)
                 emit newLog(i18n("PHD2: Please set CCD and telescope parameters in PHD2, Pixel Scale is invalid."));
             else
                 emit newLog(i18n("PHD2: Pixel Scale is %1 arcsec per pixel", QString::number(pixelScale, 'f', 2)));
@@ -589,11 +610,9 @@ void PHD2::processPHD2Result(const QJsonObject &jsonObj, QString rawString)
 
         case STAR_IMAGE:                            //get_star_image
         {
+            starImageRequested = false;
             QJsonObject jsonResult = jsonObj["result"].toObject();
-            if (jsonResult.contains("frame"))
-            {
-                processStarImage(jsonResult);
-            }
+            processStarImage(jsonResult);
             break;
         }
 
@@ -630,52 +649,58 @@ void PHD2::processPHD2Result(const QJsonObject &jsonObj, QString rawString)
         case STOP_CAPTURE_COMMAND_RECEIVED:         //stop_capture
             break;
     }
+
+    // send the next pending call
+    sendNextRpcCall();
 }
 
-void PHD2::processPHD2Error(const QJsonObject &jsonError)
+void PHD2::processPHD2Error(const QJsonObject &jsonError, const QByteArray &line)
 {
+    qCDebug(KSTARS_EKOS_GUIDE) << "PHD2: error:" << line;
+
     QJsonObject jsonErrorObject = jsonError["error"].toObject();
 
     emit newLog(i18n("PHD2 Error: %1", jsonErrorObject["message"].toString()));
 
-    if(jsonError.contains("id"))
-    {
+    PHD2ResultType resultType = takeRequestFromList(jsonError);
 
-        PHD2ResultType resultRequest = takeRequestFromList(jsonError);
-
-        //There are just a couple of requests that currently need further handling when they error, so a switch statement is not needed.
+    // This means the user mistakenly entered an invalid exposure time.
+    if (resultType == SET_EXPOSURE_COMMAND_RECEIVED)
+    {
+        emit newLog(logValidExposureTimes);  //This will let the user know the valid exposure durations
+        QTimer::singleShot(300, [=]{requestExposureTime();}); //This will reset the Exposure time in Ekos to PHD2's current exposure time after a third of a second.
+    }
+    else if (resultType == CONNECTION_RESULT)
+    {
+        connection = EQUIPMENT_DISCONNECTED;
+        emit newStatus(Ekos::GUIDE_DISCONNECTED);
+    }
+    else if (resultType == DITHER_COMMAND_RECEIVED)
+    {
+        ditherTimer->stop();
+        isSettling = false;
+        isDitherActive = false;
+        emit newStatus(GUIDE_DITHERING_ERROR);
 
-        //This means the user mistakenly entered an invalid exposure time.
-        if(resultRequest == SET_EXPOSURE_COMMAND_RECEIVED)
+        if (Options::ditherFailAbortsAutoGuide())
         {
-            emit newLog(logValidExposureTimes);  //This will let the user know the valid exposure durations
-            QTimer::singleShot(300, [=]{requestExposureTime();}); //This will reset the Exposure time in Ekos to PHD2's current exposure time after a third of a second.
+            abort();
+            emit newStatus(GUIDE_ABORTED);
         }
-        else if(resultRequest == CONNECTION_RESULT)
+        else
         {
-            connection = EQUIPMENT_DISCONNECTED;
-            emit newStatus(Ekos::GUIDE_DISCONNECTED);
+            // !FIXME-ag why is this trying to resume (un-pause)?
+            resume();
         }
-        else if(resultRequest == DITHER_COMMAND_RECEIVED && state == DITHERING)
-        {
-            ditherTimer->stop();
-            state = DITHER_FAILED;
-            emit newStatus(GUIDE_DITHERING_ERROR);
-
-            if (Options::ditherFailAbortsAutoGuide())
-            {
-                state = STOPPED;
-                emit newStatus(GUIDE_ABORTED);
-            }
-            else
-            {
-                resume();
-            }
-         }
     }
-}
-
+    else if (resultType == GUIDE_COMMAND_RECEIVED)
+    {
+        isSettling = false;
+    }
 
+    // send the next pending call
+    sendNextRpcCall();
+}
 
 //These methods process the Star Images the PHD2 provides
 
@@ -690,8 +715,13 @@ void PHD2::processStarImage(const QJsonObject &jsonStarFrame)
    int width =  jsonStarFrame["width"].toInt();
    int height = jsonStarFrame["height"].toInt();
 
-   //This sets up the Temp file which will be reused for subsequent captures
-   QString filename = KSPaths::writableLocation(QStandardPaths::TempLocation) + QLatin1Literal("phd2.fits");
+   QTemporaryFile tempfile(KSPaths::writableLocation(QStandardPaths::TempLocation) + QLatin1Literal("phd2_XXXXXX"));
+   if (!tempfile.open())
+   {
+       qCWarning(KSTARS_EKOS_GUIDE) << "could not create temp file for PHD2 star image";
+       return;
+   }
+   QString filename = tempfile.fileName();
 
    //This section sets up the FITS File
    fitsfile *fptr = nullptr;
@@ -709,6 +739,8 @@ void PHD2::processStarImage(const QJsonObject &jsonStarFrame)
    if (fits_create_img(fptr, USHORT_IMG, naxis, naxes, &status))
    {
        qCWarning(KSTARS_EKOS_GUIDE) << "fits_create_img failed:" << error_status;
+       status = 0;
+       fits_close_file(fptr, &status);
        return;
    }
 
@@ -727,6 +759,8 @@ void PHD2::processStarImage(const QJsonObject &jsonStarFrame)
    {
        fits_get_errstatus(status, error_status);
        qCWarning(KSTARS_EKOS_GUIDE) << "fits_write_img failed:" << error_status;
+       status = 0;
+       fits_close_file(fptr, &status);
        return;
    }
 
@@ -734,6 +768,8 @@ void PHD2::processStarImage(const QJsonObject &jsonStarFrame)
    {
        fits_get_errstatus(status, error_status);
        qCWarning(KSTARS_EKOS_GUIDE) << "fits_flush_file failed:" << error_status;
+       status = 0;
+       fits_close_file(fptr, &status);
        return;
    }
 
@@ -746,9 +782,10 @@ void PHD2::processStarImage(const QJsonObject &jsonStarFrame)
 
    //This loads the FITS file in the Guide FITSView
    //Then it updates the Summary Screen
-   bool imageLoad = guideFrame->loadFITS(filename, true);
-   if (imageLoad)
+   bool imageLoaded = guideFrame->loadFITS(filename, true);
+   if (imageLoaded)
    {
+       guideFrame->getImageData()->setAutoRemoveTemporaryFITS(false); // we'll take care of deleting the temp file
        guideFrame->updateFrame();
        guideFrame->setTrackingBox(QRect(0,0,width,height));
        emit newStarPixmap(guideFrame->getTrackingBoxPixmap());
@@ -763,23 +800,18 @@ void PHD2::setEquipmentConnected()
         connection = EQUIPMENT_CONNECTED;
         emit newStatus(Ekos::GUIDE_CONNECTED);
         updateGuideParameters();
-        QTimer::singleShot(800, [=]{requestExposureDurations();});
+        requestExposureDurations();
     }
 }
 
 void PHD2::updateGuideParameters()
 {
-    if(pixelScale == 0)
+    if (pixelScale == 0)
         requestPixelScale();
-    //So that the commands aren't too quick for PHD2, a slight delay before asking for the other items.
-    QTimer::singleShot(100, [=]{requestExposureTime();});
-    QTimer::singleShot(400, [=]{checkDEGuideMode();});
+    requestExposureTime();
+    checkDEGuideMode();
 }
 
-
-
-
-
 //This section handles the methods/requests sent to PHD2, some are not implemented.
 
 //capture_single_frame
@@ -810,12 +842,28 @@ bool PHD2::dither(double pixels)
         return false;
     }
 
+    if (isSettling)
+    {
+        qCDebug(KSTARS_EKOS_GUIDE) << "PHD2: ignoring dither requested while already settling";
+
+        if (!isDitherActive)
+        {
+            // act like we just dithered so we get the appropriate
+            // effects after the settling completes
+            emit newStatus(Ekos::GUIDE_DITHERING);
+            isDitherActive = true;
+        }
+        return true;
+    }
+
     QJsonArray args;
     QJsonObject settle;
 
+    int ditherTimeout = static_cast<int>(Options::ditherTimeout());
+
     settle.insert("pixels", static_cast<double>(Options::ditherThreshold()));
     settle.insert("time", static_cast<int>(Options::ditherSettle()));
-    settle.insert("timeout", static_cast<int>(Options::ditherTimeout()));
+    settle.insert("timeout", ditherTimeout);
 
     // Pixels
     args << pixels;
@@ -824,11 +872,23 @@ bool PHD2::dither(double pixels)
     // Settle
     args << settle;
 
-    state = DITHERING;
-    ditherTimer->start(static_cast<int>(Options::ditherTimeout())*1000); //Timer is in ms, timeout is in sec
+    isSettling = true;
+    isDitherActive = true;
+
+    // PHD2 will send a SettleDone event shortly after the settling
+    // timeout in PHD2. We don't really need a timer here, but we'll
+    // set one anyway (belt and suspenders). Make sure to give an
+    // extra time allowance since PHD2 won't report its timeout until
+    // the completion of the next guide exposure after the timeout
+    // period expires.
+    enum { TIMEOUT_EXTRA_SECONDS = 60 };  // at least as long as any reasonable guide exposure
+    int millis = (ditherTimeout + TIMEOUT_EXTRA_SECONDS) * 1000;
+    ditherTimer->start(millis);
 
     sendPHD2Request("dither", args);
 
+    emit newStatus(Ekos::GUIDE_DITHERING);
+
     return true;
 }
 
@@ -843,29 +903,28 @@ bool PHD2::dither(double pixels)
 //get_connected
 void PHD2::checkIfEquipmentConnected()
 {
-    QJsonArray args;
-    sendPHD2Request("get_connected", args);
+    sendPHD2Request("get_connected");
 }
 
 //get_cooler_status
 //get_current_equipment
 
 //get_dec_guide_mode
-void PHD2::checkDEGuideMode(){
-    QJsonArray args;
-    sendPHD2Request("get_dec_guide_mode", args);
+void PHD2::checkDEGuideMode()
+{
+    sendPHD2Request("get_dec_guide_mode");
 }
 
 //get_exposure
-void PHD2::requestExposureTime(){
-    QJsonArray args;
-    sendPHD2Request("get_exposure", args);
+void PHD2::requestExposureTime()
+{
+    sendPHD2Request("get_exposure");
 }
 
 //get_exposure_durations
-void PHD2::requestExposureDurations(){
-    QJsonArray args;
-    sendPHD2Request("get_exposure_durations", args);
+void PHD2::requestExposureDurations()
+{
+    sendPHD2Request("get_exposure_durations");
 }
 
 //get_lock_position
@@ -874,9 +933,9 @@ void PHD2::requestExposureDurations(){
 //get_paused
 
 //get_pixel_scale
-void PHD2::requestPixelScale(){
-    QJsonArray args;
-    sendPHD2Request("get_pixel_scale", args);
+void PHD2::requestPixelScale()
+{
+    sendPHD2Request("get_pixel_scale");
 }
 
 //get_profile
@@ -885,10 +944,23 @@ void PHD2::requestPixelScale(){
 //get_sensor_temperature
 
 //get_star_image
-void PHD2::requestStarImage(int size){
+void PHD2::requestStarImage(int size)
+{
+    if (!Options::guideRemoteImagesEnabled())
+        return;
+
+    if (starImageRequested)
+    {
+        if (Options::verboseLogging())
+            qCDebug(KSTARS_EKOS_GUIDE) << "PHD2: skip extra star image request";
+        return;
+    }
+
     QJsonArray args2;
-    args2<<size; //This is both the width and height.
-    sendPHD2Request("get_star_image", args2); //Note we don't want to add it to the request list since this request type is handled before the list is checked.
+    args2 << size; // This is both the width and height.
+    sendPHD2Request("get_star_image", args2);
+
+    starImageRequested = true;
 }
 
 //get_use_subframes
@@ -923,6 +995,8 @@ bool PHD2::guide()
 
     errorLog.clear();
 
+    isSettling = true;
+
     sendPHD2Request("guide", args);
 
     return true;
@@ -955,17 +1029,17 @@ void PHD2::connectEquipment(bool enable)
     // connected = enable
     args << enable;
 
-    if(enable)
+    if (enable)
         emit newLog(i18n("PHD2: Connecting Equipment. . ."));
     else
         emit newLog(i18n("PHD2: Disconnecting Equipment. . ."));
 
     sendPHD2Request("set_connected", args);
-
 }
 
 //set_dec_guide_mode
-void PHD2::requestSetDEGuideMode(bool deEnabled, bool nEnabled, bool sEnabled){ //Possible Settings Off, Auto, North, and South
+void PHD2::requestSetDEGuideMode(bool deEnabled, bool nEnabled, bool sEnabled) //Possible Settings Off, Auto, North, and South
+{
     QJsonArray args;
 
     if(deEnabled)
@@ -983,16 +1057,16 @@ void PHD2::requestSetDEGuideMode(bool deEnabled, bool nEnabled, bool sEnabled){
     {
         args << "Off";
     }
-    sendPHD2Request("set_dec_guide_mode", args);
 
+    sendPHD2Request("set_dec_guide_mode", args);
 }
 
 //set_exposure
-void PHD2::requestSetExposureTime(int time){ //Note: time is in milliseconds
+void PHD2::requestSetExposureTime(int time) //Note: time is in milliseconds
+{
     QJsonArray args;
-    args<<time;
+    args << time;
     sendPHD2Request("set_exposure", args);
-
 }
 
 //set_lock_position
@@ -1057,9 +1131,6 @@ bool PHD2::abort()
     return true;
 }
 
-
-
-
 //This method is not handled by PHD2
 bool PHD2::calibrate()
 {
@@ -1068,67 +1139,97 @@ bool PHD2::calibrate()
     return true;
 }
 
+//This is how information requests and commands for PHD2 are handled
 
+void PHD2::sendRpcCall(QJsonObject &call, PHD2ResultType resultType)
+{
+    assert(resultType != NO_RESULT); // should be a real request
+    assert(pendingRpcResultType == NO_RESULT);  // only one pending RPC call at a time
 
+    int rpcId = nextRpcId++;
+    call.insert("id", rpcId);
 
+    pendingRpcId = rpcId;
+    pendingRpcResultType = resultType;
 
+    QByteArray request = QJsonDocument(call).toJson(QJsonDocument::Compact);
 
-//This is how information requests and commands for PHD2 are handled
+    qCDebug(KSTARS_EKOS_GUIDE) << "PHD2: request:" << request;
+
+    request.append("\r\n");
+    qint64 n = tcpSocket->write(request);
+
+    if ((int) n != request.size())
+    {
+        qCDebug(KSTARS_EKOS_GUIDE) << "PHD2: unexpected short write:" << n << "bytes of" << request.size();
+    }
+}
+
+void PHD2::sendNextRpcCall()
+{
+    if (pendingRpcResultType != NO_RESULT)
+        return; // a request is currently outstanding
+
+    if (rpcRequestQueue.empty())
+        return; // no queued requests
+
+    RpcCall& call = rpcRequestQueue.front();
+    sendRpcCall(call.call, call.resultType);
+    rpcRequestQueue.pop_front();
+}
 
-void PHD2::sendPHD2Request(const QString &method, const QJsonArray args)
+void PHD2::sendPHD2Request(const QString &method, const QJsonArray &args)
 {
+    assert(methodResults.contains(method));
+
+    PHD2ResultType resultType = methodResults[method];
+
     QJsonObject jsonRPC;
 
     jsonRPC.insert("jsonrpc", "2.0");
     jsonRPC.insert("method", method);
 
-    if (args.empty() == false)
+    if (!args.empty())
         jsonRPC.insert("params", args);
 
-    resultRequests.append(qMakePair(methodID,method));
-    jsonRPC.insert("id", methodID++);
-
-    QJsonDocument json_doc(jsonRPC);
+    if (pendingRpcResultType == NO_RESULT)
+    {
+        // no outstanding rpc call, send it right off
+        sendRpcCall(jsonRPC, resultType);
+    }
+    else
+    {
+        // there is already an oustanding call, enqueue this call
+        // until the prior call completes
 
-    //emit newLog(json_doc.toJson(QJsonDocument::Compact));
-    qCDebug(KSTARS_EKOS_GUIDE) << json_doc.toJson(QJsonDocument::Compact);
+        if (Options::verboseLogging())
+            qCDebug(KSTARS_EKOS_GUIDE) << "PHD2: defer call" << method;
 
-    tcpSocket->write(json_doc.toJson(QJsonDocument::Compact));
-    tcpSocket->write("\r\n");
+        rpcRequestQueue.push_back(RpcCall(jsonRPC, resultType));
+    }
 }
 
-PHD2::PHD2ResultType PHD2::takeRequestFromList(const QJsonObject &jsonObj)
+PHD2::PHD2ResultType PHD2::takeRequestFromList(const QJsonObject &response)
 {
-    PHD2ResultType resultRequest = NO_RESULT;
-    int id = jsonObj["id"].toInt();
-
-    for(int i = 0; i < resultRequests.size(); i++){
-        QPair<int, QString> request = resultRequests.at(i);
-        if(request.first == id){
-            resultRequest = methodResults.value(request.second);
-            resultRequests.remove(i);
-            break;
-        }
+    if (Q_UNLIKELY(!response.contains("id")))
+    {
+        qCDebug(KSTARS_EKOS_GUIDE) << "PHD2: ignoring unexpected response with no id";
+        return NO_RESULT;
     }
-    return resultRequest;
-}
 
-void PHD2::removeBrokenRequestFromList(QString rawString)
-{
-    if(rawString.contains("\"id\":"))
+    int id = response["id"].toInt();
+
+    if (Q_UNLIKELY(id != pendingRpcId))
     {
-        int idLocation   = rawString.indexOf("\"id\":") + 5;
-        int endOfID = rawString.indexOf("}" , idLocation) - idLocation;
-        QString idString = rawString.mid(idLocation, endOfID);
-        int id = idString.toInt();
-        for(int i = 0; i < resultRequests.size(); i++){
-            QPair<int, QString> request = resultRequests.at(i);
-            if(request.first == id){
-                resultRequests.remove(i);
-                break;
-            }
-        }
+        // RPC id mismatch -- this should never happen, something is
+        // seriously wrong
+        qCDebug(KSTARS_EKOS_GUIDE) << "PHD2: ignoring unexpected response with id" << id;
+        return NO_RESULT;
     }
+
+    PHD2ResultType val = pendingRpcResultType;
+    pendingRpcResultType = NO_RESULT;
+    return val;
 }
 
 }
diff --git a/kstars/ekos/guide/externalguide/phd2.h b/kstars/ekos/guide/externalguide/phd2.h
index 49071bf32..ab92e9e95 100644
--- a/kstars/ekos/guide/externalguide/phd2.h
+++ b/kstars/ekos/guide/externalguide/phd2.h
@@ -11,10 +11,11 @@
 
 #include "../guideinterface.h"
 #include "fitsviewer/fitsview.h"
-#include <QPointer>
 
 #include <QAbstractSocket>
 #include <QJsonArray>
+#include <QJsonObject>
+#include <QPointer>
 #include <QTimer>
 
 class QTcpSocket;
@@ -33,7 +34,7 @@ class PHD2 : public GuideInterface
     Q_OBJECT
 
   public:
-    typedef enum {
+    enum PHD2Event {
         Version,
         LockPositionSet,
         CalibrationComplete,
@@ -57,36 +58,32 @@ class PHD2 : public GuideInterface
         LockPositionLost,
         Alert,
         GuideParamChange
-    } PHD2Event;
-    typedef enum {
+    };
+    enum PHD2State {
+        // these are the states exposed by phd2
         STOPPED,
         SELECTED,
+        CALIBRATING,
+        GUIDING,
         LOSTLOCK,
         PAUSED,
         LOOPING,
-        CALIBRATING,
-        CALIBRATION_FAILED,
-        CALIBRATION_SUCCESSFUL,
-        GUIDING,
-        DITHERING,
-        DITHER_FAILED,
-        DITHER_SUCCESSFUL
-    } PHD2State;
-    typedef enum {
+    };
+    enum PHD2Connection {
         DISCONNECTED,
         CONNECTED,
         EQUIPMENT_DISCONNECTED,
         EQUIPMENT_CONNECTED
-    } PHD2Connection;
-    typedef enum {
+    };
+    enum PHD2MessageType {
         PHD2_UNKNOWN,
         PHD2_RESULT,
         PHD2_EVENT,
         PHD2_ERROR,
-    } PHD2MessageType;
+    };
 
-    //These are the PHD2 Results and the commands they are associated with
-    typedef enum {
+    // These are the PHD2 Results and the commands they are associated with
+    enum PHD2ResultType {
         NO_RESULT,
             //capture_single_frame
         CLEAR_CALIBRATION_COMMAND_RECEIVED,     //clear_calibration
@@ -130,7 +127,7 @@ class PHD2 : public GuideInterface
             //set_profile
             //shutdown
         STOP_CAPTURE_COMMAND_RECEIVED           //stop_capture
-    } PHD2ResultType;
+    };
 
     PHD2();
     ~PHD2();
@@ -199,32 +196,47 @@ class PHD2 : public GuideInterface
 
     QVector<QPointF> errorLog;
 
-    void sendPHD2Request(const QString &method, const QJsonArray args = QJsonArray());
-    void sendJSONRPCRequest(const QString &method, const QJsonArray args = QJsonArray());
-    bool blockLine2=false;
+    void sendPHD2Request(const QString &method, const QJsonArray &args = QJsonArray());
+    void sendRpcCall(QJsonObject &call, PHD2ResultType resultType);
+    void sendNextRpcCall();
 
-    void processPHD2Event(const QJsonObject &jsonEvent);
-    void processPHD2Result(const QJsonObject &jsonObj, QString rawString);
+    void processPHD2Event(const QJsonObject &jsonEvent, const QByteArray &rawResult);
+    void processPHD2Result(const QJsonObject &jsonObj, const QByteArray &rawResult);
     void processStarImage(const QJsonObject &jsonStarFrame);
     void processPHD2State(const QString &phd2State);
-    void processPHD2Error(const QJsonObject &jsonError);
+    void processPHD2Error(const QJsonObject &jsonError, const QByteArray &rawResult);
+
+    PHD2ResultType takeRequestFromList(const QJsonObject &response);
 
     QTcpSocket *tcpSocket { nullptr };
-    qint64 methodID { 1 };
+    int nextRpcId { 1 };
+
+    QHash<QString, PHD2Event> events;                     // maps event name to event type
+    QHash<QString, PHD2ResultType> methodResults;         // maps method name to result type
 
-    QHash<QString, PHD2Event> events;
-    QHash<QString, PHD2ResultType> methodResults;
-    QVector<QPair<int, QString>> resultRequests;
+    int pendingRpcId;                         // ID of outstanding RPC call
+    PHD2ResultType pendingRpcResultType { NO_RESULT };      // result type of outstanding RPC call
+    bool starImageRequested { false };        // true when there is an outstanding star image request
+
+    struct RpcCall
+    {
+        QJsonObject call;
+        PHD2ResultType resultType;
+        RpcCall() = default;
+        RpcCall(const QJsonObject& call_, PHD2ResultType resultType_) : call(call_), resultType(resultType_) { }
+    };
+    QVector<RpcCall> rpcRequestQueue;
 
     PHD2State state { STOPPED };
+    bool isDitherActive { false };
+    bool isSettling { false };
     PHD2Connection connection { DISCONNECTED };
     PHD2Event event { Alert };
-    PHD2ResultType takeRequestFromList(const QJsonObject &jsonObj);
-    void removeBrokenRequestFromList(QString rawString);
     uint8_t setConnectedRetries { 0 };
 
     void setEquipmentConnected();
     void updateGuideParameters();
+    void ResetConnectionState();
 
     QTimer *abortTimer;
     QTimer *ditherTimer;
@@ -234,4 +246,5 @@ class PHD2 : public GuideInterface
 
     QString logValidExposureTimes;
 };
+
 }



More information about the Kstars-devel mailing list