Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tracking error fixes #549

Merged
merged 8 commits into from
Aug 19, 2018
Merged

Tracking error fixes #549

merged 8 commits into from
Aug 19, 2018

Conversation

wtgee
Copy link
Member

@wtgee wtgee commented Aug 18, 2018

This fixes an error with the direction of the declination tracking adjustment. Also moves the logic into the mount itself.

wtgee added 4 commits August 18, 2018 20:15
* Fixing direction depending on where pier is pointed.
* Reset Dec correction panoptes#349
* Default guide rate (tracking adjustment speed) increased
* Move tracking correction logic into mount class
* Cleanup code
* Add basic test
@wtgee wtgee requested a review from jamessynge August 18, 2018 11:52
@codecov
Copy link

codecov bot commented Aug 18, 2018

Codecov Report

Merging #549 into develop will increase coverage by 0.14%.
The diff coverage is 70.37%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #549      +/-   ##
===========================================
+ Coverage    70.31%   70.46%   +0.14%     
===========================================
  Files           64       64              
  Lines         5484     5498      +14     
  Branches       762      762              
===========================================
+ Hits          3856     3874      +18     
- Misses        1417     1421       +4     
+ Partials       211      203       -8
Impacted Files Coverage Δ
pocs/images.py 92.52% <ø> (ø) ⬆️
pocs/mount/mount.py 58.73% <69.76%> (+1.71%) ⬆️
pocs/observatory.py 86.05% <72.72%> (+4.45%) ⬆️
pocs/state/states/default/analyzing.py 43.75% <0%> (-12.5%) ⬇️
pocs/state/states/default/scheduling.py 60.86% <0%> (-8.7%) ⬇️
pocs/utils/images/fits.py 80.68% <0%> (-1.71%) ⬇️
pocs/state/states/default/tracking.py 83.33% <0%> (ø) ⬆️
pocs/serial_handlers/protocol_arduinosimulator.py 75.76% <0%> (+0.76%) ⬆️
pocs/camera/camera.py 90.81% <0%> (+1.08%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0848bed...712a94d. Read the comment docs.

u.arcsec), d_dec.to(
u.arcsec), mag.to(
u.arcsec))
d_ra.to(u.arcsec),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I read this as just a formatting change, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct. It was just annoying me too much as I was trying to understand it.

@@ -342,23 +342,49 @@ def update_tracking(self):
if self.current_offset_info is not None:
self.logger.debug("Updating the tracking")

# Get the pier side of pointing image
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add some more comments? I'm certainly ignorant about what is happening here, and the lack of problem description in the PR isn't helping.

@@ -72,8 +72,8 @@ def __init__(self, location, commands=None, *args, **kwargs
self._state = 'Parked'

self.sidereal_rate = ((360 * u.degree).to(u.arcsec) / (86164 * u.second))
self.ra_guide_rate = 0.5 # Sidereal
self.dec_guide_rate = 0.5 # Sidereal
self.ra_guide_rate = 0.9 # Sidereal
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does guide rate mean the speed with which we make adjustments to the position between images?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is documented somewhere but the fact that I don't know where and that you have a question here makes that irrelevant.

I want to turn these into properties of the class, which would be more consistent with all these other values and would allow for an easier docstring. - #552

Args:
offset_info (`OffsetError`): A named tuple describing the offset
error. See `pocs.images.OffsetError`.
pointing_ha (float): The Hour Angle (HA) of the mount at the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the point here that the mount will do a meridian flip to go from 89 degrees to 91 degrees (90 being pointing to the meridian)? And thus the HA will tell us if we started observing with the cameras on the left or right side of the pier?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added some more info in the docstring.

pointing_ha
)

try:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it worth putting in a limit, where we don't make a correction if the error is sufficiently low?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It happens back in get_tracking_correction where it checks for anything under 50 ms. Nothing goes into the returned dict so the correct_tracking method then just skips that whole axis.

The 50 ms was determined empirically by me where I noticed when it was shorter than about 40ms it would just happen too quickly and then the system would get stuck waiting. It's also less than 1 pix of movement.

I'll make a note in the docstring to get_tracking_correction about how the values are clipped for min max.

@wtgee wtgee merged commit 709d7f7 into panoptes:develop Aug 19, 2018
@wtgee wtgee deleted the tracking-error-fixes branch August 19, 2018 23:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants