Add dummy stepper motor delay in default configuration #681
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #681 +/- ##
==========================================
- Coverage 84.99% 84.97% -0.03%
==========================================
Files 69 69
Lines 3459 3461 +2
==========================================
+ Hits 2940 2941 +1
- Misses 519 520 +1 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Ahh, this makes way more sense. I did see the dummy stepper motor device has a timer that makes use of move_duration, and the init docstring says "This class uses a simple timer to notify when the move is complete after a fixed amount of time." - I just assumed it was faster than I could click 😅
| if move_duration < 0.0: | ||
| raise ValueError("move_duration cannot be negative") |
There was a problem hiding this comment.
I'd restrict to a positive number, as it should not be possible to have this duration as zero.
| if move_duration < 0.0: | |
| raise ValueError("move_duration cannot be negative") | |
| if move_duration <= 0.0: | |
| raise ValueError("move_duration needs to be strictly positive") |
There was a problem hiding this comment.
It is a bit weird, but it does sort of make sense to have it as an option for e.g. unit tests. We don't want to be waiting unnecessarily there. Instantaneous movement shouldn't actually break anything.
| @@ -3,6 +3,8 @@ name: "Dummy devices + FTSW500 spectrometer" | |||
| devices: | |||
| stepper_motor: | |||
| class_name: stepper_motor.dummy.DummyStepperMotor | |||
| params: | |||
There was a problem hiding this comment.
Do we need steps per rotation, here?
There was a problem hiding this comment.
If we miss it out, the default will be used instead, which should be fine.
Description
It turns out that while the
DummyStepperMotorhas the facility to add a delay between starting and stopping a movement, we're not actually using this facility anywhere, for some reason, which is a bit of an oversight. The parameter wasn't changeable via the frontend either. Let's fix this, as it's useful for testing things like @dc2917's work on #680.