Added per-motor control to Thunderscope diagnostics #3401
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Muxite
requested review from
itsarune,
williamckha and
sauravbanna
as code owners
|
this looks pretty cool |
someone2060
requested review from
a team,
JoshuaWu7 and
ds570001
and removed request for
a team
williamckha
reviewed
Oct 31, 2024
src/software/thunderscope/robot_diagnostics/drive_and_dribbler_widget.py
Outdated
Show resolved
Hide resolved
src/software/thunderscope/robot_diagnostics/drive_and_dribbler_widget.py
Outdated
Show resolved
Hide resolved
src/software/thunderscope/robot_diagnostics/drive_and_dribbler_widget.py
Outdated
Show resolved
Hide resolved
src/software/thunderscope/robot_diagnostics/drive_and_dribbler_widget.py
Show resolved
Hide resolved
src/software/thunderscope/robot_diagnostics/drive_and_dribbler_widget.py
Outdated
Show resolved
Hide resolved
…r-motor # Conflicts: # src/software/thunderscope/robot_diagnostics/drive_and_dribbler_widget.py
itsarune
reviewed
Nov 9, 2024
Comment on lines
57
to
81
| if not self.drive_mode == DriveMode.VELOCITY: | ||
| motor_control.ClearField("direct_per_wheel_control") | ||
| motor_control.direct_velocity_drive.velocity.x_component_meters = ( | ||
| self.x_velocity_slider.value() | ||
| ) | ||
| motor_control.direct_velocity_drive.velocity.y_component_meters = ( | ||
| self.y_velocity_slider.value() | ||
| ) | ||
| motor_control.direct_velocity_drive.angular_velocity.radians_per_second = ( | ||
| self.angular_velocity_slider.value() | ||
| ) | ||
| else: | ||
| motor_control.ClearField("direct_velocity_control") | ||
| motor_control.direct_per_wheel_drive.front_left_wheel_velocity = ( | ||
| self.front_left_motor_slider.value() | ||
| ) | ||
| motor_control.direct_per_wheel_drive.front_right_wheel_velocity = ( | ||
| self.front_right_motor_slider.value() | ||
| ) | ||
| motor_control.direct_per_wheel_drive.back_left_wheel_velocity = ( | ||
| self.back_left_motor_slider.value() | ||
| ) | ||
| motor_control.direct_per_wheel_drive.back_right_wheel_velocity = ( | ||
| self.back_right_motor_slider.value() | ||
| ) |
There was a problem hiding this comment.
nit: why structure it this way?
why not if self.drive_mode == DriveMode.MOTOR to avoid that confusing not if you are handling the else case here as well?
so
if self.drive_mode == DriveMode.MOTOR
...
else:
itsarune
reviewed
Nov 9, 2024
Comment on lines
46
to
48
| self.drive_mode = DriveMode.VELOCITY | ||
| self.use_direct_velocity.setChecked(True) | ||
| self.toggle_drive_mode(DriveMode.VELOCITY) |
There was a problem hiding this comment.
I believe you could replace this with self.use_direct_velocity.click() since it is attached with the QtSignal to toggle_drive_mode
itsarune
reviewed
Nov 9, 2024
src/software/thunderscope/robot_diagnostics/drive_and_dribbler_widget.py
Show resolved
Hide resolved
itsarune
reviewed
Nov 9, 2024
Comment on lines
70
to
81
| motor_control.direct_per_wheel_drive.front_left_wheel_velocity = ( | ||
| self.front_left_motor_slider.value() | ||
| ) | ||
| motor_control.direct_per_wheel_drive.front_right_wheel_velocity = ( | ||
| self.front_right_motor_slider.value() | ||
| ) | ||
| motor_control.direct_per_wheel_drive.back_left_wheel_velocity = ( | ||
| self.back_left_motor_slider.value() | ||
| ) | ||
| motor_control.direct_per_wheel_drive.back_right_wheel_velocity = ( | ||
| self.back_right_motor_slider.value() | ||
| ) |
There was a problem hiding this comment.
it should be motor_control.direct_per_wheel_control.front_left_wheel_velocity, etc.
Currently, this line errors out when you run it.
itsarune
reviewed
Nov 9, 2024
itsarune
reviewed
Nov 16, 2024
Comment on lines
+397
to
+400
| def disconnect_dribbler_sliders(self) -> None: | ||
| self.dribbler_speed_rpm_slider.valueChanged.disconnect() | ||
|
|
||
| def disconnect_motor_sliders(self) -> None: |
There was a problem hiding this comment.
nit: add some function docs
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Added per motor control as a set of radio buttons in thunderscope diagnostic. Direct velocity and per motor control will switch depending on which radio button is pressed. The two control widgets will not appear at the same time, since both inputs are not allowed at the same time. Pressing the button for per-motor control will shut off direct-velocity control and reset those sliders, then switch the widget for per-motor control. Vice versa also applies.
Testing Done
It was tested on a robot in the mezz. Test was made on all sliders of direct-velocity, switching to per-motor without using reset, changing sliders on per-motor, and switching back to direct-velocity. Back-and-forth was done twice. The robot did all the commands it was told to on all motors. It was also tested using print() on the terminal.
Resolved Issues
resolves #3320
Length Justification and Key Files to Review
Key file: drive_and_dribbler_widget.py (relates to primitive.cpp, but primitive was not edited)
Added a function for switching between direct velocity and per motor control mode. Changed the organization of the reset_motor functions to split them up so that each could be run individually instead of having to reset all. I removed the function for enabling the motors, as it was unused (there was no way to use it via the UI).
Review Checklist
.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 issueThe current max speed for per-motor speeds is the same variable in tbots_cpp.create2021RobotConstants() called robot_max_speed_m_per_s.