Added ability to control pulse width for kick and chip commands in RobotDiagnostics #3440
Conversation
wmostrenko
requested review from
itsarune,
sauravbanna and
williamckha
as code owners
itsarune
left a comment
itsarune
left a comment
There was a problem hiding this comment.
Looks good, I like saurav's suggestion about hiding the control sliders when the mode isn't selected. Let's try to get that in as well.
|
This PR is stale because it has been open for 30 days with no activity. Remove stale label or comment or this will be closed in 5 days. |
…what mode you are in
…l_pulse_width # Conflicts: # src/software/thunderscope/robot_diagnostics/chicker_widget.py
itsarune
left a comment
itsarune
left a comment
There was a problem hiding this comment.
looks good, left something minor comments! have you field tested this version yet?
Not yet!! I believe last time we tried to field test it there was some sort of issue connecting to the robots so perhaps we could try again if you're available sometime soon to show me how as I completely forgot? Thanks! |
We're in the mezz every saturday, so feel free to swing by and I can help! (or lmk if you want me to just field test for you) |
Great sounds good!! I'll drop by soon when find some time! Thanks! |
| ) | ||
|
|
||
| chicker_widget_vbox_layout.addWidget(self.auto_kick_chip_buttons_box) | ||
| chicker_widget_vbox_layout.addWidget(self.power_mode_buttons_box) |
There was a problem hiding this comment.
can you add the power_mode_buttons_box below the kick_chip_sliders_box? The sliders looks a bit stranded from the buttons that actually toggles their capability
itsarune
left a comment
itsarune
left a comment
There was a problem hiding this comment.
looks good, left another comment
Thunderbots
left a comment
Thunderbots
left a comment
There was a problem hiding this comment.
Looks good to me! Unfortunately kicking and chipping is broken on the robot we have right now, so we can't really test this out :( We'll merge your PR in as soon as #3493 is resolved.
William
| case TbotsProto::PowerControl::ChickerControl::kChipPulseWidth: | ||
| { | ||
| LOG(FATAL) | ||
| << "Chiper control using pulse widths is not supported in simulation"; |
There was a problem hiding this comment.
| << "Chiper control using pulse widths is not supported in simulation"; | |
| << "Chipper control using pulse widths is not supported in simulation"; |
5 participants
Please fill out the following before requesting review on this PR
Description
Added the ability to toggle between "m/s or m" power mode or "pulse width" power mode when kicking/chipping in robot diagnostics. Also added a slider to manually control the pulse width for the kicker and chipper. Also works for autokick/autochip.
Testing Done
Testing will need to be done with robots as when we tried previously there we too many technical difficulties connecting my computer to the robots.
Resolved Issues
resolves #3329
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