Skip to content

Teleop improvements - #47

Merged
caguero merged 10 commits into
lyricalfrom
caguero/joy_calibrate_owns_joy_node
Sep 11, 2026
Merged

Teleop improvements#47
caguero merged 10 commits into
lyricalfrom
caguero/joy_calibrate_owns_joy_node

Conversation

@caguero

@caguero caguero commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

What this adds

This patch adds four changes to the gamepad teleop tool:

  1. The mapping tool owns its joy_node. Mapping is now one command. If nothing is publishing /joy, the tool starts its own joy_node (with the autorepeat the baseline capture needs) and stops it on exit, spawned with the parent death signal so the helper dies even if the tool is killed. A running teleop session's joy_node is detected and reused. The old flow required a background joy_node, and forgetting to kill it left a stale publisher that would break the next teleop session.

  2. joy_calibrate is now joy_map (c1197f3). The tool discovers which physical control drives which function. The executable, node, docs and config headers are renamed.

  3. One shared pad mapping (d8d8f13). The mapping describes the pad, not a vehicle, so the three per vehicle copies collapse into one config/pad/.

  4. Store mapping config in a user dir (feb5299). joy_map writes $ROS_HOME/bluerobotics_teleop/pad/ and the launch prefers it over the shipped defaults, logging which it loaded. Writing into the installed share only worked under a symlink install: a copy install would lost the mapping on rebuild and a binary install cannot write /opt/ros at all. Same pattern as configure_vehicle.py --cache.

How to test

ros2 run bluerobotics_teleop joy_map
  1. With nothing else running: a joy_node appears for the walkthrough and is gone after quitting.
  2. The walkthrough saves under ~/.ros/bluerobotics_teleop/pad/; ros2 launch bluerobotics_teleop teleop.launch.py vehicle:=bluerov2 (or blueboat, bluerov2_heavy) logs Pad mapping: <path> and every vehicle drives from that one mapping. Delete the directory to fall back to the shipped defaults.
  3. colcon test --packages-select bluerobotics_teleop.

…ode can leak and break teleop

Assisted-by: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Carlos Agüero <caguero@honurobotics.com>
@caguero caguero changed the title Make joy_calibrate own its joy_node Improve teleop calibration Aug 30, 2026
… docs site

Assisted-by: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Carlos Agüero <caguero@honurobotics.com>
@bsb808
bsb808 self-requested a review August 31, 2026 14:58
@bsb808

bsb808 commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Reviewing this got me looking at the tool it wraps rather than the change itself, and two design questions came out of it. Neither is a reason to hold up the PR, but both get more expensive to act on the longer the tool sits where it is.

Where it lives. Nothing about this walkthrough is specific to the BlueROV2 or the BlueBoat, or to marine vehicles at all. It asks which stick and which button, and writes out the input side config the teleop stack reads. That is useful for any teleop task in the project, and arguably well beyond it. Two options worth weighing: move it to gz-maritime so it is available project wide rather than to the two Blue Robotics vehicles, and if it proves itself there, consider whether it wants to be upstreamed — this feels like a gap in the ROS teleop tooling generally, not something we should be the only ones with.

Does moving it to gz-maritime fit how you were thinking about the teleop package's scope, or is there a reason to keep it with the vehicles?

@bsb808

bsb808 commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

What it is called. I do not think "calibrate" is the right word. Calibration on a gamepad conventionally means setting the stick zero, the travel limits, and the deadzone, and that is the driver's job — jscal at the OS level, joy_node's deadzone parameter at the ROS level. What this tool does is discover which physical control maps to which function, which is closer to auto configuration, or a binding or mapping step. The distinction matters because the name is what a user reads before deciding whether the tool solves their problem: someone with a drifting stick would reasonably reach for something called joy_calibrate and come away unhelped. Something like joy_map or joy_configure would set the right expectation. The rename touches the executable name, the node name, the docs section and the config file headers, so it is cheapest now while the package is pre release and nothing downstream depends on the name.

…rather than calibrating the stick

Assisted-by: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Carlos Agüero <caguero@honurobotics.com>
@caguero

caguero commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

What it is called. I do not think "calibrate" is the right word. Calibration on a gamepad conventionally means setting the stick zero, the travel limits, and the deadzone, and that is the driver's job — jscal at the OS level, joy_node's deadzone parameter at the ROS level. What this tool does is discover which physical control maps to which function, which is closer to auto configuration, or a binding or mapping step. The distinction matters because the name is what a user reads before deciding whether the tool solves their problem: someone with a drifting stick would reasonably reach for something called joy_calibrate and come away unhelped. Something like joy_map or joy_configure would set the right expectation. The rename touches the executable name, the node name, the docs section and the config file headers, so it is cheapest now while the package is pre release and nothing downstream depends on the name.

Thanks for the suggestion. I renamed it to joy_map as suggested. I hope it's more clear. c1197f3

@caguero

caguero commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

What it is called. I do not think "calibrate" is the right word. Calibration on a gamepad conventionally means setting the stick zero, the travel limits, and the deadzone, and that is the driver's job — jscal at the OS level, joy_node's deadzone parameter at the ROS level. What this tool does is discover which physical control maps to which function, which is closer to auto configuration, or a binding or mapping step. The distinction matters because the name is what a user reads before deciding whether the tool solves their problem: someone with a drifting stick would reasonably reach for something called joy_calibrate and come away unhelped. Something like joy_map or joy_configure would set the right expectation. The rename touches the executable name, the node name, the docs section and the config file headers, so it is cheapest now while the package is pre release and nothing downstream depends on the name.

