D29821: Add autofocus on temperature change
Eric Dejouhanet <[email protected]> Wed, 20 May 2020 06:01:48 +0000
| Newsgroups | gmane.comp.kde.education |
|---|---|
| Message-ID | <[email protected]> |
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