uwreact / uwreact_robot

Software behind our fully autonomous FIRST robots

BSD 3-Clause "New" or "Revised" License

8 stars 6 forks source link

Added network visualizer Catkin package #65

Closed Ivan-Z closed 5 years ago

Ivan-Z commented 5 years ago

Pull Request

Closes #33

Adds a Catkin package for the network visualization tool, along with a blank PyQt window.

Contribution Checklist

[X] I have read the contributing guide.
[X] I have referenced relevant issues.
[X] I have labeled accordingly.
[X] I have detailed my changes as much as possible.

Change Checklist

[ ] I have added tests.
[ ] I have added necessary documentation.
[ ] I have tested my changes on real hardware.

wmmc88 commented 5 years ago

Ur formatting has an extra space

matthew-reynolds commented 5 years ago

Most of the code should live in the src/package_name directory, that's where window.py should be. The files in scripts should just be short little script that interact with the main code imported from the package_name module. By convention, these scripts also typically don't end in .py

The snippet in my change request comment should be everything you need in the open_network_visualizer file in the script directory - The MainWindow class should live instead in src/network_visualizer/window.py as before.

matthew-reynolds commented 5 years ago

window.py needs the shebang and copyright, and the package.xml needs the change commented above (depend instead of build_depend). Other than that, looks ready to land.

matthew-reynolds commented 5 years ago

window.py looks good, just missing the executable permissions on the open_network_visualizer script. (Just a chmod +x open_network_visualizer or chmod 775 open_network_visualizer will do)

matthew-reynolds commented 5 years ago

@wmmc88 Please give this one last scan