Fix shutter position consts and logic for setting shutter pos#63
Fix shutter position consts and logic for setting shutter pos#63
Conversation
GDYendell
left a comment
GDYendell
left a comment
There was a problem hiding this comment.
This seems fine. I assume the reason for this is that the epics record expects an int, not the enum string? Could you add a note saying as much in the commit body? It isn't clear this change is practically any different without the context.
| if shutter_state == SHUTTER_CLOSED: | ||
| self._configure_param({"shutter_closed_position": val}) | ||
|
|
||
| current_shutter_state = "CLOSED" if self.shutter.get() == 0 else "OPEN" |
There was a problem hiding this comment.
@GDYendell The reason I changed it to ints was because of this line. It would then pass a string into _set_shutter() which expects 0, otherwise the else is triggered. So pretty much it would always trigger the else clause as they were strings.
There was a problem hiding this comment.
I don't understand. You haven't changed the conditional, it is still checking self.shutter.get() == 0, you have only changed the assignment to be an int rather than a string, which just changes the value passed to self._set_shutter (which does make sense to me, just the explanation does not).
| SHUTTER_CLOSED if self.shutter.get() == 0 else SHUTTER_OPEN | ||
| ) | ||
| if current_shutter_state == shutter_state: | ||
| self._set_shutter(shutter_state) |
There was a problem hiding this comment.
Isn't this wrong? Shouldn't it only call set if the current value does not match the new value?
There was a problem hiding this comment.
So _set_shutter_pos() only updates the stored position value and doesn't actually move the shutter, _set_shutter() does. This line was added to make sure the shutter updated it's position if in the currently selected state.
There was a problem hiding this comment.
e.g. if in the SHUTTER_CLOSED state, but the shutter_pos_closed is updated from 50 cts to 100 cts, it will then trigger the shutter to move to 100 cts
2 participants
No description provided.