FIX: PyDMSlider out of bounds bug #1084
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…es and retest other functionality
… step size after calc
| @@ -538,7 +543,9 @@ def calc_step_size(self): | |||
| Given max, min, and num steps calculate step size | |||
| """ | |||
| self.step_size = (self.maximum - self.minimum) / self.num_steps | |||
There was a problem hiding this comment.
should we check for 0-division here?
could self.num_steps be 0 here if self.maximum == self.minimum?
Line 1020 in 1b896f5
There was a problem hiding this comment.
Yeah, it could. I'll put another check there.
YektaY
changed the title
nstelter-slac
approved these changes
Jul 1, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The PyDMSlider has a bug(s) when the channel it is connected to is a certain type of PV (epics record). If the record is an analog out (ao), and it does not have the record fields DRVH and DRVL set, then the slider is not forced to be within the bounds of the minimum and maximum properties. Since the default values of DRVH and DRVL are both zero, the slider wouldn't have a range of allowed values outside of "0".
This can cause two bugs. The first was calculated step size becomes 0, which would essentially form a never ending while loop of allowed positions when the slider value was out of bounds. The second bug is PV values could be out of the slider bounds.
These bugs only occur when both DRVH and DRVL were at default values. If either was set it would not occur. This PR has code to catch these edges and then raise value errors since we neither want a unending loop or a slider with values existing out of bounds.