Skip to content

Commit

Permalink
feat(hardware): handle motor driver errors (#13867)
Browse files Browse the repository at this point in the history
This PR enables the reporting of Flex motor driver errors, adding the
MotorDriverError exception. Specific motor driver errors are
over-temperature, short circuit, and open circuit. An ErrorMessage is
reported from the Flex firmware in two cases:
1. As a response to a move request when a motor driver is in error
state. In this case, the error is logged.
2. If a motor driver enters error state during a move. In this case, a
subsequent ReadMotorDriverErrorStatusResponse message is reported from
the Flex firmware, which details the specific error. This PR logs these
messages and also raises these messages as MotorDriverError exceptions.

<!--
Thanks for taking the time to open a pull request! Please make sure
you've read the "Opening Pull Requests" section of our Contributing
Guide:


https://github.com/Opentrons/opentrons/blob/edge/CONTRIBUTING.md#opening-pull-requests

To ensure your code is reviewed quickly and thoroughly, please fill out
the sections below to the best of your ability!
-->

# Overview

<!--
Use this section to describe your pull-request at a high level. If the
PR addresses any open issues, please tag the issues here.
-->

# Test Plan

- [ ] Test both use cases on robot. In second use case, ensure
ReadMotorDriverErrorStatusResponse message is associated with requesting
MoveID.
- [x] Write software tests for both use cases. In second use case,
ensure each specific error is parsed.

<!--
Use this section to describe the steps that you took to test your Pull
Request.
If you did not perform any testing provide justification why.

OT-3 Developers: You should default to testing on actual physical
hardware.
Once again, if you did not perform testing against hardware, justify
why.

Note: It can be helpful to write a test plan before doing development

Example Test Plan (HTTP API Change)

- Verified that new optional argument `dance-party` causes the robot to
flash its lights, move the pipettes,
then home.
- Verified that when you omit the `dance-party` option the robot homes
normally
- Added protocol that uses `dance-party` argument to G-Code Testing
Suite
- Ran protocol that did not use `dance-party` argument and everything
was successful
- Added unit tests to validate that changes to pydantic model are
correct

-->

# Changelog

<!--
List out the changes to the code in this PR. Please try your best to
categorize your changes and describe what has changed and why.

Example changelog:
- Fixed app crash when trying to calibrate an illegal pipette
- Added state to API to track pipette usage
- Updated API docs to mention only two pipettes are supported

IMPORTANT: MAKE SURE ANY BREAKING CHANGES ARE PROPERLY COMMUNICATED
-->

# Review requests

<!--
Describe any requests for your reviewers here.
-->

# Risk assessment

Low. Adds MotorDriverError to existing list of exceptions. Introduces
handling and parsing of ReadMotorDriverErrorStatusResponse message,
using existing well-tested frameworks and concepts.

<!--
Carefully go over your pull request and look at the other parts of the
codebase it may affect. Look for the possibility, even if you think it's
small, that your change may affect some other part of the system - for
instance, changing return tip behavior in protocol may also change the
behavior of labware calibration.

Identify the other parts of the system your codebase may affect, so that
in addition to your own review and testing, other people who may not
have the system internalized as much as you can focus their attention
and testing there.
-->
  • Loading branch information
pmoegenburg authored Mar 25, 2024
1 parent 36de306 commit 3094f5e
Show file tree
Hide file tree
Showing 10 changed files with 188 additions and 0 deletions.
4 changes: 4 additions & 0 deletions hardware/opentrons_hardware/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
LabwareDroppedError,
PythonException,
HepaUVFailedError,
MotorDriverError,
)

from opentrons_hardware.firmware_bindings.messages.message_definitions import (
Expand Down Expand Up @@ -138,6 +139,9 @@ def raise_from_error_message( # noqa: C901
message="Unexpected robotics error", detail=detail_dict
)

if error_code in (ErrorCode.motor_driver_error_detected,):
raise MotorDriverError(detail=detail_dict)

raise RoboticsControlError(message="Hardware error", detail=detail_dict)


Expand Down
13 changes: 13 additions & 0 deletions hardware/opentrons_hardware/firmware_bindings/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,8 @@ class MessageId(int, Enum):
write_motor_current_request = 0x33
read_motor_current_request = 0x34
read_motor_current_response = 0x35
read_motor_driver_error_status_request = 0x36
read_motor_driver_error_status_response = 0x37

set_brushed_motor_vref_request = 0x40
set_brushed_motor_pwm_request = 0x41
Expand Down Expand Up @@ -265,6 +267,7 @@ class MessageId(int, Enum):
class ErrorSeverity(int, Enum):
"""Error Severity levels."""

none = 0x0
warning = 0x1
recoverable = 0x2
unrecoverable = 0x3
Expand All @@ -290,6 +293,16 @@ class ErrorCode(int, Enum):
over_pressure = 0x0D
door_open = 0x0E
reed_open = 0x0F
motor_driver_error_detected = 0x10


@unique
class MotorDriverErrorCode(int, Enum):
"""Motor driver error codes."""

over_temperature = 0x2000000
short_circuit = 0x18000000
open_circuit = 0x60000000


@unique
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,26 @@ class ReadMotorDriverResponse(BaseMessage): # noqa: D101
] = MessageId.read_motor_driver_register_response


@dataclass
class ReadMotorDriverErrorStatusRequest(BaseMessage): # noqa: D101
payload: payloads.EmptyPayload
payload_type: Type[payloads.EmptyPayload] = payloads.EmptyPayload
message_id: Literal[
MessageId.read_motor_driver_error_status_request
] = MessageId.read_motor_driver_error_status_request


@dataclass
class ReadMotorDriverErrorStatusResponse(BaseMessage): # noqa: D101
payload: payloads.ReadMotorDriverErrorStatusResponsePayload
payload_type: Type[
payloads.ReadMotorDriverErrorStatusResponsePayload
] = payloads.ReadMotorDriverErrorStatusResponsePayload
message_id: Literal[
MessageId.read_motor_driver_error_status_response
] = MessageId.read_motor_driver_error_status_response


@dataclass
class WriteMotorCurrentRequest(BaseMessage): # noqa: D101
payload: payloads.MotorCurrentPayload
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@
defs.WriteMotorDriverRegister,
defs.ReadMotorDriverRequest,
defs.ReadMotorDriverResponse,
defs.ReadMotorDriverErrorStatusRequest,
defs.ReadMotorDriverErrorStatusResponse,
defs.WriteMotorCurrentRequest,
defs.SetBrushedMotorVrefRequest,
defs.SetBrushedMotorPwmRequest,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,14 @@ class ReadMotorDriverRegisterResponsePayload(EmptyPayload):
data: utils.UInt32Field


@dataclass(eq=False)
class ReadMotorDriverErrorStatusResponsePayload(EmptyPayload):
"""Read motor driver error status response payload."""

reg_addr: utils.UInt8Field
data: utils.UInt32Field


@dataclass(eq=False)
class MotorCurrentPayload(EmptyPayload):
"""Read motor current register payload."""
Expand Down
41 changes: 41 additions & 0 deletions hardware/opentrons_hardware/hardware_control/move_group_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
EStopActivatedError,
MotionFailedError,
PythonException,
MotorDriverError,
)

from opentrons_hardware.firmware_bindings import ArbitrationId
Expand All @@ -22,6 +23,7 @@
ErrorSeverity,
GearMotorId,
MoveAckId,
MotorDriverErrorCode,
SensorId,
)
from opentrons_hardware.drivers.can_bus.can_messenger import CanMessenger
Expand All @@ -39,6 +41,7 @@
TipActionResponse,
ErrorMessage,
StopRequest,
ReadMotorDriverErrorStatusResponse,
AddSensorLinearMoveRequest,
)
from opentrons_hardware.firmware_bindings.messages.payloads import (
Expand Down Expand Up @@ -526,6 +529,42 @@ def _handle_move_completed(
# pick up groups they don't care about, and need to not fail.
pass

def _handle_motor_driver_error(
self, message: ReadMotorDriverErrorStatusResponse, arbitration_id: ArbitrationId
) -> None:
node_id = arbitration_id.parts.originating_node_id
data = message.payload.data.value
if data & MotorDriverErrorCode.over_temperature.value:
log.error(f"Motor driver over-temperature error from node {node_id}")
self._errors.append(
MotorDriverError(
detail={
"node": NodeId(node_id).name,
"error": "over temperature",
}
)
)
if data & MotorDriverErrorCode.short_circuit.value:
log.error(f"Motor driver short circuit error from node {node_id}")
self._errors.append(
MotorDriverError(
detail={
"node": NodeId(node_id).name,
"error": "short circuit",
}
)
)
if data & MotorDriverErrorCode.open_circuit.value:
log.error(f"Motor driver open circuit error from node {node_id}")
self._errors.append(
MotorDriverError(
detail={
"node": NodeId(node_id).name,
"error": "open circuit",
}
)
)

def __call__(
self, message: MessageDefinition, arbitration_id: ArbitrationId
) -> None:
Expand All @@ -539,6 +578,8 @@ def __call__(
self._handle_move_completed(message, arbitration_id)
elif isinstance(message, ErrorMessage):
self._handle_error(message, arbitration_id)
elif isinstance(message, ReadMotorDriverErrorStatusResponse):
self._handle_motor_driver_error(message, arbitration_id)

def _handle_tip_action_motors(self, message: TipActionResponse) -> bool:
gear_id = GearMotorId(message.payload.gear_motor_id.value)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
ErrorSeverity,
PipetteTipActionType,
MoveAckId,
MotorDriverErrorCode,
)
from opentrons_hardware.drivers.can_bus.can_messenger import (
MessageListenerCallback,
Expand All @@ -34,6 +35,7 @@
ExecuteMoveGroupRequestPayload,
HomeRequestPayload,
ErrorMessagePayload,
ReadMotorDriverErrorStatusResponsePayload,
)
from opentrons_hardware.firmware_bindings.messages.fields import (
MotorPositionFlagsField,
Expand Down Expand Up @@ -1418,3 +1420,83 @@ async def test_moves_removed_on_stall_detected(
mock_can_messenger.ensure_send.side_effect = mock_sender.mock_ensure_send
mock_can_messenger.send.side_effect = mock_sender.mock_send
await subject.run(can_messenger=mock_can_messenger)


class MockSendMoveDriverErrorCompleter:
"""Side effect mock of CanMessenger.send that immediately sends an error."""

def __init__(
self,
move_groups: MoveGroups,
listener: MessageListenerCallback,
start_at_index: int = 0,
) -> None:
"""Constructor."""
self._move_groups = move_groups
self._listener = listener
self._start_at_index = start_at_index
self.call_count = 0

@property
def groups(self) -> MoveGroups:
"""Retrieve the groups, for instance from a child class."""
return self._move_groups

async def mock_send(
self,
node_id: NodeId,
message: MessageDefinition,
) -> None:
"""Mock send function."""
if isinstance(message, md.ExecuteMoveGroupRequest):
# Iterate through each move in each sequence and send a move
# completed for it.
payload = EmptyPayload()
payload.message_index = message.payload.message_index
arbitration_id = ArbitrationId(
parts=ArbitrationIdParts(originating_node_id=node_id)
)
self._listener(md.Acknowledgement(payload=payload), arbitration_id)
for seq_id, moves in enumerate(
self._move_groups[message.payload.group_id.value - self._start_at_index]
):
for node, move in moves.items():
assert isinstance(move, MoveGroupSingleAxisStep)
code = MotorDriverErrorCode.over_temperature
payload = ReadMotorDriverErrorStatusResponsePayload(
reg_addr=UInt8Field(111),
data=UInt32Field(code),
)
payload.message_index = message.payload.message_index
arbitration_id = ArbitrationId(
parts=ArbitrationIdParts(originating_node_id=node)
)
self.call_count += 1
self._listener(
md.ReadMotorDriverErrorStatusResponse(payload=payload),
arbitration_id,
)

async def mock_ensure_send(
self,
node_id: NodeId,
message: MessageDefinition,
timeout: float = 3,
expected_nodes: List[NodeId] = [],
) -> ErrorCode:
"""Mock ensure_send function."""
await self.mock_send(node_id, message)
return ErrorCode.ok


async def test_single_move_driver_error(
mock_can_messenger: AsyncMock, move_group_single: MoveGroups
) -> None:
"""It should send a start group command."""
subject = MoveScheduler(move_groups=move_group_single)
mock_sender = MockSendMoveDriverErrorCompleter(move_group_single, subject)
mock_can_messenger.ensure_send.side_effect = mock_sender.mock_ensure_send
mock_can_messenger.send.side_effect = mock_sender.mock_send
with pytest.raises(MotionFailedError):
await subject.run(can_messenger=mock_can_messenger)
assert mock_sender.call_count == 1
4 changes: 4 additions & 0 deletions shared-data/errors/definitions/1/errors.json
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,10 @@
"detail": "Gripper Pickup Failed",
"category": "roboticsControlError"
},
"2016": {
"detail": "Motor Driver Error",
"category": "roboticsControlError"
},
"3000": {
"detail": "A robotics interaction error occurred.",
"category": "roboticsInteractionError"
Expand Down
1 change: 1 addition & 0 deletions shared-data/python/opentrons_shared_data/errors/codes.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ class ErrorCodes(Enum):
POSITION_UNKNOWN = _code_from_dict_entry("2013")
EXECUTION_CANCELLED = _code_from_dict_entry("2014")
FAILED_GRIPPER_PICKUP_ERROR = _code_from_dict_entry("2015")
MOTOR_DRIVER_ERROR = _code_from_dict_entry("2016")
ROBOTICS_INTERACTION_ERROR = _code_from_dict_entry("3000")
LABWARE_DROPPED = _code_from_dict_entry("3001")
LABWARE_NOT_PICKED_UP = _code_from_dict_entry("3002")
Expand Down
13 changes: 13 additions & 0 deletions shared-data/python/opentrons_shared_data/errors/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -598,6 +598,19 @@ def __init__(
super().__init__(ErrorCodes.EXECUTION_CANCELLED, message, detail, wrapping)


class MotorDriverError(RoboticsControlError):
"""An error indicating that a motor driver is in error state."""

def __init__(
self,
message: Optional[str] = None,
detail: Optional[Dict[str, str]] = None,
wrapping: Optional[Sequence[EnumeratedError]] = None,
) -> None:
"""Build a MotorDriverError."""
super().__init__(ErrorCodes.MOTOR_DRIVER_ERROR, message, detail, wrapping)


class LabwareDroppedError(RoboticsInteractionError):
"""An error indicating that the gripper dropped labware it was holding."""

Expand Down

0 comments on commit 3094f5e

Please sign in to comment.