-
-
Notifications
You must be signed in to change notification settings - Fork 510
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
grpc: Support multiple device handling #202
Conversation
grpc/server/dronecore_server.cpp.in
Outdated
bool discovered_device = false; | ||
DroneCore::ConnectionResult connection_result = dc.add_udp_connection(); | ||
uint16_t discovered_device = 0; | ||
DroneCore::ConnectionResult connection_result = dc.add_udp_connection(14550); |
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.
For now this is fine, later we need to have one config which gives connection information TCP/UDP/Serial and respective parameters.
dronecorerpc::ActionResult *response) override | ||
{ | ||
const Action::Result action_result = dc->device().action().land(); | ||
const Action::Result action_result = dc->device(request->uuid()).action().land(); |
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 think we can give option to user to send NULL if they are handling single device. Then we need a NULL check here.
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 think these changes will conflict with @JonasVautherin is doing. You two need to coordinate your work.
grpc/server/dronecore_server.cpp.in
Outdated
while (file >> plugin) { | ||
auto service_obj = map[plugin](&dc); | ||
list.push_back(service_obj); | ||
} | ||
|
||
bool discovered_device = false; | ||
DroneCore::ConnectionResult connection_result = dc.add_udp_connection(); | ||
uint16_t discovered_device = 0; |
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 don't think uint16_t
is the right type here. The UUID is uint64_t
. And otherwise, it makes sense to just use `int.
{ | ||
std::vector<uint64_t> list; | ||
list=dc->device_uuids(); | ||
for(std::vector<uint64_t>::iterator it=list.begin(); it!=list.end(); ++it) |
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 suggest you just use for (auto it : list) {
.
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 think it will be easier for me to manage the conflicts on this one. Let's maybe merge it before my PR (and let me merge mine myself, when it gets accepted of course :-)).
grpc/proto/common.proto
Outdated
@@ -0,0 +1,17 @@ | |||
syntax = "proto3"; |
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.
Doesn't it make sense to have this inside dronecore.proto
instead of creating a common.proto
service? What's the purpose of dronecore.proto
otherwise? I see it as the entrypoint for the gRPC clients. Listing devices and their available plugins should be done there, in my opinion.
What do you guys think?
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'd agree, I vote dronecore.proto
for that.
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.
Thinking about it, I am wondering if the multi-device workflow will not be like this (I refer to "dronecore.proto" for the executable that will run the corresponding gRPC server):
- dronecore.proto starts listening for devices
- when a device registers to dronecore.proto, dronecore.proto gets its uuid and (maybe?) its list of compatible plugins (dronecore.proto could also have a way to recognize the device and know the compatible plugins, maybe, e.g. through libdronecore.so)
- dronecore.proto starts the plugins for the newly-connected device
- when a (frontend) client connects to dronecore.proto, it gets the list of devices and their corresponding plugins
- maybe a client can tell dronecore.proto to start a plugin for a device (basically saying "I do know this device and I can ensure you that this plugin will work")
My point there is that those interactions will be centralized in this dronecore.proto gRPC server, and that is the sole purpose of dronecore.proto.
f598d33
to
2dc2dc4
Compare
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'd be in favor of merging this soon, so that I can work on merging #197.
Updates Dronecore Server, grpc plugin api's to consider UUID Updates Python Client examples to take UUID as a parameter for distinguishing between devices Adds a serive in dronecore plugin to fetch the list of UUID's discovered by dronecore server Adds multiple_device_handling.py to illustrate an example for handling multiple devices
2dc2dc4
to
e249767
Compare
4ae8143
to
e249767
Compare
mission_raw: remove leftover @brief tag
Updates Dronecore Server, grpc plugin api's to consider UUID
Updates Python Client examples to take UUID as a parameter for distinguishing between devices
Adds a common plugin to fetch the list of UUID's discovered by dronecore server
Adds multiple_device_handling.py to illustrate an example for handling multiple devices