Consolidate stimulus sequence verification logic #530
Conversation
|
@cjsha I have updated this PR, if you could take a look at it again and see if the behavior now makes sense, that would be great! |
There was a problem hiding this comment.
|
@cjsha I wanted to confirm if you've pulled down the recent changes on this branch, as I did replace the specific |
|
ahhh you're right, I don't know how this happened. I was looking at the wrong commit in github >.< |
There was a problem hiding this comment.
The behavior is good. The two remaining comments are nit picks. One of them involves adding a detail in the messageBox dialog, and the other is just me struggling to understand some logic (though the logic seems to give the desired UI behavior). I'll just approve now so you can just decide yourself if you think it's worth acting on either of these two comments
- Add a check when closing the Headstage64 dialog - Move existing logic to base class - Ensure that the dialogs can be closed and the contents discarded correctly, allowing for one device to save parameters and another device to discard parameters where appropriate
This PR moves the stimulus sequence verification logic from the inherited classes to the base class, to increase maintainability. It also adds the verification check when closing the Headstage64 dialog, which confirms that both the Electrical and Optical stimulus sequences are valid before continuing.
Fixes #528