Shaun teleop - basic teleop control + estop and zero_state buttons#153
Shaun teleop - basic teleop control + estop and zero_state buttons#153
Conversation
imzaynb
left a comment
imzaynb
left a comment
There was a problem hiding this comment.
Look at what I have so far, and then when you push that, I will look at the testing stuff.
Good work!
There was a problem hiding this comment.
If possible, remove this file from your commit. The tagged line is supposed to be commented out unless you are on Mac. Thanks!
There was a problem hiding this comment.
What's the purpose of this file? From what I can see there is only one source file which is joystick_teleop_continuous.py.
The file is attempting to launch a non-existant node joystick_teleop. The only node that exists currently is joystick_teleop_continuous. These nodes are registered with ROS2 in the setup.py.
Maybe remove this file?
| self._is_pressed = is_pressed | ||
|
|
||
| @property | ||
| def just_pressed(self) -> bool: |
There was a problem hiding this comment.
Does this suffice for debouncing? I'm curious.
There was a problem hiding this comment.
I think yes-- since we are reading from a pretty high level ROS node for the joystick, I believe the node will hand debouncing and we can assume that any outputs from the controller are 'ideal'.
| def __init__(self) -> None: | ||
| super().__init__("joystick_teleop_continuous") | ||
|
|
||
| params_path = self.declare_parameter( |
There was a problem hiding this comment.
You don't need to load parameters in this way.
ROS will handle loading in the yaml file under the folder mrobosub_teleop/params/<file>.yaml provided you specify this file in the launch file. I explained this within the launch file above.
Additionally, you will want to change the format of your file to be inline with the way that ROS expects to load in parameters from your files. A good example of this is mrobosub_hal/params/thruster_controller_params.yaml
From there, you can use Muskaan's parameter loading to handle loading in the parameters.
Step 1: List out all parameters
import Param from mrobosub_lib
Then create a list of parameters, perhaps called params, based off of the parameters in your param file.
params = [Param("deadzone", Parameter.Type.INTEGER, "The deadzone of the controller"), <more parameters here>]I think they should work with the nested parameters that you provided with the dictionary. Test it out if you can. If this doesn't work, I am sure we can make our parameter abstraction more robust to handle it.
Step 2: Declare parameters
After you create the list of parameters, you need to declare them with the node, like such:
self.declare_params(params)Step 3: Use the params
The parameters will now be available under self.<param> for instance, self.deadzone, etc. And these parameters will automatically refresh if they are changed from, for instance, RQT.
There was a problem hiding this comment.
Overall, the file looks really solid, good job. Please consider the changes I suggest if you can.
There was a problem hiding this comment.
See if you can change the formatting of this file to use the ros params way instead.
|
was able to get to everything except the param loading, not sure I quite undertand how ROS does it, will take a look tomorrow |
refactored to use enums; function map for button actions; more accurate button state tracking; naming conventions; still need to address param loading style,
3 participants
No description provided.