Add minimal support for domes into POCS. #248
Conversation
Tests TBS.
Codecov Report
@@ Coverage Diff @@
## develop #248 +/- ##
===========================================
- Coverage 79.8% 79.41% -0.39%
===========================================
Files 42 42
Lines 3204 3231 +27
Branches 415 425 +10
===========================================
+ Hits 2557 2566 +9
- Misses 503 515 +12
- Partials 144 150 +6
Continue to review full report at Codecov.
|
pocs/core.py
Outdated
| @@ -41,6 +41,7 @@ class POCS(PanStateMachine, PanBase): | |||
| Attributes: | |||
| name (str): Name of PANOPTES unit | |||
| next_state (str): The next state for the state machine | |||
| TODO(jamessynge): Why document this here? It is an attribute of the base class. | |||
There was a problem hiding this comment.
At one point I was going to document all the attributes of PanBase and PanStateMachine here so this is the only place you would have to look. That never happened and isn't a good idea, so this is a remnant.
There was a problem hiding this comment.
Removed next_state from docstring.
pocs/core.py
Outdated
| if not self.observatory.close_dome(): | ||
| self.logger.critical('Unable to close dome!') | ||
|
|
||
| # TODO(jamessynge): Determine why parking is conditional. Is self.park() idempotent? |
There was a problem hiding this comment.
If you try to park from a state that doesn't support the transition it will complain. The transitions library has since added support for nested states as well as other features that could clean this up, but we should discuss in another PR.
There was a problem hiding this comment.
Got it. I've been wondering about having separate state machines for the different equipment, since it doesn't make sense to continue expanding the base state machine.
| @@ -223,6 +224,9 @@ def power_down(self): | |||
| "Shutting down {}, please be patient and allow for exit.".format( | |||
| self.name)) | |||
|
|
|||
| if not self.observatory.close_dome(): | |||
| self.logger.critical('Unable to close dome!') | |||
There was a problem hiding this comment.
This is perhaps one of those places where we need to somehow add email support.
There was a problem hiding this comment.
Makes sense. Perhaps we should extend the idea of the "say" method to include a "yell" method, designed to ask for help.
pocs/core.py
Outdated
| @@ -488,6 +498,8 @@ def _check_messages(self, queue_type, q): | |||
| def _interrupt_and_park(self): | |||
| self.logger.info('Park interrupt received') | |||
| self._interrupted = True | |||
| # TODO(jamessynge): Should we close the dome here? Or extend the state machine | |||
| # with dome related states and actions? | |||
There was a problem hiding this comment.
This will call park from any state that it is currently in and move to the parking state. I think it makes most sense to close the dome in the parking state itself and leave this as is.
There was a problem hiding this comment.
OK, I'll move it, but I'm concerned that POCS.power_down() assumes that the only device that needs to be dealt with for parking is the mount, and so makes decisions about whether to change state based on whether the mount is connected. @wtgee What do you suggest I do about that?
pocs/core.py
Outdated
| @@ -461,6 +469,8 @@ def _check_environment(self): | |||
| os.makedirs("{}/logs".format(pandir)) | |||
|
|
|||
| def _check_messages(self, queue_type, q): | |||
| # TODO(jamessynge): Where do these messages come from? PAWS? | |||
| # TODO(jamessynge): Determine what to do about the dome. | |||
There was a problem hiding this comment.
Regarding comment below, I don't think this needs to be changed.
There was a problem hiding this comment.
Do the messages come from PAWS?
Responding to feedback.
Basically just support for closing the dome when done. Still need to figure out how to integrate the dome into the approach state transition. Advice sought.
Tests TBS.
Partially address #199.