From c3601055666eaaac56a0c458bcbd1e61a9f29d9a Mon Sep 17 00:00:00 2001 From: Ben Galvin Date: Fri, 8 Mar 2024 00:03:23 +1300 Subject: [PATCH 1/4] Fix iguider issues (#2012) * Fix iGuider/iPolar exposure issue * Attempt to improve iGuider start up times * Ensuring only valid durations/ticks are used * Address PR comments * Removing this, as it's not consistently helping --- drivers/video/v4l2driver.cpp | 167 ++++++++++++++++++++++++++++++----- drivers/video/v4l2driver.h | 7 ++ 2 files changed, 150 insertions(+), 24 deletions(-) diff --git a/drivers/video/v4l2driver.cpp b/drivers/video/v4l2driver.cpp index 314388f169..0370e47319 100644 --- a/drivers/video/v4l2driver.cpp +++ b/drivers/video/v4l2driver.cpp @@ -56,13 +56,25 @@ static const PixelSizeInfo CameraDatabase[] = { nullptr, nullptr, nullptr, 5.6f, -1, 0, 0, false} // sentinel and default pixel size, needs to be last }; +static const double IOptronDurations[] = { 0.01, 0.02, 0.05, 0.1, 0.2, 0.5, 1.0, 1.5, 2.0, 2.5, 3.0, 3.5, -1 }; +static const double IOptronTicks[] = { 1, 2, 5, 10, 20, 39, 78, 156, 312, 625, 1250, 2500, -1 }; + +#define MAX_IOPTRON_INDEX (sizeof(IOptronDurations) / sizeof(double) - 2) +#define MAX_IOPTRON_DURATION (IOptronDurations[MAX_IOPTRON_INDEX]) +#define MAX_IOPTRON_TICKS (IOptronTicks[MAX_IOPTRON_INDEX]) + +#define IOPTRON_ABS_EXPOSURE_CTRL_ID 0x9A0902 +#define IOPTRON_AUTO_EXPOSURE_CTRL_ID 0x9A0901 +#define IOPTRON_WATCHDOG_PERIOD_IN_MS 10000 + + V4L2_Driver::V4L2_Driver(std::string label, std::string path) { setDeviceName(label.c_str()); strncpy(defaultVideoPort, path.c_str(), 256); strncpy(configPort, path.c_str(), 256); - setVersion(1, 1); + setVersion(1, 2); allocateBuffers(); @@ -89,6 +101,9 @@ V4L2_Driver::V4L2_Driver(std::string label, std::string path) lx = new Lx(); lxtimer = -1; stdtimer = -1; + + ioptron_watchdog_timer.callOnTimeout(std::bind(&V4L2_Driver::iOptronWatchdogCallback, this)); + ioptron_watchdog_timer.setInterval(IOPTRON_WATCHDOG_PERIOD_IN_MS); } V4L2_Driver::V4L2_Driver() @@ -120,6 +135,8 @@ V4L2_Driver::V4L2_Driver() lx = new Lx(); lxtimer = -1; stdtimer = -1; + ioptron_watchdog_timer.callOnTimeout(std::bind(&V4L2_Driver::iOptronWatchdogCallback, this)); + ioptron_watchdog_timer.setInterval(IOPTRON_WATCHDOG_PERIOD_IN_MS); } V4L2_Driver::~V4L2_Driver() @@ -278,6 +295,7 @@ void V4L2_Driver::ISGetProperties(const char * dev) defineProperty(&ColorProcessingSP); defineProperty(&CaptureColorSpaceTP); #endif + } } @@ -720,6 +738,20 @@ bool V4L2_Driver::ISNewText(const char * dev, const char * name, char * texts[], return INDI::CCD::ISNewText(dev, name, texts, names, n); } +int findIndexOfNearestValue(double value, const double allowed_values[]) +{ + int n = 1; + while (allowed_values[n] != -1) + { + if (value < (allowed_values[n-1] + allowed_values[n]) / 2.0) + { + return n - 1; + } + n++; + } + return n - 1; +} + bool V4L2_Driver::ISNewNumber(const char * dev, const char * name, double values[], char * names[], int n) { char errmsg[ERRMSGSIZ]; @@ -779,7 +811,15 @@ bool V4L2_Driver::ISNewNumber(const char * dev, const char * name, double values for (int i = 0; i < ImageAdjustNP.nnp; i++) { unsigned int const ctrl_id = *((unsigned int *)ImageAdjustNP.np[i].aux0); - double const value = ImageAdjustNP.np[i].value; + + // iOptron cameras only accept some tick values. Round the value to the nearest one. + if (isIOptron() && ctrl_id == IOPTRON_ABS_EXPOSURE_CTRL_ID) + { + int n = findIndexOfNearestValue(ImageAdjustNP.np[i].value, IOptronTicks); + ImageAdjustNP.np[i].value = IOptronTicks[n]; + } + + const double value = ImageAdjustNP.np[i].value; LOGF_DEBUG(" Setting %s (%s) to %f, ctrl_id = 0x%X", ImageAdjustNP.np[i].name, ImageAdjustNP.np[i].label, value, ctrl_id); @@ -806,6 +846,12 @@ bool V4L2_Driver::ISNewNumber(const char * dev, const char * name, double values return true; } + // Ensure duration is set to one of the allowed durations, or, if we're going to stack, a value greater than the maximum + if (isIOptron() && strcmp(name, "CCD_EXPOSURE") == 0 && values[0] < MAX_IOPTRON_DURATION) + { + values[0] = IOptronDurations[findIndexOfNearestValue(values[0], IOptronDurations)]; + } + return INDI::CCD::ISNewNumber(dev, name, values, names, n); } @@ -881,6 +927,15 @@ bool V4L2_Driver::StartExposure(float duration) } else stdtimer = -1; + + // iGuider/iPolar occasionally fails to start. We deal with this in the same was + // as the iOptron ASCOM driver on windows: if it hasn't responded with a frame in + // 10 seconds, stop and start the capture. + if (isIOptron()) + { + valid_frame_has_arrived = false; + ioptron_watchdog_timer.start(); + } } return is_capturing; @@ -929,6 +984,11 @@ bool V4L2_Driver::setShutter(double duration) } } +bool V4L2_Driver::isIOptron() +{ + return strstr(getDeviceName(), "iGuider") || strstr(getDeviceName(), "iPolar"); +} + bool V4L2_Driver::setManualExposure(double duration) { /* N.B. Check how this differs from one camera to another. This is just a proof of concept for now */ @@ -940,11 +1000,18 @@ bool V4L2_Driver::setManualExposure(double duration) // INT control for manual exposure duration is an integer in 1/10000 seconds long ticks = lround(duration * 10000.0f); - // iOptron iPolar & iGuider have different tick rate - // With max about 5000 ticks - if (strstr(getDeviceName(), "iGuider") || strstr(getDeviceName(), "iPolar")) + // iOptron tick values are non-linear, so look them up. + bool ioptron_maximum_duration_exceeded = false; + if (isIOptron()) { - ticks = std::min(5000l, lround(duration * 1333.33)); + if (duration <= MAX_IOPTRON_DURATION) + { + ticks = IOptronTicks[findIndexOfNearestValue(duration, IOptronDurations)]; + } + else + { + ioptron_maximum_duration_exceeded = true; + } } /* First check the presence of an absolute exposure control */ @@ -983,7 +1050,7 @@ bool V4L2_Driver::setManualExposure(double duration) } } /* Then if we have an exposure control, check the requested exposure duration */ - else if (AbsExposureN->max < ticks) + else if (AbsExposureN->max < ticks || ioptron_maximum_duration_exceeded) { if( CaptureFormatSP[IMAGE_MONO].getState() == ISS_ON && m_StackMode == STACK_NONE ) { @@ -1004,7 +1071,18 @@ bool V4L2_Driver::setManualExposure(double duration) (double) AbsExposureN->min / 10000.0f, (double) AbsExposureN->max / 10000.0f, duration, StackModeSP[m_StackMode].getName()); } - ticks = AbsExposureN->max; + if (isIOptron()) + { + ticks = MAX_IOPTRON_TICKS; + frame_duration.tv_sec = floor(MAX_IOPTRON_DURATION); + frame_duration.tv_usec = (MAX_IOPTRON_DURATION - frame_duration.tv_sec) * 1000000; + } + else + { + ticks = AbsExposureN->max; + frame_duration.tv_sec = ticks / 10000; + frame_duration.tv_usec = (ticks % 10000) * 100; + } } /* We can't expose as long as requested and stacking is not configured, bail out */ else @@ -1015,19 +1093,15 @@ bool V4L2_Driver::setManualExposure(double duration) return false; } } - /* Lower-than-minimal exposure duration is left managed below */ - - - frame_duration.tv_sec = ticks / 10000; - frame_duration.tv_usec = (ticks % 10000) * 100; - - if (strstr(getDeviceName(), "iGuider") || strstr(getDeviceName(), "iPolar")) + else { - frame_duration.tv_sec = ticks / 1333; - frame_duration.tv_usec = (ticks % 1333) * 100; + frame_duration.tv_sec = floor(duration); + frame_duration.tv_usec = (duration - frame_duration.tv_sec) * 1000000; } - if( v4l_capture_started ) + /* Lower-than-minimal exposure duration is left managed below */ + + if ( v4l_capture_started ) { if( AbsExposureN->value != ticks ) { @@ -1128,6 +1202,26 @@ void V4L2_Driver::stdtimerCallback(void * userpointer) p->PrimaryCCD.setExposureLeft(remaining); } +// iPolar and iGuider cameras occasionally fail to restart once capturing is stopped. This occurs in Windows too. Here +// we take the same approach as the iOptron ASCOM driver and restart the capture if no frames have been received for a while. +void V4L2_Driver::iOptronWatchdogCallback() +{ + if (valid_frame_has_arrived) + { + ioptron_watchdog_timer.stop(); + return; + } + + LOG_DEBUG("Valid frame hasn't arrived yet. Restart capturing."); + + char errmsg[ERRMSGSIZ]; + v4l_base->stop_capturing(errmsg); + + v4l_base->setINTControl(IOPTRON_ABS_EXPOSURE_CTRL_ID, AbsExposureN->value, errmsg); + v4l_base->setOPTControl(IOPTRON_AUTO_EXPOSURE_CTRL_ID, 1, errmsg); // Manual + v4l_base->start_capturing(errmsg); +} + bool V4L2_Driver::start_capturing(bool do_stream) { // FIXME Must migrate completely to Stream @@ -1178,6 +1272,14 @@ bool V4L2_Driver::stop_capturing() return true; } + // For iGuider/iPolar we don't stop capturing, as it doesn't reliably restart. This + // is the same behaviour as IOptron's ASCOM driver in Windows. + if (isIOptron()) + { + is_capturing = false; + return true; + } + if (!Streamer->isBusy() && 0.0f < getRemainingExposure()) { LOGF_WARN("Stopping running exposure %.3f seconds before completion", @@ -1448,18 +1550,33 @@ void V4L2_Driver::newFrame() struct timeval capture_frame_dif = { .tv_sec = 0, .tv_usec = 0 }; timersub(&frame_received, &capture_start, &capture_frame_dif); - float cfd = (float) capture_frame_dif.tv_sec + (float) capture_frame_dif.tv_usec / 1000000.0f; - float fd = (float) frame_duration.tv_sec + (float) frame_duration.tv_usec / 1000000.0f; + float time_since_capture_started = (float) capture_frame_dif.tv_sec + (float) capture_frame_dif.tv_usec / 1000000.0f; + float expected_frame_duration = (float) frame_duration.tv_sec + (float) frame_duration.tv_usec / 1000000.0f; - if( cfd < fd * 0.9 ) + if( time_since_capture_started < expected_frame_duration * 0.9 ) { LOGF_DEBUG("Skip early frame cfd = %ld.%06ld seconds.", capture_frame_dif.tv_sec, capture_frame_dif.tv_usec); return; } + // Since we don't clear the frame buffer when changing exposures, we can get several frames coming through with the old + // duration. Discard these. + float current_frame_duration_f = (float) current_frame_duration.tv_sec + (float) current_frame_duration.tv_usec / 1000000.0f; + if( m_StackMode == STACK_NONE && (current_frame_duration_f < expected_frame_duration * 0.9 || current_frame_duration_f > expected_frame_duration * 1.1)) + { + LOGF_DEBUG("Skip frame with incorrect duration (%f seconds)", current_frame_duration_f); + return; + } + + valid_frame_has_arrived = true; + if (ioptron_watchdog_timer.isActive()) + { + ioptron_watchdog_timer.stop(); + } + timeradd(&elapsed_exposure, &frame_duration, &elapsed_exposure); - LOGF_DEBUG("Frame took %ld.%06ld s, e = %ld.%06ld s, t = %ld.%06ld s., cfd = %ld.%06ld s.", + LOGF_INFO("Frame took %ld.%06ld s, e = %ld.%06ld s, t = %ld.%06ld s., cfd = %ld.%06ld s.", current_frame_duration.tv_sec, current_frame_duration.tv_usec, elapsed_exposure.tv_sec, elapsed_exposure.tv_usec, exposure_duration.tv_sec, exposure_duration.tv_usec, @@ -1650,7 +1767,7 @@ void V4L2_Driver::newFrame() if (Streamer->isBusy() == false) stop_capturing(); - if (PrimaryCCD.getExposureDuration() >= 3) + //if (PrimaryCCD.getExposureDuration() >= 3) LOGF_INFO("Capture of LX frame took %ld.%06ld seconds.", current_exposure.tv_sec, current_exposure.tv_usec); ExposureComplete(&PrimaryCCD); } @@ -1669,6 +1786,7 @@ void V4L2_Driver::newFrame() if (PrimaryCCD.getExposureDuration() >= 3) LOGF_INFO("Capture of one frame (%d stacked frames) took %ld.%06ld seconds.", subframeCount, current_exposure.tv_sec, current_exposure.tv_usec); + ExposureComplete(&PrimaryCCD); } } @@ -1676,7 +1794,8 @@ void V4L2_Driver::newFrame() { non_capture_frames++; - if( non_capture_frames > 10 ) + // We don't abort for iGuider/iPolar as it doesn't reliably restart after having the stream stopped. This is the same behaviour as the ASCOM driver. + if( non_capture_frames > 10 && !isIOptron()) { /* If we arrive here, PrimaryCCD is not exposing anymore, we can't forward the frame and we can't be aborted neither, thus abort the exposure right now. * That issue can be reproduced when clicking the "Set" button on the "Main Control" tab while an exposure is running. diff --git a/drivers/video/v4l2driver.h b/drivers/video/v4l2driver.h index 1075162465..a1f010922c 100644 --- a/drivers/video/v4l2driver.h +++ b/drivers/video/v4l2driver.h @@ -186,6 +186,7 @@ class V4L2_Driver : public INDI::CCD bool startlongexposure(double timeinsec); static void lxtimerCallback(void *userpointer); static void stdtimerCallback(void *userpointer); + void iOptronWatchdogCallback(); /* start/stop functions */ bool start_capturing(bool do_stream); @@ -193,6 +194,10 @@ class V4L2_Driver : public INDI::CCD virtual void updateV4L2Controls(); + bool isIOptron(); + + void resetDevice(int bus_num, int dev_num); + /* Variables */ INDI::V4L2_Base *v4l_base; @@ -222,11 +227,13 @@ class V4L2_Driver : public INDI::CCD bool v4l_capture_started; bool is_capturing; bool is_exposing; + bool valid_frame_has_arrived; //Long Exposure Lx *lx; int lxtimer; int stdtimer; + INDI::Timer ioptron_watchdog_timer; short lxstate; PixelSizeInfo * m_Info {nullptr}; From 315deea40a1c1b762d222fa06739d98e1fb16086 Mon Sep 17 00:00:00 2001 From: Jasem Mutlaq Date: Thu, 7 Mar 2024 15:30:38 +0300 Subject: [PATCH 2/4] Add additional iPolar model --- drivers/video/v4l2driver.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/video/v4l2driver.cpp b/drivers/video/v4l2driver.cpp index 0370e47319..2a6d9a3154 100644 --- a/drivers/video/v4l2driver.cpp +++ b/drivers/video/v4l2driver.cpp @@ -42,8 +42,9 @@ static const PixelSizeInfo CameraDatabase[] = { "Skyris 132M", "Skyris 132M", nullptr, 3.75f, -1, 0, 0, false }, { "Skyris 236C", "Skyris 236C", nullptr, 2.8f, -1, 0, 0, false }, { "Skyris 236M", "Skyris 236M", nullptr, 2.8f, -1, 0, 0, false }, - { "iOptron iPolar", "iOptron iPolar: iOptron iPolar", "iOptron iPolar", 3.75f, -1, 0, 0, true }, - { "iOptron iPolar", "iOptron iPolar", "iOptron iPolar", 3.75f, -1, 0, 0, true }, + { "iOptron iPolar", "iOptron iPolar: iOptron iPolar", "iOptron iPolar", 3.75f, -1, 1280, 960, true }, + { "iOptron iPolar", "iOptron iPolar", "iOptron iPolar", 3.75f, -1, 1280, 960, true }, + { "iOptron iPolar", "iOptron iPolar 1.1: iOptron iPo", "iOptron iPolar", 3.75f, -1, 1280, 960, true }, { "iOptron iGuider", "iOptron iGuider: iOptron iGuide", "iOptron iGuider", 3.75f, -1, 1280, 960, true }, { "iOptron iGuider", "iOptron iGuider 1", "iOptron iGuider", 3.75f, -1, 1280, 960, true }, { "iOptron iGuider", "iOptron iGuider External: iOptr", "iOptron iGuider", 3.75f, -1, 1280, 960, true }, From 6409e235121bec6af1ea44853b08746730f36d1c Mon Sep 17 00:00:00 2001 From: Jasem Mutlaq Date: Thu, 7 Mar 2024 16:21:03 +0300 Subject: [PATCH 3/4] Add missing iPolar label --- drivers/video/indi_v4l2driver.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/video/indi_v4l2driver.cpp b/drivers/video/indi_v4l2driver.cpp index daa71bd374..c5d0c1ba1d 100644 --- a/drivers/video/indi_v4l2driver.cpp +++ b/drivers/video/indi_v4l2driver.cpp @@ -45,6 +45,7 @@ static class Loader {"Skyris 236M", "Skyris 236M"}, {"iOptron iPolar: iOptron iPolar", "iOptron iPolar"}, {"iOptron iPolar", "iOptron iPolar"}, + {"iOptron iPolar 1.1: iOptron iPo", "iOptron iPolar"}, {"iOptron iGuider: iOptron iGuide", "iOptron iGuider"}, {"iOptron iGuider 1", "iOptron iGuider"}, {"iOptron iGuider External: iOptr", "iOptron iGuider"}, From f5b4ea40bc3fb7317f84b54725193181b56f314f Mon Sep 17 00:00:00 2001 From: Jasem Mutlaq Date: Thu, 7 Mar 2024 21:20:39 +0300 Subject: [PATCH 4/4] Fix issues with iPolar capture --- drivers/video/v4l2driver.cpp | 47 +++++++++++++++--------------------- 1 file changed, 20 insertions(+), 27 deletions(-) diff --git a/drivers/video/v4l2driver.cpp b/drivers/video/v4l2driver.cpp index 2a6d9a3154..782f67f1dc 100644 --- a/drivers/video/v4l2driver.cpp +++ b/drivers/video/v4l2driver.cpp @@ -58,11 +58,12 @@ static const PixelSizeInfo CameraDatabase[] = }; static const double IOptronDurations[] = { 0.01, 0.02, 0.05, 0.1, 0.2, 0.5, 1.0, 1.5, 2.0, 2.5, 3.0, 3.5, -1 }; -static const double IOptronTicks[] = { 1, 2, 5, 10, 20, 39, 78, 156, 312, 625, 1250, 2500, -1 }; +static const double iGuiderTicks[] = { 1, 2, 5, 10, 20, 39, 78, 156, 312, 625, 1250, 2500, -1 }; +static const double iPolarTicks[] = { 13, 26, 66, 133, 266, 666, 1333, 1999, 2666, 3332, 3999, 4665, -1 }; #define MAX_IOPTRON_INDEX (sizeof(IOptronDurations) / sizeof(double) - 2) #define MAX_IOPTRON_DURATION (IOptronDurations[MAX_IOPTRON_INDEX]) -#define MAX_IOPTRON_TICKS (IOptronTicks[MAX_IOPTRON_INDEX]) +#define MAX_IOPTRON_TICKS (iGuiderTicks[MAX_IOPTRON_INDEX]) #define IOPTRON_ABS_EXPOSURE_CTRL_ID 0x9A0902 #define IOPTRON_AUTO_EXPOSURE_CTRL_ID 0x9A0901 @@ -816,8 +817,13 @@ bool V4L2_Driver::ISNewNumber(const char * dev, const char * name, double values // iOptron cameras only accept some tick values. Round the value to the nearest one. if (isIOptron() && ctrl_id == IOPTRON_ABS_EXPOSURE_CTRL_ID) { - int n = findIndexOfNearestValue(ImageAdjustNP.np[i].value, IOptronTicks); - ImageAdjustNP.np[i].value = IOptronTicks[n]; + int n = 0; + if (strstr(getDeviceName(), "iGuider")) + n = findIndexOfNearestValue(ImageAdjustNP.np[i].value, iGuiderTicks); + else + n = findIndexOfNearestValue(ImageAdjustNP.np[i].value, iPolarTicks); + + ImageAdjustNP.np[i].value = iGuiderTicks[n]; } const double value = ImageAdjustNP.np[i].value; @@ -1007,7 +1013,10 @@ bool V4L2_Driver::setManualExposure(double duration) { if (duration <= MAX_IOPTRON_DURATION) { - ticks = IOptronTicks[findIndexOfNearestValue(duration, IOptronDurations)]; + if (strstr(getDeviceName(), "iGuider")) + ticks = iGuiderTicks[findIndexOfNearestValue(duration, IOptronDurations)]; + else + ticks = iPolarTicks[findIndexOfNearestValue(duration, IOptronDurations)]; } else { @@ -1205,22 +1214,15 @@ void V4L2_Driver::stdtimerCallback(void * userpointer) // iPolar and iGuider cameras occasionally fail to restart once capturing is stopped. This occurs in Windows too. Here // we take the same approach as the iOptron ASCOM driver and restart the capture if no frames have been received for a while. +// JM 2024.03.07: Above approach can end in infinite capture, so we elect to abort immediately so that client can take action. void V4L2_Driver::iOptronWatchdogCallback() { + ioptron_watchdog_timer.stop(); if (valid_frame_has_arrived) - { - ioptron_watchdog_timer.stop(); return; - } - - LOG_DEBUG("Valid frame hasn't arrived yet. Restart capturing."); - - char errmsg[ERRMSGSIZ]; - v4l_base->stop_capturing(errmsg); - - v4l_base->setINTControl(IOPTRON_ABS_EXPOSURE_CTRL_ID, AbsExposureN->value, errmsg); - v4l_base->setOPTControl(IOPTRON_AUTO_EXPOSURE_CTRL_ID, 1, errmsg); // Manual - v4l_base->start_capturing(errmsg); + + stop_capturing(); + PrimaryCCD.setExposureFailed(); } bool V4L2_Driver::start_capturing(bool do_stream) @@ -1560,15 +1562,6 @@ void V4L2_Driver::newFrame() return; } - // Since we don't clear the frame buffer when changing exposures, we can get several frames coming through with the old - // duration. Discard these. - float current_frame_duration_f = (float) current_frame_duration.tv_sec + (float) current_frame_duration.tv_usec / 1000000.0f; - if( m_StackMode == STACK_NONE && (current_frame_duration_f < expected_frame_duration * 0.9 || current_frame_duration_f > expected_frame_duration * 1.1)) - { - LOGF_DEBUG("Skip frame with incorrect duration (%f seconds)", current_frame_duration_f); - return; - } - valid_frame_has_arrived = true; if (ioptron_watchdog_timer.isActive()) { @@ -1801,7 +1794,7 @@ void V4L2_Driver::newFrame() /* If we arrive here, PrimaryCCD is not exposing anymore, we can't forward the frame and we can't be aborted neither, thus abort the exposure right now. * That issue can be reproduced when clicking the "Set" button on the "Main Control" tab while an exposure is running. * Note that the patch in StartExposure returning busy instead of error prevents the flow from coming here, so now it's only a safeguard. */ - IDLog("%s: frame received while not exposing, force-aborting capture\n", __FUNCTION__); + LOGF_WARN("%s: frame received while not exposing, force-aborting capture", __FUNCTION__); AbortExposure(); } }