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