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