Conversation
|
Hi @clalancette and @JWhitleyWork, If you think this makes sense for the project, we’d love to see it merged when you have time. We’d really appreciate a PR review whenever it fits your schedule — happy to address any feedback or make adjustments. Thanks a lot for your work and for maintaining this project! |
joy/src/joy.cpp
Outdated
| status = future_.wait_for(std::chrono::seconds(0)); | ||
|
|
||
| if (haptic_ != nullptr && autocenter_) { | ||
| SDL_HapticSetAutocenter(haptic_, autocenter_); |
There was a problem hiding this comment.
Please, handle the return value: https://wiki.libsdl.org/SDL2/SDL_HapticSetAutocenter#return-value . Ideally, log a warning.
|
Thanks for the PR. I'm not yet 100% sure about the ABI breakage policy of this package, but normally, adding a class member to a public header means breaking ABI, which would mean this change could only be merged to Rolling. If you needed it to work in older ROS distros, some of the hacks for keeping ABI stable would have to be applied. Do you need this change in some of the stable distros? |
Co-authored-by: Martin Pecka <peci1@seznam.cz>
Co-authored-by: Martin Pecka <peci1@seznam.cz>
4da8bd8 to
477750e
Compare
|
Thanks for the review @peci1 :) I fixed the code accordingly. Also tested again on our Logitech G920. We are running on Jazzy. Also happy to help implementing the "hacks" if necessary. |
Adding a parameter to use autocentering.
Tested on a Logitech G920.