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

Cleanup Repository & Simplify Usage #14

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

jmount1992
Copy link

I have performed a repository cleanup, which I feel simplifies its usage. I have also fixed minor errors.

Summary of changes and reasoning:

Scout Base Package

Assumption: this package's sole purpose is to establish the interface between computer and the scout base

  • the package now only contains a single launch file scout_base.launch. I have removed display_model.launch and other launch files used for different configurations. Reasoning:
    • having multiple launch files where the only difference is a change in the default arguments is superfluous and can lead to confusion. That is the point of exposing the arguments.
    • this is the interface level and should only bring up the interface, so it should not include display.launch
  • removed rviz folder from the package - same reasoning for the removal of display_model.launch

Scout Bring Up

Assumption: this is the desired main entry point into the ecosystem, launching the robot interface, robot transforms, and RVIZ if desired.

  • Removed scout_mini_robot.launch, scout_miniomni_robot_base.launch. Added required arguments to expose all parameters in scout_base.launch. Reasoning:
    • Again having multiple launch files with different default argument values and arguments that aren't even passed down (in the case of scout_robot_base.launch, is_scout_mini argument isn't passed down), is superfluous and leads to confusion. Having a single launch file with documented arguments simplifies the user experience.
  • Change default of pub_tf to true. Reasoning:
    • most packages performing localisation or transforms require the odom to base_link transform. So makes sense to publish this by default.
  • Added rviz folder with a singular config. Set global frame to odom. Reasoning:
    • having visualization config here makes sense.
    • for new users seeing the robot move relative to odom makes more sense

Scout Description

  • Changed wheel joint types to fixed, rather than continuous. Reasoning:
    • Otherwise require to run a joint_state_publisher. And for majority of users, they will not care how much the wheel as rotated, nor will care about the wheel not rotating when the vehicle moves in RVIZ. However, there may be a reason for having the joint type set to continuous. If so easy to add in the joint_state_publisher, non-gui version, into scout bring up launch file.
  • Renamed scout_mini.urdf.xacro to scout_mini.xacro to align with the other filenames
  • Removed rviz folder as not required if Scout Bringup package is the entry point into the ecosystem.

Updated the english readme to reflect changes and fix minor documentation errors.

@jmount1992
Copy link
Author

Also in scout_base fixed logic to allow for simulated robot.

@jmount1992
Copy link
Author

The other question, the entry point (scout_bringup/launch/scout_robot_base.launch) could be renamed to for clarity. Probably something simply like scout.launch. Thoughts?

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.

1 participant