-
Notifications
You must be signed in to change notification settings - Fork 152
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
base: master
Are you sure you want to change the base?
Right_Click_Services #590
Conversation
You appear to have committed a lot of extraneous files by accident. |
I'm sorry this happened by mistake. |
There was a problem hiding this 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.
@@ -31,8 +31,15 @@ | |||
#include <GL/glew.h> | |||
#include <GL/gl.h> | |||
#include <GL/glu.h> | |||
|
|||
#include <iostream> |
There was a problem hiding this comment.
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
QMenu contextMenu(this); | ||
std::vector<QAction *> actions; | ||
|
||
for(auto service : result){ |
There was a problem hiding this comment.
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.
mapviz/src/map_canvas.cpp
Outdated
ROS_INFO("make call to %s",selectedItem->text().toStdString().c_str()); | ||
return selectedItem->text().toStdString().c_str(); | ||
}else{ | ||
ROS_ERROR("Service:'' doesn't exist"); |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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.
mapviz_plugins/msg/Location.msg
Outdated
@@ -0,0 +1,8 @@ | |||
#Location |
There was a problem hiding this comment.
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 @@ | |||
// ***************************************************************************** |
There was a problem hiding this comment.
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); |
There was a problem hiding this comment.
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.
multires_image/test.geo
Outdated
image_width: 25600 | ||
image_height: 17920 | ||
tile_size: 512 | ||
image_path: "../../drones4life_scenario/launch/data/mapviz_tiles/tiles" |
There was a problem hiding this comment.
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?
@pjreed |
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.