Calculating current HA corrected so that -12 < HA <= 12
AbandonedPublic

Authored by wreissenberger on Aug 5 2019, 9:00 PM.

Details

Summary

Bug fix for false meridian flip. This happens, since the calculation of the current HA has a weakness not ensuring that -12 < HA <= 12. For targets with small RA value like M31 a summer night with an LST of 19:00 leads to an uncorrected HA value of approx. 18:45 - which is equivalent to a corrected value of -5:15.

Test Plan

Set the current time to an August night and slew to M31. Without this fix, the Mount tab shows "Meridian flip planned" preventing preview capturing in the Capture module.

Diff Detail

Repository
R321 KStars
Branch
bug_HA_calculation
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 14779
Build 14797: arc lint + arc unit
wreissenberger created this revision.Aug 5 2019, 9:00 PM
Restricted Application added a project: KDE Edu. · View Herald TranscriptAug 5 2019, 9:00 PM
Restricted Application added a subscriber: kde-edu. · View Herald Transcript
wreissenberger requested review of this revision.Aug 5 2019, 9:00 PM
mutlaqja accepted this revision.Aug 6 2019, 6:47 AM

I prefer if we start using the functions in indicom.

rangeHA
range24
range360
rangeDec

They're very useful in KStars.

So just add #include <indicom.h>

and then

deltaHA = rangeHA(deltaHA);

This revision is now accepted and ready to land.Aug 6 2019, 6:47 AM
TallFurryMan accepted this revision.Aug 6 2019, 7:31 AM

I'd suggest using std::fmod.

mutlaqja requested changes to this revision.Aug 6 2019, 10:23 AM

Please use the <indicom.h> functions stated above.

This revision now requires changes to proceed.Aug 6 2019, 10:23 AM

Ok I found more places where this needs to be done, so I'll do it.

mutlaqja resigned from this revision.Aug 6 2019, 12:37 PM
This revision is now accepted and ready to land.Aug 6 2019, 12:37 PM

Jasem, many thanks for the corrections, I wasn't aware of rangeHA. A small hint: is there a reason why you left the old code commented out in the code instead of simply deleting it?

wreissenberger abandoned this revision.Sep 2 2019, 7:50 AM

Resolved.