Conversation
* Add route from `parked` to `ready` * Remove route from `housekeeping` to `ready` * Return value for `power_down`
Codecov Report
@@ Coverage Diff @@
## develop #421 +/- ##
===========================================
- Coverage 69.24% 69.21% -0.04%
===========================================
Files 60 60
Lines 4806 4817 +11
Branches 665 670 +5
===========================================
+ Hits 3328 3334 +6
- Misses 1301 1306 +5
Partials 177 177
Continue to review full report at Codecov.
|
jamessynge
left a comment
jamessynge
left a comment
There was a problem hiding this comment.
Can you explain WHY these changes are being made?
| self._connected = False | ||
| self.logger.info("Power down complete") | ||
|
|
||
| return self.connected is False |
There was a problem hiding this comment.
Why return this value. It looks like it can not ever return False.
| # be ready | ||
| assert pocs.goto_next_state() | ||
| assert pocs.state == 'ready' | ||
| assert pocs.power_down() |
There was a problem hiding this comment.
If you remove the POCS.power_down() change, perhaps change this to:
pocs.power_down()
assert not pocs.connected
|
@jamessynge Adjusted the PR message for explanation but also changed a bit of the logic and added a better test, PTAL. |
jamessynge
left a comment
jamessynge
left a comment
There was a problem hiding this comment.
Approving to keep things moving, but concerns raised for the long-term.
| pocs.next_state = 'housekeeping' | ||
| else: | ||
| pocs.say("No observations found.") | ||
| # TODO Should check if we are close to morning and if so do some morning |
There was a problem hiding this comment.
Oh no, I've infected you with adding TODOs! ;-)
I'd love a similar behavior at the start of the evening (i.e. evening flats, or darks while the dome is closed).
|
|
||
| pocs.reset_observing_run() | ||
| pocs.next_state = 'ready' | ||
| # We might have shutdown in long wait |
There was a problem hiding this comment.
"we might have shutdown during that sleep."
| assert pocs_process.is_alive() is False | ||
|
|
||
|
|
||
| def test_pocs_park_to_ready_with_obs(pocs): |
There was a problem hiding this comment.
I'm worried that some of these states are getting too complex. Let's have a further discussion about how to address that (e.g. multiple state machines, either nested or parallel).
There was a problem hiding this comment.
Yes, this made the parked state a bit obtuse with all the if-branching.
2 participants
Change the routing on the shutdown states so that the
housekeepingstate is only entered when actually shutting down for the day. This means the unit can wait in theparkedstate if no immediate observations are found but if otherwise safe. If it's daytime after waiting then go tohousekeeping. If it's bad weather wait another 30 minutes. Otherwise try to observe again.parkedtoreadyhousekeepingtoreadypower_down