D29355: Fix issue with Meridian Flip Retry
Chris Rowland
noreply at phabricator.kde.org
Sat May 2 12:11:04 BST 2020
chrisrowland added a comment.
I can't comment on the change to the way that the capture meridian flip state and the mount meridian flip status state machines interact because there's no description of how the capture state machine works, nor what these changes do and why.
It looks as if some additional signals and slots have been added to pass state information between the modules. This is in addition to the polling that both modules do to monitor and respond to each other's state. Is this mixture a good idea?
Could we have some more description of what this very complex code is intending to achieve? And future developers will thank you if someone could document how the state machines should work.
INLINE COMMENTS
> capture.cpp:3535
> if (m_State == CAPTURE_PAUSED)
> // paused after meridian flip
> secondsLabel->setText(i18n("Paused..."));
Could do with some curly brackets here to improve readability.
> capture.cpp:3542
> meridianFlipStage = stage;
> + emit newMeridianFlipStatus(Mount::FLIP_NONE);
> break;
What does this do and why?
> mount.cpp:1293-1296
> + if (currentTelescope->pierSide() == ISD::Telescope::PierSide::PIER_WEST)
> + flipDelayHrs = ha + delayHours - offset;
> + else
> + flipDelayHrs = rangeHA(ha + 12 + delayHours) - offset;
No issues with doing this, I'd missed that it could take some time before the flip is attempted and fails.
How will this test work if the mount doesn't report pier side? pierside could be PIER_UNKNOWN
From what I can see it will use ha + 12 which is the case for the flip as the mount is crossing the meridian under the pole at about 12h.
Maybe it would be better to check for PIER_EAST and swap the if and else statements.
REPOSITORY
R321 KStars
REVISION DETAIL
https://phabricator.kde.org/D29355
To: murveit, chrisrowland, mutlaqja
Cc: kde-edu, narvaez, apol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20200502/8f3f19f4/attachment.html>
More information about the kde-edu
mailing list