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

Added support for PS4 joystick #2060

Merged
merged 1 commit into from
May 11, 2020

Conversation

TrippleBender
Copy link
Contributor

@TrippleBender TrippleBender commented May 7, 2020

Description

Please explain the changes you made, including a reference to the related issue if applicable

Checklist

  • Required by CI: Code is auto formatted using clang-format
  • Extend the tutorials / documentation reference
  • Document API changes relevant to the user in the MIGRATION.md notes
  • Create tests, which fail without this PR reference
  • Include a screenshot if changing a GUI
  • While waiting for someone to review your request, please help review another open pull request to support the maintainers

@welcome
Copy link

welcome bot commented May 7, 2020

Thanks for helping in improving MoveIt and open source robotics!

Copy link
Contributor

@v4hn v4hn left a comment

Choose a reason for hiding this comment

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

I can't test this at the moment, but I guess you did to come up with an assignment.

That's good enough for me here.

@TrippleBender
Copy link
Contributor Author

I added support for the PS4 Controller. Also added a description in the tutorial

@codecov-io
Copy link

Codecov Report

Merging #2060 into master will decrease coverage by 0.34%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2060      +/-   ##
==========================================
- Coverage   54.51%   54.17%   -0.35%     
==========================================
  Files         328      328              
  Lines       25624    25624              
==========================================
- Hits        13970    13882      -88     
- Misses      11654    11742      +88     
Impacted Files Coverage Δ
...ls/include/moveit/py_bindings_tools/gil_releaser.h 0.00% <0.00%> (-100.00%) ⬇️
.../include/moveit/py_bindings_tools/py_conversions.h 0.00% <0.00%> (-86.67%) ⬇️
...s/include/moveit/py_bindings_tools/serialize_msg.h 0.00% <0.00%> (-80.00%) ⬇️
...ove_group_interface/src/wrap_python_move_group.cpp 31.96% <0.00%> (-11.08%) ⬇️
.../move_group_interface/src/move_group_interface.cpp 37.10% <0.00%> (-3.67%) ⬇️
...nning_scene_monitor/src/planning_scene_monitor.cpp 70.50% <0.00%> (-0.45%) ⬇️
...raint_samplers/src/default_constraint_samplers.cpp 82.90% <0.00%> (+0.36%) ⬆️
...meterization/work_space/pose_model_state_space.cpp 82.99% <0.00%> (+0.68%) ⬆️
...anning_scene_monitor/src/current_state_monitor.cpp 52.82% <0.00%> (+1.53%) ⬆️
.../ompl_interface/src/detail/constrained_sampler.cpp 64.10% <0.00%> (+17.94%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 23fbea1...fa252a5. Read the comment docs.

@simonschmeisser
Copy link
Contributor

Thanks for your first PR! This is how we move it! ;)

Please note that the text that is there when you open the PR is meant as a template, so please click edit and exchange Please explain the changes you made, including a reference to the related issue if applicable by a description of what you want to change and why. You later wrote that in a comment but it's to find and parse this way.

Also, next time, when you write Also added a description in the tutorial please add a link to the PR for the tutorial

@simonschmeisser
Copy link
Contributor

In an ideal world I would prefer it if this code would not guess the type of input device from the number of buttons but that does not seem to be exposed by this very spartan msg api, so LGTM

@v4hn
Copy link
Contributor

v4hn commented May 11, 2020

In an ideal world I would prefer it if this code would not guess the type of input device from the number of buttons but that does not seem to be exposed by this very spartan msg api, so LGTM

You could try to convince joy upstream to add the device id (Best case also unique between multiple devices) as frame_id. That's pretty much what I would expect from the field in this message.

@v4hn
Copy link
Contributor

v4hn commented May 11, 2020

Merging with two approvals.

@v4hn v4hn merged commit 8a61ed6 into moveit:master May 11, 2020
@welcome
Copy link

welcome bot commented May 11, 2020

Congrats on getting your first MoveIt pull request merged and improving open source robotics!

v4hn pushed a commit to v4hn/moveit that referenced this pull request May 18, 2020
v4hn pushed a commit to v4hn/moveit that referenced this pull request May 20, 2020
@v4hn v4hn mentioned this pull request May 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants