Skip to content

Feature/unified ground station bringup - #9

Open
Haroldmin1028 wants to merge 6 commits into
mainfrom
feature/unified-ground-station-bringup
Open

Feature/unified ground station bringup#9
Haroldmin1028 wants to merge 6 commits into
mainfrom
feature/unified-ground-station-bringup

Conversation

@Haroldmin1028

Copy link
Copy Markdown
Contributor
  • Changed commands to run ground station to a single command

  • Created ground_station.launch.py for a unified launch process

  • Added passing smoke tests

  • Updated BuildInstructions.md

  • Validation logs:

$ ros2 launch waybionic_bringup ground_station.launch.py launch_rviz:=false use_joint_state_publisher_gui:=false
[INFO] [launch]: All log files can be found below /home/hoodu/.ros/log/2026-07-21-15-38-55-615679-HKINSPIRON-3799
[INFO] [launch]: Default logging verbosity is set to INFO
[INFO] [robot_state_publisher-1]: process started with pid [3804]
[INFO] [temporary_diagnostics_publisher.py-2]: process started with pid [3805]
[robot_state_publisher-1] [INFO] [1784669935.969631502] [robot_state_publisher]: Robot initialized
[temporary_diagnostics_publisher.py-2] [INFO] [1784669936.144753095] [temp_diag_pub]: Publishing temporary diagnostics to /diagnostics in 'normal' mode at 2.0 Hz
^C[WARNING] [launch]: user interrupted with ctrl-c (SIGINT)
[robot_state_publisher-1] [INFO] [1784670104.043991761] [rclcpp]: signal_handler(SIGINT/SIGTERM)
[INFO] [robot_state_publisher-1]: process has finished cleanly [pid 3804]
[INFO] [temporary_diagnostics_publisher.py-2]: process has finished cleanly [pid 3805]

@Haroldmin1028

Copy link
Copy Markdown
Contributor Author

Addressed comments and fixed PR.

  • Added missing waybionic_unified.rviz
  • Fixed launch files to pass tests
  • Passes CI so Yassin doesn't get alerted

@yassinsolim yassinsolim left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Changes Requested

Reviewed and tested commit 7d7523b.

The unified launch works when the full workspace is built, but these issues need to be fixed before merging:

  1. Temporary diagnostics publisher crashes

    • ground_station.launch.py passes mode: mock.
    • The publisher only accepts normal, fault, stale, or cycle.
    • Enabling start_temporary_diagnostics_publisher causes an immediate ValueError.
  2. Build instructions exclude the RViz plugin

    • BuildInstructions.md and scripts/check_waybionic_ws.sh only build waybionic_description and waybionic_bringup.
    • A fresh build opens RViz without the diagnostics panel because waybionic_rviz_plugins is not installed.
    • Build the full workspace or include all three packages.
    • Add waybionic_rviz_plugins as an exec_depend in waybionic_bringup/package.xml.
  3. The new launch test is not actually running

    • test_ground_station_launch.py is not registered in CMakeLists.txt.
    • CI only runs the existing lint and metadata tests.
    • The test also uses process=None and immediately passes without confirming that the expected nodes remain alive.
    • Register it with add_launch_test and assert the expected processes or diagnostics topic.
  4. Generated Python cache was committed

    • Remove the committed __pycache__/*.pyc file.
    • Add __pycache__/ and *.pyc to .gitignore.

I verified the full build and RViz startup on Ubuntu and macOS. These failures are not macOS-specific. Once these issues are fixed and CI runs the real launch test, this should be ready to approve.

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