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

GREAT INTEGRATION #15

Merged
merged 458 commits into from
Apr 11, 2017
Merged

GREAT INTEGRATION #15

merged 458 commits into from
Apr 11, 2017

Conversation

DSsoto
Copy link
Member

@DSsoto DSsoto commented Mar 28, 2017

Lets make sure that everything is clean here before we merge back to master.

It would be helpful to have as many people as possible review this.

mattlangford and others added 30 commits October 1, 2016 18:48
GPS Bounds and Coordinate Conversion
PERCEPTION: Add initial dock shape vision service and shooter mission
…n_system

Conflicts:
	mission_control/navigator_missions/navigator_singleton/vision_services.yaml
	utils/navigator_msgs/CMakeLists.txt
SHOOTER: Work with new motor controller + other improvements
SHOOTER: Tuned timing vals/bug fixes
PERCEPTION: ros camera stream class, tf changes, and fix to point cloud colorer
MISSIONS: Start gate and teleop
The scripts here were demoted from being a ros package. It should be
removed from the SubjuGator repo.
DSsoto and others added 3 commits April 8, 2017 10:17
It was previously in the great_merge_purgatory of mil_vision
TOOLS: add script to rename topics/frames in bags
@@ -0,0 +1,5 @@
Some of these files can be made generic and therefore be able to stay as a part of mil_common. Until that is done, they will remain in this folder.
Copy link
Member

Choose a reason for hiding this comment

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

great_merge_purgatory

Copy link
Contributor

@kev-the-dev kev-the-dev left a comment

Choose a reason for hiding this comment

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

Could use some work. 6/10

Some of these changes need to be done before merging:

  • namespace changes
  • anything specific to sub8/navigator

Later

  • document

#include <image_geometry/pinhole_camera_model.h>
#include <opencv2/core/eigen.hpp>

namespace nav{
Copy link
Contributor

Choose a reason for hiding this comment

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

All namespaces should be mil_vision for resources in this library.


namespace nav{

class CameraObserver
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's have just one (David's) wrapper for camera subscribers/publishers. @DSsoto can you remove the other ones that do the same thing?

const float low_thresh_gain = 0.5,
const float high_thresh_gain = 0.5);

cv::Mat triangulate_Linear_LS(cv::Mat mat_P_l, cv::Mat mat_P_r,
Copy link
Contributor

Choose a reason for hiding this comment

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

No documentation for this function. I do I use it/ what does it do? Same with others

};


class PcdColorizer : public PcdSubPubAlgorithm{
Copy link
Contributor

Choose a reason for hiding this comment

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

Add TODO comment, mention unfinished

ros::Publisher rviz_pub;
ros::NodeHandle nh;
RvizVisualizer(std::string rviz_topic);
void visualize_buoy(geometry_msgs::Pose &pose, std::string &frame_id);
Copy link
Contributor

Choose a reason for hiding this comment

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

sub specific, remove

from mil_ros_tools import wait_for_param


def get_parameter_range(parameter_root):
Copy link
Contributor

Choose a reason for hiding this comment

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

sub specific, remove

self.bridge = CvBridge()
self.callback = callback

def wait_for_camera_info(self, timeout=10):
Copy link
Contributor

Choose a reason for hiding this comment

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

known bug when time goes backwards, fixed on sub8 version

import rospy


def rosmsg_to_numpy(rosmsg, keys=None):
Copy link
Contributor

Choose a reason for hiding this comment

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

@RustyBamboo has some fix for this. Please PR it

if k == ord('z'):
set_bars((0, 0, 0), (0, 0, 0))

# This functionality was configured to replace lines in navigator's tf launch file, should refactor to be general later
Copy link
Contributor

Choose a reason for hiding this comment

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

IF someone wants to refactor to be generic, that'd be cool

self.paused = not self.paused

def main():
rospy.init_node('video_player', anonymous=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

use argparse instead of params

DSsoto and others added 5 commits April 10, 2017 13:15
This is new mesage should be used by the depth driver instead of
Float64Stamped
Replaced with RangeStamped. DVL related cold that previously used
Float64Stamped should be transitioned to use RangeStamped instead.

Other depth driver related code should be changed to use
DepthStamped.

The only usage of Float64Stamped that is not DVL or depth or depth
driver related is in the start gate mission and should be removed
(@RustyBamboo).
Node used to be in the great_merge_transformer subdirectory.
All other nodes were considered not generic and removed from the repo.
It was decided that all the nodes directory would only be used for python nodes, wile the src and include directories will include c++ nodes, not just library code.
@kev-the-dev
Copy link
Contributor

Comments addressed and tested (at least for building) with SubjuGator, except for changes mentioned in #30, which can be done in a post-integration repo.

@sentree
Copy link
Member

sentree commented Apr 11, 2017

This built on the CI and passed all unit tests except the ros_alarms test that always fails.
-- Your friendly temporary CI guy

@sentree sentree merged commit dfe89ed into master Apr 11, 2017
sentree pushed a commit that referenced this pull request Apr 11, 2017
CONTROLLER and INTERFACE: lake day changes
@DSsoto DSsoto deleted the great_merge branch April 11, 2017 05:32
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.

7 participants