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

Right_Click_Services #590

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

Conversation

alex94263
Copy link

We developed a new plugin, which is called right_click_services.
Upon right clicking into mapviz-plugin a service "available_commands" is called, which retreives commands/services available (passing the GPS coordinates as well).
By Choosing a command in the context-menu a user can choose one of the returned commands which then gets passed to the service "execute_commands".
We developed this plugin to tell our drone to move to a specific point, to specify a waypoint-route and let the drone fly along this route.
We feel like it is an improvement and therefore would like to see it in the main repository included.

@evenator
Copy link
Contributor

You appear to have committed a lot of extraneous files by accident.

@alex94263
Copy link
Author

I'm sorry this happened by mistake.
I removed the extraneous files.

@pjreed pjreed changed the base branch from kinetic-devel to master November 9, 2018 23:47
Copy link
Contributor

@pjreed pjreed left a comment

Choose a reason for hiding this comment

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

Thanks for the work; sorry it's taken me a long time to get to this, it's been a while since I've had the time to work on mapviz. Other than the comments I've left on the diffs, it would also be good to have a description of this in the README.md and maybe a few examples of where it is useful.

mapviz/include/mapviz/map_canvas.h Outdated Show resolved Hide resolved
@@ -31,8 +31,15 @@
#include <GL/glew.h>
#include <GL/gl.h>
#include <GL/glu.h>

#include <iostream>
Copy link
Contributor

Choose a reason for hiding this comment

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

This include is duplicated.

mapviz/src/map_canvas.cpp Outdated Show resolved Hide resolved
QMenu contextMenu(this);
std::vector<QAction *> actions;

for(auto service : result){
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a general comment that applies to the rest of the code here, but please indent with two spaces per level and put braces on their own lines.

ROS_INFO("make call to %s",selectedItem->text().toStdString().c_str());
return selectedItem->text().toStdString().c_str();
}else{
ROS_ERROR("Service:'' doesn't exist");
Copy link
Contributor

Choose a reason for hiding this comment

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

This error message looks like it should be printing out a string; is it missing something?

// *****************************************************************************


#ifndef MAPVIZ_PLUGINS_right_click_services_H
Copy link
Contributor

Choose a reason for hiding this comment

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

Preprocessor definitions should be in all capitals.

@@ -0,0 +1,8 @@
#Location
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a need to create a new message for this rather than using sensor_msgs/NavSatFix? Re-using an existing message would remove the need to add message generation to this package and also make it easier for this to interact with other packages.

@@ -0,0 +1,283 @@
// *****************************************************************************
Copy link
Contributor

Choose a reason for hiding this comment

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

Please include the three-clause BSD license in the header here, as seen in all of the other source code files.

}

//call service to execute chosen command
ros::service::waitForService("/mapviz/gps_command_execute",10);
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure that having a hard-coded absolute path here is a good idea. It would be useful for the service to be user-selectable in the GUI; see the select_service_dialog.h header for an example of a dialog that can be used to make it easier for users to select services.

image_width: 25600
image_height: 17920
tile_size: 512
image_path: "../../drones4life_scenario/launch/data/mapviz_tiles/tiles"
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this file need to be modified at all?

@alex94263
Copy link
Author

@pjreed
I tried to apply the changes requested, but I seem to be running into problems with the Travis CI build failing.
I am not completely sure how to resolve these issues.

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.

3 participants