I'm not sure about this one: Mostly for two reasons: (1) this is a pure ROS package not even related with simulation. (2) it has a built-in mixer for these two vehicles. You don't teleoperate the thruster directly, you use the keypad to specify where do you want to go, and the mixer, translates that into thruster commands. I think the optimal solution should be to make a contribution to the upstream teleop_twist_joy package, and leave here the very minimum code.

@bsb808

bsb808 commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

@caguero I'm a little confused with the two different replies to the What it is called suggestion - one seems positive and the other has a question. In the second one, when you say "I'm not sure about this one:" - is there a specific aspect you of the comment you are unsure about?

The mixer and the calibration/configuration I see as two separate issues. The mixer (close to) vehicle specific. There are some general frames, but the mixer config would be best alongside each vehicle.

The configuration of the input device - which joy to use - seems like it does not belong alongside a specific vehicle or set of vehicles.

I may be misunderstanding what you are trying to accomplish. I think that is the hazard of trying to infer intent from the implementation - if I don't understand what you are trying to accomplish it makes it hard to provide constructive comments on the implementation.

…p to config/pad

Assisted-by: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Carlos Agüero <caguero@honurobotics.com>
…t over the shipped defaults

Assisted-by: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Carlos Agüero <caguero@honurobotics.com>
@caguero caguero changed the title Improve teleop calibration Improve the gamepad mapping tool Aug 31, 2026
@caguero caguero changed the title Improve the gamepad mapping tool Teleop improvements Aug 31, 2026
@caguero

caguero commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

I may be misunderstanding what you are trying to accomplish. I think that is the hazard of trying to infer intent from the implementation - if I don't understand what you are trying to accomplish it makes it hard to provide constructive comments on the implementation.

Don't be mad at me :)

I updated this PR and the description to capture all improvements in the teleop tool. Hopefully the description shows better the intent now. And I agree that there is some general-purpose functionality here, that ideally we should upstream, more than moving it to gz-maritime in my opinion.

Assisted-by: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Carlos Agüero <caguero@honurobotics.com>
Assisted-by: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Carlos Agüero <caguero@honurobotics.com>

@bsb808 bsb808 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is very cool set of features for configuring the joystick.

See associated PRs in slack

caguero and others added 2 commits September 11, 2026 21:20
…hen a stick lands on the D pad

The shipped mapping had been regenerated on another machine and no longer
matched the F310 in X mode: sway sat on the left trigger (which idles at
+1, a constant sway under the deadman), heave on the right stick's
horizontal axis, and EPA on the right trigger. joy_node lists the F310's
sticks as axes 0, 1, 3, 4 and its D pad hat as 6, 7.

The recommended layout is now ArduSub's, what QGroundControl gives a
BlueROV2 pilot: left stick heave and yaw, right stick surge and sway. The
shipped pair is surge 4, sway 3, heave 1, yaw 0, RB deadman and D pad EPA
on 7; a joy_map run on the F310 reproduces it exactly. The walkthrough
hints follow.

The walkthrough names X mode and a dark Mode LED as the state to map in
(lit, the F310 swaps the left stick and the D pad, which is how a mapping
ends up driving from the D pad), and flags a stick step that reads on a
hat axis so the operator can redo it. Saving exits straight to one
terminal line saying where the mapping went (the in screen "Saved!" page
repeated it and vanished with curses); quitting prints nothing. A config
test keeps the EPA axis off the motion axes.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EK8rFrqH5ggeFaand8nXBb
The walkthrough asked for EPA up and EPA down as two steps because a D
pad reaches joy_node either as one hat axis (most pads: up +1, down -1)
or as separate buttons, but only the up answer was used and only as an
axis: a button D pad silently got the default axis 7 and its clicks did
nothing. save_configs now writes whichever form the pad gave, axis_epa
for a hat and btn_epa_up / btn_epa_down for buttons, -1 marking the
unused form, and twist_to_thrust reads both: a click is the hat value
changing past +-0.5 or the rising edge of a button, factored into
epa_click so the edge logic is unit tested.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EK8rFrqH5ggeFaand8nXBb
caguero added a commit that referenced this pull request Sep 11, 2026
…op the D pad one recorded here

The mapping this branch shipped was recorded with the F310's Mode LED on,
so it drove surge and yaw from the D pad; #47 now ships the ArduSub
layout and joy_map warns about that state. The pad files take #47's
version, the rewritten how-to stays.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EK8rFrqH5ggeFaand8nXBb

# Conflicts:
#	bluerobotics_teleop/config/pad/joystick.config.yaml
#	docs/how-to/teleop.md
#55)

Assisted-by: Claude Fable 5.1
Co-authored-by: Carlos Agüero <caguero@honurobotics.com>
@caguero

caguero commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

This is very cool set of features for configuring the joystick.

See associated PRs in slack

Thanks for the suggestions, merged most of them!

@caguero
caguero merged commit 4bf5322 into lyrical Sep 11, 2026
2 checks passed
@caguero
caguero deleted the caguero/joy_calibrate_owns_joy_node branch September 11, 2026 22:25
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.

2 participants