diff --git a/finesse/hardware/data_file_writer.py b/finesse/hardware/data_file_writer.py index 2d7adac7..d0315ad0 100644 --- a/finesse/hardware/data_file_writer.py +++ b/finesse/hardware/data_file_writer.py @@ -68,8 +68,8 @@ def _get_stepper_motor_angle() -> tuple[float, bool]: """Get the current angle of the stepper motor. This function returns a float indicating the angle in degrees and a boolean - indicating whether the motor is currently moving. If an error occurs or the motor is - moving, the angle returned will be nan. + indicating whether the motor is currently moving. If an error occurs, the angle + returned will be nan. """ stepper = get_stepper_motor_instance() @@ -79,10 +79,8 @@ def _get_stepper_motor_angle() -> tuple[float, bool]: try: angle = stepper.angle - if angle is None: - return (float("nan"), True) - else: - return (angle, False) + is_moving = stepper.is_moving + return (angle, is_moving) except Exception as error: stepper.send_error_message(error) return (float("nan"), False) diff --git a/finesse/hardware/plugins/stepper_motor/st10_controller.py b/finesse/hardware/plugins/stepper_motor/st10_controller.py index 79aa228e..f90527a2 100644 --- a/finesse/hardware/plugins/stepper_motor/st10_controller.py +++ b/finesse/hardware/plugins/stepper_motor/st10_controller.py @@ -335,21 +335,19 @@ def is_moving(self) -> bool: return self.status_code & 0x0010 == 0x0010 @property - def step(self) -> int | None: + def step(self) -> int: """The current state of the device's step counter. - As this can only be requested when the motor is stationary, if the motor is - moving then None will be returned. + This makes use of the "IP" command, which estimates the immediate position of + the motor. If the motor is moving, this is an estimated (calculated trajectory) + position. If the motor is stationary, this is the actual position. Raises: SerialException: Error communicating with device SerialTimeoutException: Timed out waiting for response from device ST10ControllerError: Malformed message received from device """ - if self.is_moving: - return None - - step = self._request_value("SP") + step = self._request_value("IP") try: return int(step) except ValueError: diff --git a/finesse/hardware/plugins/stepper_motor/stepper_motor_base.py b/finesse/hardware/plugins/stepper_motor/stepper_motor_base.py index d8982770..2a72b6d3 100644 --- a/finesse/hardware/plugins/stepper_motor/stepper_motor_base.py +++ b/finesse/hardware/plugins/stepper_motor/stepper_motor_base.py @@ -41,12 +41,8 @@ def steps_per_rotation(self) -> int: @property @abstractmethod - def step(self) -> int | None: - """The current state of the device's step counter. - - As this can only be requested when the motor is stationary, if the motor is - moving then None will be returned. - """ + def step(self) -> int: + """The current state of the device's step counter.""" @step.setter @abstractmethod @@ -67,20 +63,13 @@ def is_moving(self) -> bool: """Whether the motor is currently moving.""" @property - def angle(self) -> float | None: + def angle(self) -> float: """The current angle of the motor in degrees. - As this can only be requested when the motor is stationary, if the motor is - moving then None will be returned. - Returns: - The current angle or None if the stepper motor is moving + The current angle """ - step = self.step - if step is None: - return None - - return step * 360.0 / self.steps_per_rotation + return self.step * 360.0 / self.steps_per_rotation def move_to(self, target: float | str) -> None: """Move the motor to a specified rotation and send message when complete. diff --git a/tests/hardware/plugins/stepper_motor/test_st10_controller.py b/tests/hardware/plugins/stepper_motor/test_st10_controller.py index cf8dac9d..f382bf0e 100644 --- a/tests/hardware/plugins/stepper_motor/test_st10_controller.py +++ b/tests/hardware/plugins/stepper_motor/test_st10_controller.py @@ -240,7 +240,7 @@ def test_write_check(dev: ST10Controller) -> None: if response.startswith(f"{name}=") else pytest.raises(ST10ControllerError), ) - for name in ["hello", "IS", "SP"] + for name in ["hello", "IS", "IP"] for value in ["", "value", "123"] for response in [f"{name}={value}", value, "%", "*", "?4"] ], @@ -357,41 +357,22 @@ def test_is_moving( @pytest.mark.parametrize( "step,response,raises", chain( - [(step, f"SP={step}", does_not_raise()) for step in range(0, 250, 50)], - [(4, "SP=hello", pytest.raises(ST10ControllerError))], + [(step, f"IP={step}", does_not_raise()) for step in range(0, 250, 50)], + [(4, "IP=hello", pytest.raises(ST10ControllerError))], ), ) -@patch( - "finesse.hardware.plugins.stepper_motor.st10_controller.ST10Controller.is_moving", - new_callable=PropertyMock, -) -def test_get_step_not_moving( - is_moving_mock: PropertyMock, +def test_get_step( step: int, response: str, raises: Any, dev: ST10Controller, ) -> None: - """Test getting the step property when the motor is stationary.""" - is_moving_mock.return_value = False + """Test getting the step property.""" with read_mock(dev, response): with raises: assert dev.step == step -@patch( - "finesse.hardware.plugins.stepper_motor.st10_controller.ST10Controller.is_moving", - new_callable=PropertyMock, -) -def test_get_step_moving( - is_moving_mock: PropertyMock, - dev: ST10Controller, -) -> None: - """Test getting the step property when the motor is moving.""" - is_moving_mock.return_value = True - assert dev.step is None - - @pytest.mark.parametrize("step", range(0, 40, 7)) def test_set_step(dev: ST10Controller, step: int) -> None: """Test setting the step property.""" diff --git a/tests/hardware/plugins/stepper_motor/test_stepper_motor_base.py b/tests/hardware/plugins/stepper_motor/test_stepper_motor_base.py index c113eeb9..60b685bd 100644 --- a/tests/hardware/plugins/stepper_motor/test_stepper_motor_base.py +++ b/tests/hardware/plugins/stepper_motor/test_stepper_motor_base.py @@ -22,7 +22,7 @@ def is_moving(self) -> bool: return False @property - def step(self) -> int | None: + def step(self) -> int: return self._step @step.setter diff --git a/tests/hardware/test_data_file_writer.py b/tests/hardware/test_data_file_writer.py index bee7a5e5..c4fad9da 100644 --- a/tests/hardware/test_data_file_writer.py +++ b/tests/hardware/test_data_file_writer.py @@ -129,6 +129,7 @@ def test_write( """Test the write() method.""" get_stepper_mock.return_value = stepper = MagicMock() stepper.angle = 90.0 + stepper.is_moving = False get_tc_mock.return_value = hot_bb = MagicMock() hot_bb.power = 10 @@ -144,6 +145,33 @@ def test_write( sendmsg_mock.assert_called_once_with("data_file.writing") +@patch("finesse.hardware.data_file_writer.get_temperature_controller_instance") +@patch("finesse.hardware.data_file_writer.get_stepper_motor_instance") +def test_write_moving( + get_stepper_mock: Mock, + get_tc_mock: Mock, + writer: DataFileWriter, + sendmsg_mock: Mock, +) -> None: + """Test the write() method when the stepper motor is moving.""" + get_stepper_mock.return_value = stepper = MagicMock() + stepper.angle = 95.0 + stepper.is_moving = True + get_tc_mock.return_value = hot_bb = MagicMock() + hot_bb.power = 10 + + time = datetime(2023, 4, 14, 0, 1, 0) # one minute past midnight + data = [Decimal(i) for i in range(3)] + + writer._writer = MagicMock() + writer.write(time, data) + writer._writer.writerow.assert_called_once_with( + ("20230414", "00:01:00", *data, 60, 95.0, True, 10) + ) + + sendmsg_mock.assert_called_once_with("data_file.writing") + + @patch("finesse.hardware.data_file_writer.get_temperature_controller_instance") @patch("finesse.hardware.data_file_writer.get_stepper_motor_instance") def test_write_error( @@ -155,6 +183,7 @@ def test_write_error( """Test the write() method when an error occurs.""" get_stepper_mock.return_value = stepper = MagicMock() stepper.angle = 90.0 + stepper.angle = False get_tc_mock.return_value = hot_bb = MagicMock() hot_bb.power = 10