D29821: Add autofocus on temperature change

Eric Dejouhanet noreply at phabricator.kde.org
Wed May 20 07:01:48 BST 2020


TallFurryMan added a comment.


  Excellent, thanks for this proposal! I have nothing but minor points, looks good to me.

INLINE COMMENTS

> capture.cpp:1948
>          // force refocus
> -        qCDebug(KSTARS_EKOS_CAPTURE) << "Capture is triggering autofocus on line 1904.";
> +        qCDebug(KSTARS_EKOS_CAPTURE) << "Capture is triggering autofocus on line 1948.";
>          emit checkFocus(0.1);

Good spot, wouldn't `__LINE__` solve this problem? :)

> capture.cpp:1956
> +
> +    if (isInSequenceFocus && inSequenceFocusCounter == 0)
>      {

Yes, there was an `else` after a `if-then-return`, but with big blocks of code like this, it is probably better to consider the `if` addresses `isRefocus`, `isInSequenceFocus` and `inSequenceFocusCounter` as a set of three related flags, instead of the things that happen in those blocks. So please keep the `else` for clarity, so that others understand? (minor)

> focus.cpp:584
> +        currentTemperature = focuserTemperature->np[0].value;
> +        qCDebug(KSTARS_EKOS_FOCUS) << QString("Setting current focuser temperature: %1").arg(currentTemperature);
> +    }

You could use numeric precision here for your log.

REPOSITORY
  R321 KStars

REVISION DETAIL
  https://phabricator.kde.org/D29821

To: fsignoret, mutlaqja
Cc: TallFurryMan, kde-edu, narvaez, apol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20200520/7187f580/attachment-0001.htm>


More information about the kde-edu mailing list