Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(motion-control): handle motor driver errors #716

Merged
merged 75 commits into from
Mar 25, 2024

Conversation

pmoegenburg
Copy link
Contributor

@pmoegenburg pmoegenburg commented Aug 29, 2023

Overview

This PR enables Flex motor drivers (tmc2130/2160) to report when they are in error state due to over-temperature, short circuit, or open circuit. A driver's diag0 pin will now go active when the driver is in error state, and will trigger an interrupt for the specific motion subsystem that will:
- produce a MotionControllerTask message that will stop the subsystem motion controller
- set a CancelRequest for the motor interrupt handler, which will produce an ErrorMessage on the CAN bus
- spawn a SPI message to retrieve the specific error from the motor driver, which will surface as a ReadMotorDriverErrorStatusResponse message on the CAN bus

This PR also produces an ErrorMessage when a motor driver is in error state and an EnableMotorRequest,
AddLinearMoveRequest, or HomeRequest message is sent to that motion subsystem.

Test Plan

  • Verify pin setup correctly to output low when error state entered
    • Heat driver IC to >150*C (default over-temperature threshold) via heat gun
  • Verify interrupt triggered when pin outputs low
  • Verify subsystem motion controller enters error state when interrupt triggers
  • Verify SPI communication to driver to gather specific error occurs when interrupt triggers
  • Verify CAN message reports this specific error
  • Verify all of this during an actual move

Changelog

Review requests

Risk assessment

This PR introduces the detection of motor driver errors for each motion subsystem. It will effect each motion subsystem in the ways described above, specifically stopping the subsystem's motion controller and motor interrupt handler and producing an ErrorMessage and ReadMotorDriverErrorStatusResponse message when an error is detected.

@pmoegenburg pmoegenburg self-assigned this Aug 29, 2023
@pmoegenburg pmoegenburg marked this pull request as draft August 29, 2023 21:38
@pmoegenburg pmoegenburg changed the title (feat): handle motor driver errors feat(motion-control): handle motor driver errors Aug 30, 2023
@pmoegenburg pmoegenburg marked this pull request as ready for review October 26, 2023 02:34
@pmoegenburg pmoegenburg requested a review from sfoster1 October 26, 2023 04:11
Copy link
Member

@sfoster1 sfoster1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Structure's looking really good, a last couple tweaks and I think we'll be there!

include/gantry/core/interfaces_proto.hpp Outdated Show resolved Hide resolved
include/gantry/core/interfaces_rev1.hpp Outdated Show resolved Hide resolved
gantry/firmware/stm32g4xx_it.c Outdated Show resolved Hide resolved
Copy link
Member

@sfoster1 sfoster1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good and almost there, couple things to clean up. Please also update the description of the PR to have more about the flow of information and control through the tasks and describe the high points of the changes you made.

@codecov-commenter
Copy link

codecov-commenter commented Jan 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.05%. Comparing base (a6476b3) to head (827a8be).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #716      +/-   ##
==========================================
- Coverage   83.28%   83.05%   -0.24%     
==========================================
  Files         101      103       +2     
  Lines        4063     4102      +39     
==========================================
+ Hits         3384     3407      +23     
- Misses        679      695      +16     

see 11 files with indirect coverage changes

@pmoegenburg pmoegenburg requested a review from sfoster1 January 26, 2024 00:42
Copy link
Member

@sfoster1 sfoster1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking really good, just a couple small changes I think we should see. This is gonna be a huge step forward!

head/firmware/main_rev1.cpp Show resolved Hide resolved
pipettes/firmware/hardware_config.c Outdated Show resolved Hide resolved
include/pipettes/core/tasks/motion_controller_task.hpp Outdated Show resolved Hide resolved
Copy link
Member

@sfoster1 sfoster1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent! Really great work, I'm happy to see this merged. But, let's hold off on actually clicking the button until

  • the monorepo side is ready to merge
  • we have 7.2 out and know we won't need any other little firmware fixes.

@pmoegenburg pmoegenburg merged commit 762d6d5 into main Mar 25, 2024
31 checks passed
caila-marashaj added a commit that referenced this pull request Apr 22, 2024
ryanthecoder added a commit that referenced this pull request May 10, 2024
ryanthecoder added a commit that referenced this pull request May 10, 2024
* Revert "attempted fix for 96-channel motor driver error false positive (#772)"

This reverts commit 6599290.

* Revert "feat(motion-control): handle motor driver errors (#716)"

This reverts commit 762d6d5.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants