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

Wait for tracking adjustment #332

Merged
merged 7 commits into from
Jan 15, 2018

Conversation

wtgee
Copy link
Member

@wtgee wtgee commented Jan 13, 2018

The tracking adjustment for ioptron uses move_ms_<direction>, which places
the mount in guide mode (as indicated by the status) but is non-blocking.
If the tracking adjustment is too long (~ > 4-5 seconds) and no blocking
is done the machine will try to transition to next state. There is a
condition on the state machine that the mount must be in tracking
to observe so it gets stuck and then tries again. After 5 iterations
of same state without successful transition the machine will give up
and park the mount.

Here we just wait out for up to 30 seconds until back in tracking
mode.

Note: Ideally we should never have to have an adjustment of 4-5 seconds,
so not entirely sure what is going on there. Seems to happen most on
first adjustment after pointing but also depends on where mount is poining.
This is a problem with PAN001 that we have had and might not be an issue
on other units.

The tracking adjustment for ioptron uses `move_ms_<dir>`, which places
the mount in `guide` mode (as indicated by the status) but is non-blocking.
If the tracking adjustment is too long (~ > 4-5 seconds) and no blocking
is done the machine will try to transition to next state. There is a
condition on the state machine that the mount must be in `tracking`
to `observe` so it gets stuck and then tries again. After 5 iterations
of same state without successful transition the machine will give up
and park the mount.

Here we just wait out for up to 30 seconds until back in `tracking`
mode.

Note: Ideally we should never have to have an adjustment of 4-5 seconds,
so not entirely sure what is going on there. Seems to happen most on
first adjustment after pointing but also depends on where mount is poining.
This is a problem with PAN001 that we have had and might not be an issue
on other units.
@wtgee wtgee requested a review from a team January 13, 2018 21:10
@codecov
Copy link

codecov bot commented Jan 14, 2018

Codecov Report

Merging #332 into develop will decrease coverage by 0.3%.
The diff coverage is 69.56%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #332      +/-   ##
===========================================
- Coverage    83.81%   83.51%   -0.31%     
===========================================
  Files           51       51              
  Lines         3368     3409      +41     
  Branches       422      432      +10     
===========================================
+ Hits          2823     2847      +24     
- Misses         404      415      +11     
- Partials       141      147       +6
Impacted Files Coverage Δ
pocs/mount/mount.py 57.01% <ø> (ø) ⬆️
pocs/mount/simulator.py 92.79% <100%> (+0.26%) ⬆️
pocs/observatory.py 85.23% <44.44%> (-0.98%) ⬇️
pocs/state/states/default/tracking.py 83.33% <80%> (-16.67%) ⬇️
pocs/dome/astrohaven.py 70.47% <0%> (-8.2%) ⬇️
pocs/state/states/default/pointing.py 87.75% <0%> (ø) ⬆️
pocs/dome/protocol_astrohaven_simulator.py 77.63% <0%> (+1.68%) ⬆️

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 21e5408...491b184. Read the comment docs.

@wtgee
Copy link
Member Author

wtgee commented Jan 14, 2018

Figuring out why this dropped covered so much. DO NOT MERGE yet.]

Fixed major drop. Would still be nice to get some more in the tracking state but should be part of a better mount simulator.

wtgee added 3 commits January 15, 2018 08:30
* Mount simulator will always report a fake offset
* Fixing call to `get_ms_offset` so it correctly uses axis (has no practical effect on ioptrons, which don't allow for differing rates between the axis)
* Renamed `test_run` -> `test_run_complete` so easier to run in isolation
* Fixing docstrings
Copy link
Contributor

@jamessynge jamessynge left a comment

Choose a reason for hiding this comment

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

Mostly LGTM, but please respond to feedback.

@@ -501,10 +501,10 @@ def get_ms_offset(self, offset, axis='ra'):
""" Get offset in milliseconds at current speed

Args:
offset (float): Offset in arcseconds
offset (astropy.units.Angle): Offset in arcseconds
Copy link
Contributor

Choose a reason for hiding this comment

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

Support for using type annotations. :-)

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually the astropy units was one of the things I was concerned about. I'm hoping it works seamlessly for them.

offset (astropy.units.Angle): Offset in arcseconds

Returns:
astropy.units.Second: Offset in milliseconds at current speed
Copy link
Contributor

Choose a reason for hiding this comment

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

How does the type (Second) match with the comment saying it is milliseconds?

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually this should be aQuantity anyway. I'll get it changed.

pocs.next_state = 'parking'

# If we came from pointing then don't try to adjust
if event_data.transition.source != 'pointing':
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm confused about pointing vs tracking vs observing w.r.t. moving / adjusting the mount.

Copy link
Member Author

Choose a reason for hiding this comment

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

In what sense? For what the mount reports or for the actual state? Maybe some documentation of the states will eventually help with this.

if event_data.transition.source != 'pointing':
pocs.say("Checking our tracking")
try:
ra_info, dec_info = pocs.observatory.update_tracking()
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like we should move more of this code into POCS or Observatory. i.e. these on_enter functions should have little logic of their own. Thoughts?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, good point. Doing that now.

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've done this but removed the say output that says what correction was. It's been useful for me the last few days to watch it but it's also a little too much information for the say box. Ideally we should be plotting this in real-time within PAWS. The info is delivered via the status update so information is there.

This was mostly because I've been keeping any say calls exclusively in the states (except for the setup in core.py). Not sure if that is necessary or not.

* Move wait on tracking adjustment to inside the method
* Fix return type docs for ge_ms_offset
@wtgee wtgee merged commit 6b0142b into panoptes:develop Jan 15, 2018
@wtgee wtgee deleted the wait-for-tracking-adjustment branch January 15, 2018 00:55
@wtgee wtgee mentioned this pull request Jan 24, 2018
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