-
Notifications
You must be signed in to change notification settings - Fork 3
Feature/pm 524 Generalize motor controller #284
base: develop
Are you sure you want to change the base?
Changes from 14 commits
6abd775
c8f3af5
f07ebf8
1db4b01
7cf1336
9d320e5
fffab68
ae33d44
00e9bad
2e1b2f4
ef553b9
a60801f
087227e
9d8bc65
81423e5
5e73645
f0c296f
ea89e8e
03c994a
df36bbf
5edecb5
58fbd2f
855e436
e68a574
0a1c16a
c49c6b3
0fbe358
4f66858
b48674d
81f0db4
41257c8
79a93a3
1e55adb
ea171c0
9b5cc5c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| // Copyright 2019 Project March. | ||
|
|
||
| #include "actuation_mode.h" | ||
| #include "motor_controller_state.h" | ||
| #include <string> | ||
|
|
||
| namespace march | ||
| { | ||
| class MotorController | ||
| { | ||
| public: | ||
| virtual double getAngleRadAbsolute() = 0; | ||
| virtual double getAngleRadIncremental() = 0; | ||
| virtual double getVelocityRadAbsolute() = 0; | ||
| virtual double getVelocityRadIncremental() = 0; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What if my motor controller does not have either an absolute or incremental encoder?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thought about this; the incremental/absolute encoders are a characteristic of Joint rather than of the motor controller, so it would make more sense to move such specifications there. That would require me to subclass Joint into different types, which I preferred to avoid, especially since our current three options for motor controllers will probably be connected to both types of encoders. I think such a refactor of the joint class is better postponed to a moment in the future where an encoder is actually connected to something other than a motor controller.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm on second thought, I could move the calculations using both encoders downstream to the motor controller. Then these would simplify to getPosition() and getVelocity(). Downside would be that these calculations would be that this code would probably be duplicate in each different MotorController class, but that might again be resolvable by inheriting from a MultipleEncodersMotorControllers class that implements these methods common for motor controllers with both encoders. That was a nice spam of hersenspinsels, let me know if it makes any sense and what you think.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Decided that since the 3 motor controllers currently under consideration all have both an incremental and an absolute encoder, this change is not very useful for now. It won't be difficult to implement later on when such a motor controller is actually put to use. |
||
| virtual int16_t getTorque() = 0; | ||
| virtual MotorControllerStates getStates() = 0; | ||
|
|
||
| virtual ActuationMode getActuationMode() const = 0; | ||
| virtual uint16_t getSlaveIndex() const = 0; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What if a motor controller isn't a slave or doesn't use the master-slave structure.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added a docstring to explain |
||
|
|
||
| virtual float getMotorCurrent() = 0; | ||
| virtual float getMotorControllerVoltage() = 0; | ||
| virtual float getMotorVoltage() = 0; | ||
| virtual bool getIncrementalMorePrecise() const = 0; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This also seems like a very specific function that should not be inside the motor controller class. When I read this method call I cannot tell what it would do generally on most controllers.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added a docstring with explanation. This method is relevant as long as a motor controller has both an incremental and an absolute encoder. |
||
|
|
||
| virtual void actuateRad(double target_rad) = 0; | ||
| virtual void actuateTorque(int16_t target_torque) = 0; | ||
|
|
||
| virtual void goToOperationEnabled() = 0; | ||
|
Olavhaasie marked this conversation as resolved.
Outdated
|
||
|
|
||
| virtual bool initialize(int cycle_time) = 0; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What if my motor controller does not need a cycle time, but it does need something else for initialization. That would require adding more parameters and therefore breaking the contract.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you could alternatively pass something like that to the constructor as well, but I guess that wouldn't be very need since you need it nowhere else. I think I am going to :ostrich: this problem since it is highly dependent on how exactly the startup of another motor controller would work, which is a bit difficult to predict. I will see if I get useful enough insight from the ODrive and the Ingenia to make general statements about this. Otherwise, I think I will leave it up to future generations to redesign this a bit. |
||
| virtual void reset() = 0; | ||
| virtual bool checkState(std::ostringstream& error_msg, std::string joint_name) = 0; | ||
|
|
||
| virtual ~MotorController() noexcept = default; | ||
| }; | ||
|
|
||
| } // namespace march | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| // Copyright 2019 Project March. | ||
| #ifndef MARCH_HARDWARE_MOTOR_CONTROLLER_STATE_H | ||
| #define MARCH_HARDWARE_MOTOR_CONTROLLER_STATE_H | ||
|
|
||
| #include <string> | ||
| #include "imotioncube_state.h" | ||
|
|
||
| namespace march | ||
| { | ||
| struct MotorControllerStates | ||
| { | ||
| public: | ||
| MotorControllerStates() = default; | ||
|
|
||
| float motorCurrent; | ||
| float controllerVoltage; | ||
| float motorVoltage; | ||
| int absoluteEncoderValue; | ||
| int incrementalEncoderValue; | ||
| double absoluteVelocity; | ||
| double incrementalVelocity; | ||
|
|
||
| std::string statusWord; | ||
| std::string motionError; | ||
| std::string detailedError; | ||
| std::string secondDetailedError; | ||
| IMCState state; | ||
| std::string detailedErrorDescription; | ||
| std::string motionErrorDescription; | ||
| std::string secondDetailedErrorDescription; | ||
|
Roelemans marked this conversation as resolved.
Outdated
|
||
| }; | ||
|
|
||
| } // namespace march | ||
|
|
||
| #endif // MARCH_HARDWARE_IMOTIONCUBE_STATE_H | ||
Uh oh!
There was an error while loading. Please reload this page.