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

ENABLE_BUTTON is now optional. Also added a braking button. #22

Open
wants to merge 10 commits into
base: indigo-devel
Choose a base branch
from

Conversation

andresgongora
Copy link

@andresgongora andresgongora commented May 12, 2016

ENABLE_BUTTON
The enable_button can be disabled by assigning joystick button "-1" to it. By default it is still assigned to button 0.

BREAK_BUTTON BRAKE_BUTTON
Because without enable_button the no-movement command would never be sent (it gets sent when enable_button is released) a breaking button was added which does exactly that.

The break_button brake_button is by default disabled (assigned to button "-1"). If enabled, breaking takes preference over any other kind of movement.

…thout enable_button the no-movement command would never be sent (it gets sent when enable_button is released) a breaking button was added which does exactly that. break_button is by default disabled (assigned to button -1). If enabled, breaking takes preference over any other kind of movement.
@mikepurvis
Copy link
Member

Name should be BRAKE_BUTTON, but I'm wondering if you could describe more about your use-case— having an enable/deadman button is pretty standard in robots; what you're basically doing here is inverting that and offering the brake as an "unenable" button.

@andresgongora
Copy link
Author

@mikepurvis Thank you, i didn't notice my typo. I renamed break_button to brake_button.

Also, about the uses-cases. All my motor-controlling nodes have an internal timeout, with which, if no twist message is received in the last N seconds, they stop.

teleop_twist_node, on the other hand implements the dead-man-switch directly in the controller, but this does not guarantee that a robot stays put if the teleop_twist_node for some reason fails. Therefore, in my case, having to compulsory use enable_button was more inconvenience than solution.

But if I simply disable enable_button, and the dead-zone on my joystick is not correctly defined, the robot might receive a tiny and unwanted movement_smg/twist command (note that if enable_button is not used, teleop_twist_node is continuously publishing on the movement topic).

Therefore I added the brake_button, which tells teleop_twist_node not to publish any twist (if braking is enabled and while pressed).

@mikepurvis
Copy link
Member

Part of the point is that teleop_twist_node is very simple, so it is unlikely to just fail for no reason. :)

You should be able to get most of what you want using the options available in joy_node, specifically ~deadzone and ~autorepeat_rate; the combination of these two will ensure that your motors get a constant stream of commands, and that zero-ish commands are interpreted as zero.

@andresgongora
Copy link
Author

@mikepurvis True indeed. I just wanted the means for manually overriding any movement command with the brake_button, but at the same time eliminate the need for the enable_button.

I see this solution might not fill all cases. Could you help me to think out an alternative? :)

@andresgongora
Copy link
Author

andresgongora commented May 12, 2016

The Travis CI build failed.
After reading the report I can't figure out the source of the failure. Could some please point it out to me? I will try to fix it ASAP.

EDIT: I hope it was the break/brake typo. Let's give it another run.

@andresgongora andresgongora changed the title enable_button is optional. Also added breaking button. ENABLE_BUTTON is now optional. Also added a braking button. May 12, 2016
@mikepurvis
Copy link
Member

Travis is upset about linter violations. Build the roslint target locally and see what it complains about.

@andresgongora
Copy link
Author

@mikepurvis Thank you :) I think I've fixed them all

it->first.c_str(), it->second, pimpl_->scale_linear_map[it->first]);
ROS_INFO_COND_NAMED(pimpl_->enable_turbo_button >= 0, "TeleopTwistJoy",
"Turbo for linear axis %s is scale %f.", it->first.c_str(), pimpl_->scale_linear_turbo_map[it->first]);
ROS_INFO_COND_NAMED(pimpl_->enable_button >= 0,
Copy link
Member

Choose a reason for hiding this comment

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

Extra space here and below.

@mikepurvis
Copy link
Member

mikepurvis commented May 31, 2016

Sorry for the delay on this. Would like to merge and then release to at least Kinetic. A few remaining concerns though:

  • I would prefer not to modify the default behaviour— it should remain as it was, with no brake button, and
  • If the brake_button == -1 (the default) it should have no effect.
  • It would be great to have some more tests covering these new scenarios (brake button + enable button config, brake button only config).

@arne48 Will this implementation meet your needs in #19? Alternatively, we could avoid the brake button business altogether and go with your simpler change which only allows disabling the enable button.

@andresgongora
Copy link
Author

@mikepurvis: absolutely, maybe forgetting about the brake button altogether for the sake of KISS would be better.

I guess I'll simply add the option to make enable_button == -1 as in #19.

Regarding your comments:

  • It was a few weeks back, but I recall that the default behaviour was not modified (enable_button enabled, and brake button disabled)
  • brake_button==-1 has no effect
  • If we want to keep the brake_button (which will probably be ditched) I can write a few test scenarios for Travis with different combinations.

I'll take a look at the code ASAP.

@andresgongora
Copy link
Author

I'm not sure why "enable_button_disabled_joy.test" fails.
Maybe it's because I give it an empty button vector (I'll try to change it).
But I fear it is more severe (Travis reports something about a timeout)

@andresgongora
Copy link
Author

Ok. This worked.

I have removed the brake button, simply making ENABLE_BUTTON optional (can be explicitly disabled by asigning it to key -1).
I've also added some test for travis to test the disabled ENABLE_BUTTON with and without the TURBO_BUTTON.

Ehm. I don't know. Do you like the idea of the optional ENABLE_BUTTON?
How do I proceed?

@faizuddinfaruqui
Copy link

Hello all, I want to publish on /MyRobot1/cmd_vel, /MyRobot2/cmd_vel, /MyRobot3/cmd_vel for multiple robots in gazebo using three joysticks. How would I do that using this code.
Thanks in advance.

@andresgongora
Copy link
Author

andresgongora commented Oct 24, 2016

Hello @faizuddinfaruqui. This thread is related with a merge-request (modification) of the joystick code. So you will probably find no answer here. However, let me point you to "ROS launch files". You can rename topics and nodes using these files. This would allow you to rename, for instance , all topics adding an identificaton number at the end (ie: cmd_vel1, cmd_vel2 and cmd_vel3). Just google it. Good luck ;)

bdigney89 pushed a commit to bdigney89/teleop_twist_joy_old that referenced this pull request Feb 18, 2024
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.

4 participants