Stspin Motor driver integration#3734
Conversation
GrayHoang
requested review from
Andrewyx,
sauravbanna and
williamckha
as code owners
…are into motor_driver_integration
…or_driver_integration
StarrryNight
left a comment
StarrryNight
left a comment
There was a problem hiding this comment.
Reviewed a bit, will finish the rest later
| * motor fault | ||
| * @param motor_faults a set of faults associated with this motor | ||
| */ | ||
| MotorFaultIndicator(bool drive_enabled, |
There was a problem hiding this comment.
Passing in drive_enabled seems redundant as we can just initialize the boolean as false.
There was a problem hiding this comment.
I'm not sure about if this exact use case exists but I'm pretty sure there's an instance where we may want to be able to disable the motors immediately upon startup of motor service (e.g. the robot is reset after a motor fault, and wants to immediately disable previously faulted motors).
Andrewyx
left a comment
Andrewyx
left a comment
There was a problem hiding this comment.
Almost there. I think there is a lot of work involved in cleaning up motor.cpp and motor.h after auditing them. Both of these files seem to include far to much state leakage and exposes too much of the SPI related implementation. Otherwise, for the scope of this PR, I left a few minor comments. Please create a subsequent issue detailing the work involved for a future motor refactor.
|
A lot of the stuff in motor.cpp looks like unified fault handling and command distribution that is common to both trinamics and stspin drivers. We could probably simplify that whole interface by abstracting the motor fault handling to its own separate class? But that's for another pr lol |
5 participants
Description
Isolates the motor driver related changes from #3715
resolves #3711
Please look mainly at motor.cpp and the new motor_controller directory in embedded
Testing Done
Resolved Issues
Length Justification and Key Files to Review
Review Checklist
It is the reviewers responsibility to also make sure every item here has been covered
.hfile) should have a javadoc style comment at the start of them. For examples, see the functions defined inthunderbots/software/geom. Similarly, all classes should have an associated Javadoc comment explaining the purpose of the class.TODO(or similar) statements should either be completed or associated with a github issue