From dcccd07d094d06af50b8948a57e4314771cc1638 Mon Sep 17 00:00:00 2001 From: Jonas Vautherin Date: Mon, 12 Feb 2018 10:39:57 +0100 Subject: [PATCH 1/4] dronecore_server: simplify plugins instantiation --- grpc/server/src/dronecore_server.cpp | 31 ++++++---------------------- 1 file changed, 6 insertions(+), 25 deletions(-) diff --git a/grpc/server/src/dronecore_server.cpp b/grpc/server/src/dronecore_server.cpp index f62ec671c7..af592dbe76 100644 --- a/grpc/server/src/dronecore_server.cpp +++ b/grpc/server/src/dronecore_server.cpp @@ -33,33 +33,11 @@ static DroneCore dc; template ::grpc::Service *createInstances(DroneCore *dc_obj) { return new T(dc_obj); } -typedef std::map map_type; - void RunServer() { std::string server_address("0.0.0.0:50051"); CoreServiceImpl service(&dc); - map_type map; - std::string plugin; - std::fstream file; - file.open("grpc/server/src/plugins/plugins.conf"); - - if (!file) { - LogErr() << "Error in reading conf file"; - return; - } - - std::vector<::grpc::Service *> list; - map["action"] = &createInstances; - map["telemetry"] = &createInstances; - map["mission"] = &createInstances; - - while (file >> plugin) { - auto service_obj = map[plugin](&dc); - list.push_back(service_obj); - } - int discovered_device = 0; ConnectionResult connection_result = dc.add_udp_connection(14550); if (connection_result != ConnectionResult::SUCCESS) { @@ -71,6 +49,7 @@ void RunServer() LogInfo() << "Discovered device with UUID: " << uuid; discovered_device += 1; }); + // We usually receive heartbeats at 1Hz, therefore we should find a device after around 2 seconds. std::this_thread::sleep_for(std::chrono::seconds(10)); if (discovered_device == 0) { @@ -81,9 +60,11 @@ void RunServer() ServerBuilder builder; builder.AddListeningPort(server_address, grpc::InsecureServerCredentials()); builder.RegisterService(&service); - for (auto plugin_service : list) { - builder.RegisterService(plugin_service); - } + + builder.RegisterService(createInstances(&dc)); + builder.RegisterService(createInstances(&dc)); + builder.RegisterService(createInstances(&dc)); + std::unique_ptr server(builder.BuildAndStart()); LogInfo() << "Server listening on " << server_address; server->Wait(); From 8f7b9ec48e975de82421f0931036a9b6a5e1d62f Mon Sep 17 00:00:00 2001 From: Jonas Vautherin Date: Mon, 12 Feb 2018 15:18:47 +0100 Subject: [PATCH 2/4] dronecore_server: various refactoring --- grpc/server/src/dronecore_server.cpp | 31 ++++++++++++---------------- 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/grpc/server/src/dronecore_server.cpp b/grpc/server/src/dronecore_server.cpp index af592dbe76..fd12123c66 100644 --- a/grpc/server/src/dronecore_server.cpp +++ b/grpc/server/src/dronecore_server.cpp @@ -33,34 +33,30 @@ static DroneCore dc; template ::grpc::Service *createInstances(DroneCore *dc_obj) { return new T(dc_obj); } -void RunServer() +int RunServer() { - std::string server_address("0.0.0.0:50051"); - CoreServiceImpl service(&dc); + ConnectionResult connection_result = dc.add_udp_connection(14540); - int discovered_device = 0; - ConnectionResult connection_result = dc.add_udp_connection(14550); if (connection_result != ConnectionResult::SUCCESS) { LogErr() << "Connection failed: " << connection_result_str(connection_result); - return; + return 1; } + LogInfo() << "Waiting to discover device..."; - dc.register_on_discover([&discovered_device](uint64_t uuid) { - LogInfo() << "Discovered device with UUID: " << uuid; - discovered_device += 1; + dc.register_on_discover([](uint64_t uuid) { + LogInfo() << "Device discovered [UUID: " << uuid << "]"; + }); + dc.register_on_timeout([](uint64_t uuid) { + LogInfo() << "Device timed out [UUID: " << uuid << "]"; }); - // We usually receive heartbeats at 1Hz, therefore we should find a device after around 2 seconds. - std::this_thread::sleep_for(std::chrono::seconds(10)); - if (discovered_device == 0) { - LogErr() << "No device found, exiting."; - return; - } + std::string server_address("0.0.0.0:50051"); ServerBuilder builder; builder.AddListeningPort(server_address, grpc::InsecureServerCredentials()); - builder.RegisterService(&service); + CoreServiceImpl service(&dc); + builder.RegisterService(&service); builder.RegisterService(createInstances(&dc)); builder.RegisterService(createInstances(&dc)); builder.RegisterService(createInstances(&dc)); @@ -72,6 +68,5 @@ void RunServer() int main(int argc, char **argv) { - RunServer(); - return 0; + return RunServer(); } From 6e5b783f2d51657550247eb33384fef228b62aa2 Mon Sep 17 00:00:00 2001 From: Jonas Vautherin Date: Tue, 27 Feb 2018 11:11:35 +0100 Subject: [PATCH 3/4] dronecore_server: make a library out of the grpc server * Required for some platforms (e.g. iOS) * Rename dronecore_server into libdronecore_backend and backend_bin * Use smart pointers and references instead of pointers --- core/device.cpp | 2 +- grpc/server/CMakeLists.txt | 2 +- grpc/server/src/CMakeLists.txt | 94 +++++---------- grpc/server/src/backend.cpp | 110 ++++++++++++++++++ grpc/server/src/backend.h | 33 ++++++ grpc/server/src/cmake/compile_proto.cmake | 30 +++++ grpc/server/src/core/corerpc_impl.h | 12 +- grpc/server/src/dronecore_server.cpp | 72 +----------- .../src/plugins/action/actionrpc_impl.h | 16 +-- .../src/plugins/mission/missionrpc_impl.h | 16 ++- grpc/server/src/plugins/plugins.conf | 3 - .../src/plugins/telemetry/telemetryrpc_impl.h | 12 +- plugins/mission/mission_impl.h | 3 +- 13 files changed, 237 insertions(+), 168 deletions(-) create mode 100644 grpc/server/src/backend.cpp create mode 100644 grpc/server/src/backend.h create mode 100644 grpc/server/src/cmake/compile_proto.cmake delete mode 100644 grpc/server/src/plugins/plugins.conf diff --git a/core/device.cpp b/core/device.cpp index ba6b4e478d..1f48b8b331 100644 --- a/core/device.cpp +++ b/core/device.cpp @@ -387,7 +387,7 @@ void Device::set_disconnected() _parent->notify_on_timeout(_target_uuid); // Let's reset the flag hope again for the next time we see this target. - _target_uuid_initialized = 0; + _target_uuid_initialized = false; } { diff --git a/grpc/server/CMakeLists.txt b/grpc/server/CMakeLists.txt index 04abf32d94..01d66c13a2 100644 --- a/grpc/server/CMakeLists.txt +++ b/grpc/server/CMakeLists.txt @@ -1,4 +1,4 @@ -cmake_minimum_required(VERSION 2.8.12) +cmake_minimum_required(VERSION 3.1) project(dronecore-server) diff --git a/grpc/server/src/CMakeLists.txt b/grpc/server/src/CMakeLists.txt index 3bdb2e92d2..02e52b7da0 100644 --- a/grpc/server/src/CMakeLists.txt +++ b/grpc/server/src/CMakeLists.txt @@ -1,67 +1,33 @@ -cmake_minimum_required(VERSION 2.8) +cmake_minimum_required(VERSION 3.1) -file(STRINGS ${PLUGINS_DIR}/plugins.conf PLUGINS_LIST) +set(COMPONENTS_LIST core action mission telemetry) -set(PROTOC_BINARY ${CMAKE_BINARY_DIR}/../default/third_party/protobuf/bin/protoc) -set(GRPC_CPP_PLUGIN_BINARY ${CMAKE_BINARY_DIR}/../default/third_party/grpc/bin/grpc_cpp_plugin) +include(cmake/compile_proto.cmake) -if(NOT EXISTS ${PROTOC_BINARY} OR NOT EXISTS ${GRPC_CPP_PLUGIN_BINARY}) - message(FATAL_ERROR "Could not find 'protoc' or 'grpc_cpp_plugin' in the 'default' build folder. Please build for your host first (`make BUILD_DRONECORESERVER=YES default`).") -endif() +foreach(COMPONENT_NAME ${COMPONENTS_LIST}) + compile_proto_pb(${COMPONENT_NAME} PB_COMPILED_SOURCE) + list(APPEND PB_COMPILED_SOURCES ${PB_COMPILED_SOURCE}) -add_custom_command(OUTPUT core/core.grpc.pb.cc - COMMAND ${PROTOC_BINARY} - -I ${PROTO_DIR} - --grpc_out=. - --plugin=protoc-gen-grpc=${GRPC_CPP_PLUGIN_BINARY} - ${PROTO_DIR}/core/core.proto; -) - -add_custom_command(OUTPUT core/core.pb.cc - COMMAND ${PROTOC_BINARY} - -I ${PROTO_DIR} - --cpp_out=. - ${PROTO_DIR}/core/core.proto -) - -foreach(PLUGIN ${PLUGINS_LIST}) - add_custom_command(OUTPUT ${PLUGIN}/${PLUGIN}.grpc.pb.cc - COMMAND ${PROTOC_BINARY} - -I ${PROTO_DIR} - --grpc_out=. - --plugin=protoc-gen-grpc=${GRPC_CPP_PLUGIN_BINARY} - --cpp_out=. - ${PROTO_DIR}/${PLUGIN}/${PLUGIN}.proto - ) -endforeach() - -foreach(PLUGIN ${PLUGINS_LIST}) - add_custom_command(OUTPUT ${PLUGIN}/${PLUGIN}.pb.cc - COMMAND ${PROTOC_BINARY} - -I ${PROTO_DIR} - --cpp_out=. - ${PROTO_DIR}/${PLUGIN}/${PLUGIN}.proto - ) + compile_proto_grpc(${COMPONENT_NAME} GRPC_COMPILED_SOURCE) + list(APPEND GRPC_COMPILED_SOURCES ${GRPC_COMPILED_SOURCE}) endforeach() -set(PLUGINS_SRC dronecore_server.cpp core/core.pb.cc core/core.grpc.pb.cc) -foreach(PLUGIN ${PLUGINS_LIST}) - list(APPEND PLUGINS_SRC ${PLUGIN}/${PLUGIN}.pb.cc) - list(APPEND PLUGINS_SRC ${PLUGIN}/${PLUGIN}.grpc.pb.cc) -endforeach() - -add_executable(dronecore_server ${PLUGINS_SRC}) +add_library(backend + backend.cpp + ${GRPC_COMPILED_SOURCES} + ${PB_COMPILED_SOURCES} +) -target_compile_options( - dronecore_server - PUBLIC - -Wno-unused-parameter - -Wno-shadow +target_link_libraries(backend + dronecore + dronecore_action + dronecore_mission + dronecore_telemetry + gRPC::grpc++ ) -target_include_directories( - dronecore_server - PUBLIC +target_include_directories(backend + PRIVATE ${CMAKE_SOURCE_DIR}/core ${CMAKE_SOURCE_DIR}/plugins/action ${CMAKE_SOURCE_DIR}/plugins/mission @@ -70,12 +36,16 @@ target_include_directories( ${PLUGINS_DIR} ) -target_link_libraries( - dronecore_server +add_executable(backend_bin + dronecore_server.cpp +) + +target_link_libraries(backend_bin + backend dronecore - dronecore_action - dronecore_mission - dronecore_telemetry - gRPC::grpc++ - dl +) + +target_include_directories(backend_bin + PRIVATE + ${CMAKE_SOURCE_DIR}/core ) diff --git a/grpc/server/src/backend.cpp b/grpc/server/src/backend.cpp new file mode 100644 index 0000000000..1ef86dedb9 --- /dev/null +++ b/grpc/server/src/backend.cpp @@ -0,0 +1,110 @@ +#include "backend.h" + +#include +#include +#include +#include +#include + +#include "action/actionrpc_impl.h" +#include "core/corerpc_impl.h" +#include "log.h" +#include "mission/missionrpc_impl.h" +#include "telemetry/telemetryrpc_impl.h" + +namespace dronecore { +namespace backend { + +bool DroneCoreBackend::run(const int mavlink_listen_port) +{ + if (!connect_to_vehicle(mavlink_listen_port)) { + return false; + } + + grpc::ServerBuilder builder; + setup_port(builder); + + CoreServiceImpl core(dc); + builder.RegisterService(&core); + + Action action(&dc.device()); + ActionServiceImpl actionService(action); + builder.RegisterService(&actionService); + + Mission mission(&dc.device()); + MissionServiceImpl missionService(mission); + builder.RegisterService(&missionService); + + Telemetry telemetry(&dc.device()); + TelemetryServiceImpl telemetryService(telemetry); + builder.RegisterService(&telemetryService); + + server = builder.BuildAndStart(); + LogInfo() << "Server started"; + server->Wait(); + + return true; +} + +bool DroneCoreBackend::connect_to_vehicle(const int port) +{ + if (!add_udp_connection(port)) { + return false; + } + + log_uuid_on_timeout(); + wait_for_discovery(); + + return true; +} + +bool DroneCoreBackend::add_udp_connection(const int port) +{ + dronecore::ConnectionResult connection_result = dc.add_udp_connection(port); + + if (connection_result != ConnectionResult::SUCCESS) { + LogErr() << "Connection failed: " << connection_result_str(connection_result); + return false; + } + + return true; +} + +void DroneCoreBackend::log_uuid_on_timeout() +{ + dc.register_on_timeout([](uint64_t uuid) { + LogInfo() << "Device timed out [UUID: " << uuid << "]"; + }); +} + +void DroneCoreBackend::wait_for_discovery() +{ + LogInfo() << "Waiting to discover device..."; + auto discoveryFuture = wrapped_register_on_discover(); + discoveryFuture.wait(); + LogInfo() << "Device discovered [UUID: " << discoveryFuture.get() << "]"; +} + +std::future DroneCoreBackend::wrapped_register_on_discover() +{ + auto promise = std::make_shared>(); + auto future = promise->get_future(); + + dc.register_on_discover([this, promise](uint64_t uuid) { + std::call_once(discovery_flag, [promise, uuid]() { + promise->set_value(uuid); + }); + }); + + return future; +} + +void DroneCoreBackend::setup_port(grpc::ServerBuilder &builder) +{ + std::string server_address("0.0.0.0:50051"); + builder.AddListeningPort(server_address, grpc::InsecureServerCredentials()); + LogInfo() << "Server set to listen on " << server_address; +} + +} // namespace backend +} //namespace dronecore diff --git a/grpc/server/src/backend.h b/grpc/server/src/backend.h new file mode 100644 index 0000000000..01278d8694 --- /dev/null +++ b/grpc/server/src/backend.h @@ -0,0 +1,33 @@ +#include +#include +#include + +#include "dronecore.h" + +namespace dronecore { +namespace backend { + +class DroneCoreBackend +{ +public: + DroneCoreBackend() {} + ~DroneCoreBackend() {} + + bool run(const int mavlink_listen_port = 14540); + +private: + bool connect_to_vehicle(int port); + bool add_udp_connection(int port); + void log_uuid_on_timeout(); + void wait_for_discovery(); + std::future wrapped_register_on_discover(); + + bool run_server(); + void setup_port(grpc::ServerBuilder &builder); + + dronecore::DroneCore dc; + std::unique_ptr server; +}; + +} // namespace backend +} // namespace dronecore diff --git a/grpc/server/src/cmake/compile_proto.cmake b/grpc/server/src/cmake/compile_proto.cmake new file mode 100644 index 0000000000..b6d2356ee3 --- /dev/null +++ b/grpc/server/src/cmake/compile_proto.cmake @@ -0,0 +1,30 @@ +set(PROTOC_BINARY ${CMAKE_BINARY_DIR}/../default/third_party/protobuf/bin/protoc) +set(GRPC_CPP_PLUGIN_BINARY ${CMAKE_BINARY_DIR}/../default/third_party/grpc/bin/grpc_cpp_plugin) + +if(NOT EXISTS ${PROTOC_BINARY} OR NOT EXISTS ${GRPC_CPP_PLUGIN_BINARY}) + message(FATAL_ERROR "Could not find 'protoc' or 'grpc_cpp_plugin' in the 'default' build folder. Please build for your host first (`make BUILD_DRONECORESERVER=YES default`).") +endif() + +function(compile_proto_pb COMPONENT_NAME PB_COMPILED_SOURCE) + add_custom_command(OUTPUT ${COMPONENT_NAME}/${COMPONENT_NAME}.pb.cc + COMMAND ${PROTOC_BINARY} + -I ${PROTO_DIR} + --cpp_out=. + ${PROTO_DIR}/${COMPONENT_NAME}/${COMPONENT_NAME}.proto + ) + + set(PB_COMPILED_SOURCE ${COMPONENT_NAME}/${COMPONENT_NAME}.pb.cc PARENT_SCOPE) +endfunction() + +function(compile_proto_grpc COMPONENT_NAME GRPC_COMPILED_SOURCES) + add_custom_command(OUTPUT ${COMPONENT_NAME}/${COMPONENT_NAME}.grpc.pb.cc + COMMAND ${PROTOC_BINARY} + -I ${PROTO_DIR} + --grpc_out=. + --plugin=protoc-gen-grpc=${GRPC_CPP_PLUGIN_BINARY} + --cpp_out=. + ${PROTO_DIR}/${COMPONENT_NAME}/${COMPONENT_NAME}.proto + ) + + set(GRPC_COMPILED_SOURCE ${COMPONENT_NAME}/${COMPONENT_NAME}.grpc.pb.cc PARENT_SCOPE) +endfunction() diff --git a/grpc/server/src/core/corerpc_impl.h b/grpc/server/src/core/corerpc_impl.h index 798f12e972..0cd5f91707 100644 --- a/grpc/server/src/core/corerpc_impl.h +++ b/grpc/server/src/core/corerpc_impl.h @@ -9,18 +9,16 @@ using namespace dronecore; class CoreServiceImpl final: public rpc::core::CoreService::Service { public: - CoreServiceImpl(DroneCore *dc_obj) - { - dc = dc_obj; - } + CoreServiceImpl(DroneCore &dc) + : dc(dc) {} Status SubscribeDevices(ServerContext *context, const rpc::core::SubscribeDevicesRequest *request, ServerWriter *writer) override { - std::vector list = dc->device_uuids(); + std::vector list = dc.device_uuids(); - for (auto uuid : dc->device_uuids()) { + for (auto uuid : dc.device_uuids()) { auto *rpc_uuid = new rpc::core::UUID(); rpc_uuid->set_value(uuid); @@ -34,5 +32,5 @@ class CoreServiceImpl final: public rpc::core::CoreService::Service } private: - DroneCore *dc; + DroneCore &dc; }; diff --git a/grpc/server/src/dronecore_server.cpp b/grpc/server/src/dronecore_server.cpp index fd12123c66..63cafbb014 100644 --- a/grpc/server/src/dronecore_server.cpp +++ b/grpc/server/src/dronecore_server.cpp @@ -1,72 +1,12 @@ -#include -#include -#include -#include -#include -#include +#include "backend.h" -#include -#include -#include -#include -#include - -#include "dronecore.h" -#include "log.h" -#include "action/actionrpc_impl.h" -#include "core/core.grpc.pb.h" -#include "core/corerpc_impl.h" -#include "mission/missionrpc_impl.h" -#include "telemetry/telemetryrpc_impl.h" - -using grpc::Server; -using grpc::ServerBuilder; -using grpc::ServerContext; -using grpc::ServerReader; -using grpc::ServerReaderWriter; -using grpc::Status; - -using namespace dronecore; -using namespace std::placeholders; - -static DroneCore dc; - -template ::grpc::Service *createInstances(DroneCore *dc_obj) { return new T(dc_obj); } - -int RunServer() +int main(int argc, char **argv) { - ConnectionResult connection_result = dc.add_udp_connection(14540); + dronecore::backend::DroneCoreBackend backend; - if (connection_result != ConnectionResult::SUCCESS) { - LogErr() << "Connection failed: " << connection_result_str(connection_result); + if (!backend.run()) { return 1; + } else { + return 0; } - - LogInfo() << "Waiting to discover device..."; - dc.register_on_discover([](uint64_t uuid) { - LogInfo() << "Device discovered [UUID: " << uuid << "]"; - }); - dc.register_on_timeout([](uint64_t uuid) { - LogInfo() << "Device timed out [UUID: " << uuid << "]"; - }); - - std::string server_address("0.0.0.0:50051"); - - ServerBuilder builder; - builder.AddListeningPort(server_address, grpc::InsecureServerCredentials()); - - CoreServiceImpl service(&dc); - builder.RegisterService(&service); - builder.RegisterService(createInstances(&dc)); - builder.RegisterService(createInstances(&dc)); - builder.RegisterService(createInstances(&dc)); - - std::unique_ptr server(builder.BuildAndStart()); - LogInfo() << "Server listening on " << server_address; - server->Wait(); -} - -int main(int argc, char **argv) -{ - return RunServer(); } diff --git a/grpc/server/src/plugins/action/actionrpc_impl.h b/grpc/server/src/plugins/action/actionrpc_impl.h index 31b142fa61..990d7ca177 100644 --- a/grpc/server/src/plugins/action/actionrpc_impl.h +++ b/grpc/server/src/plugins/action/actionrpc_impl.h @@ -9,16 +9,13 @@ using namespace dronecore; class ActionServiceImpl final : public rpc::action::ActionService::Service { public: - ActionServiceImpl(DroneCore *dc_obj) - { - dc = dc_obj; - action = std::make_shared(&dc->device()); - } + ActionServiceImpl(const Action &action) + : action(action) {} Status Arm(ServerContext *context, const rpc::action::ArmRequest *request, rpc::action::ActionResult *response) override { - const Action::Result action_result = action->arm(); + const Action::Result action_result = action.arm(); response->set_result(static_cast(action_result)); response->set_result_str(Action::result_str(action_result)); return Status::OK; @@ -27,7 +24,7 @@ class ActionServiceImpl final : public rpc::action::ActionService::Service Status TakeOff(ServerContext *context, const rpc::action::TakeOffRequest *request, rpc::action::ActionResult *response) override { - const Action::Result action_result = action->takeoff(); + const Action::Result action_result = action.takeoff(); response->set_result(static_cast(action_result)); response->set_result_str(Action::result_str(action_result)); return Status::OK; @@ -36,13 +33,12 @@ class ActionServiceImpl final : public rpc::action::ActionService::Service Status Land(ServerContext *context, const rpc::action::LandRequest *request, rpc::action::ActionResult *response) override { - const Action::Result action_result = action->land(); + const Action::Result action_result = action.land(); response->set_result(static_cast(action_result)); response->set_result_str(Action::result_str(action_result)); return Status::OK; } private: - DroneCore *dc; - std::shared_ptr action; + const Action &action; }; diff --git a/grpc/server/src/plugins/mission/missionrpc_impl.h b/grpc/server/src/plugins/mission/missionrpc_impl.h index 6711c4e6b8..cb0ffa4409 100644 --- a/grpc/server/src/plugins/mission/missionrpc_impl.h +++ b/grpc/server/src/plugins/mission/missionrpc_impl.h @@ -1,3 +1,5 @@ +#include + #include "mission.h" #include "mission/mission.grpc.pb.h" @@ -9,11 +11,8 @@ using namespace dronecore; class MissionServiceImpl final : public rpc::mission::MissionService::Service { public: - MissionServiceImpl(DroneCore *dc_obj) - { - dc = dc_obj; - mission = std::make_shared(&dc->device()); - } + MissionServiceImpl(Mission &mission) + : mission(mission) {} Status SendMission(ServerContext *context, const rpc::mission::SendMissionRequest *request, rpc::mission::MissionResult *response) override @@ -36,7 +35,7 @@ class MissionServiceImpl final : public rpc::mission::MissionService::Service mission_items.push_back(new_item); } - mission->upload_mission_async( + mission.upload_mission_async( mission_items, [prom, response](Mission::Result result) { response->set_result(static_cast(result)); response->set_result_str(Mission::result_str(result)); @@ -56,7 +55,7 @@ class MissionServiceImpl final : public rpc::mission::MissionService::Service auto prom = std::make_shared>(); auto future_result = prom->get_future(); - mission->start_mission_async( + mission.start_mission_async( [prom, response](Mission::Result result) { response->set_result(static_cast(result)); response->set_result_str(Mission::result_str(result)); @@ -70,6 +69,5 @@ class MissionServiceImpl final : public rpc::mission::MissionService::Service } private: - DroneCore *dc; - std::shared_ptr mission; + Mission &mission; }; diff --git a/grpc/server/src/plugins/plugins.conf b/grpc/server/src/plugins/plugins.conf deleted file mode 100644 index 9c48ef396d..0000000000 --- a/grpc/server/src/plugins/plugins.conf +++ /dev/null @@ -1,3 +0,0 @@ -action -telemetry -mission diff --git a/grpc/server/src/plugins/telemetry/telemetryrpc_impl.h b/grpc/server/src/plugins/telemetry/telemetryrpc_impl.h index 6d64535796..dacb0b00e2 100644 --- a/grpc/server/src/plugins/telemetry/telemetryrpc_impl.h +++ b/grpc/server/src/plugins/telemetry/telemetryrpc_impl.h @@ -10,17 +10,14 @@ using namespace dronecore; class TelemetryServiceImpl final : public rpc::telemetry::TelemetryService::Service { public: - TelemetryServiceImpl(DroneCore *dc_obj) - { - dc = dc_obj; - telemetry = std::make_shared(&dc->device()); - } + TelemetryServiceImpl(Telemetry &telemetry) + : telemetry(telemetry) {} Status SubscribePosition(ServerContext *context, const rpc::telemetry::SubscribePositionRequest *request, ServerWriter *writer) override { - telemetry->position_async([&writer]( + telemetry.position_async([&writer]( Telemetry::Position position) { rpc::telemetry::Position rpc_position; rpc_position.set_latitude_deg(position.latitude_deg); @@ -38,6 +35,5 @@ class TelemetryServiceImpl final : public rpc::telemetry::TelemetryService::Serv } private: - DroneCore *dc; - std::shared_ptr telemetry; + Telemetry &telemetry; }; diff --git a/plugins/mission/mission_impl.h b/plugins/mission/mission_impl.h index fdc51ae3c9..d38211ed9d 100644 --- a/plugins/mission/mission_impl.h +++ b/plugins/mission/mission_impl.h @@ -3,9 +3,10 @@ #include #include #include + #include "device.h" -#include "mission.h" #include "mavlink_include.h" +#include "mission.h" #include "plugin_impl_base.h" #include From 81392467778117cb3524ca7e908567468deeae8c Mon Sep 17 00:00:00 2001 From: Jonas Vautherin Date: Thu, 22 Feb 2018 18:47:58 +0100 Subject: [PATCH 4/4] backend: add connection_initiator and corresponding tests --- CMakeLists.txt | 23 +++-- cmake/unit_tests.cmake | 4 +- core/mocks/dronecore_mock.h | 19 +++++ grpc/server/CMakeLists.txt | 4 + grpc/server/src/backend.cpp | 70 ++-------------- grpc/server/src/backend.h | 14 ++-- grpc/server/src/connection_initiator.h | 82 ++++++++++++++++++ grpc/server/test/CMakeLists.txt | 21 +++++ .../server/test/connection_initiator_test.cpp | 84 +++++++++++++++++++ 9 files changed, 239 insertions(+), 82 deletions(-) create mode 100644 core/mocks/dronecore_mock.h create mode 100644 grpc/server/src/connection_initiator.h create mode 100644 grpc/server/test/CMakeLists.txt create mode 100644 grpc/server/test/connection_initiator_test.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 6164e045b9..c2629a2976 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -7,10 +7,17 @@ list(APPEND CMAKE_FIND_ROOT_PATH ${CMAKE_BINARY_DIR}/third_party) project(dronecore) +option(BUILD_TESTS "Build tests" ON) + include(cmake/compiler_flags.cmake) include(cmake/zlib.cmake) include(cmake/curl.cmake) +if(BUILD_TESTS AND (IOS OR ANDROID)) + message(STATUS "Building for iOS or Android: forcing BUILD_TESTS to FALSE...") + set(BUILD_TESTS OFF) +endif() + if(ANDROID) set(lib_path "lib/android/${ANDROID_ABI}") elseif(IOS) @@ -31,14 +38,7 @@ if (DEFINED EXTERNAL_DIR AND NOT EXTERNAL_DIR STREQUAL "") include_directories(${EXTERNAL_DIR}) endif() -if (CMAKE_BUILD_DRONECORESERVER) - message(STATUS "Building dronecore server") - add_subdirectory(grpc/server) -else() - message(STATUS "BUILD_DRONECORESERVER not set: not building dronecore server") -endif() - -if(NOT IOS AND NOT ANDROID) +if(BUILD_TESTS) enable_testing() add_subdirectory(${CMAKE_SOURCE_DIR}/third_party/gtest EXCLUDE_FROM_ALL) @@ -52,6 +52,13 @@ if(NOT IOS AND NOT ANDROID) include(cmake/unit_tests.cmake) endif() +if (CMAKE_BUILD_DRONECORESERVER) + message(STATUS "Building dronecore server") + add_subdirectory(grpc/server) +else() + message(STATUS "BUILD_DRONECORESERVER not set: not building dronecore server") +endif() + if (DROP_DEBUG EQUAL 1) add_definitions(-DDROP_DEBUG=${DROP_DEBUG}) diff --git a/cmake/unit_tests.cmake b/cmake/unit_tests.cmake index 4d059d6971..f8374a0a7c 100644 --- a/cmake/unit_tests.cmake +++ b/cmake/unit_tests.cmake @@ -20,6 +20,4 @@ target_link_libraries(unit_tests_runner gmock ) -add_test(unit_tests - unit_tests_runner -) +add_test(unit_tests unit_tests_runner) diff --git a/core/mocks/dronecore_mock.h b/core/mocks/dronecore_mock.h new file mode 100644 index 0000000000..419ee136e0 --- /dev/null +++ b/core/mocks/dronecore_mock.h @@ -0,0 +1,19 @@ +#include + +#include "connection_result.h" + +namespace dronecore { +namespace testing { + +typedef std::function event_callback_t; + +class MockDroneCore +{ +public: + MOCK_CONST_METHOD1(add_udp_connection, ConnectionResult(int local_port_number)); + MOCK_CONST_METHOD1(register_on_discover, void(event_callback_t)); + MOCK_CONST_METHOD1(register_on_timeout, void(event_callback_t)); +}; + +} // testing +} // dronecore diff --git a/grpc/server/CMakeLists.txt b/grpc/server/CMakeLists.txt index 01d66c13a2..33dd2e7033 100644 --- a/grpc/server/CMakeLists.txt +++ b/grpc/server/CMakeLists.txt @@ -9,3 +9,7 @@ include(GNUInstallDirs) include(cmake/helpers/build_external.cmake) add_subdirectory(src) + +if(BUILD_TESTS) + add_subdirectory(test) +endif() diff --git a/grpc/server/src/backend.cpp b/grpc/server/src/backend.cpp index 1ef86dedb9..bcb51b236b 100644 --- a/grpc/server/src/backend.cpp +++ b/grpc/server/src/backend.cpp @@ -17,88 +17,34 @@ namespace backend { bool DroneCoreBackend::run(const int mavlink_listen_port) { - if (!connect_to_vehicle(mavlink_listen_port)) { - return false; - } + _connection_initiator.start(_dc, 14540); + _connection_initiator.wait(); grpc::ServerBuilder builder; setup_port(builder); - CoreServiceImpl core(dc); + CoreServiceImpl core(_dc); builder.RegisterService(&core); - Action action(&dc.device()); + Action action(&_dc.device()); ActionServiceImpl actionService(action); builder.RegisterService(&actionService); - Mission mission(&dc.device()); + Mission mission(&_dc.device()); MissionServiceImpl missionService(mission); builder.RegisterService(&missionService); - Telemetry telemetry(&dc.device()); + Telemetry telemetry(&_dc.device()); TelemetryServiceImpl telemetryService(telemetry); builder.RegisterService(&telemetryService); - server = builder.BuildAndStart(); + _server = builder.BuildAndStart(); LogInfo() << "Server started"; - server->Wait(); + _server->Wait(); return true; } -bool DroneCoreBackend::connect_to_vehicle(const int port) -{ - if (!add_udp_connection(port)) { - return false; - } - - log_uuid_on_timeout(); - wait_for_discovery(); - - return true; -} - -bool DroneCoreBackend::add_udp_connection(const int port) -{ - dronecore::ConnectionResult connection_result = dc.add_udp_connection(port); - - if (connection_result != ConnectionResult::SUCCESS) { - LogErr() << "Connection failed: " << connection_result_str(connection_result); - return false; - } - - return true; -} - -void DroneCoreBackend::log_uuid_on_timeout() -{ - dc.register_on_timeout([](uint64_t uuid) { - LogInfo() << "Device timed out [UUID: " << uuid << "]"; - }); -} - -void DroneCoreBackend::wait_for_discovery() -{ - LogInfo() << "Waiting to discover device..."; - auto discoveryFuture = wrapped_register_on_discover(); - discoveryFuture.wait(); - LogInfo() << "Device discovered [UUID: " << discoveryFuture.get() << "]"; -} - -std::future DroneCoreBackend::wrapped_register_on_discover() -{ - auto promise = std::make_shared>(); - auto future = promise->get_future(); - - dc.register_on_discover([this, promise](uint64_t uuid) { - std::call_once(discovery_flag, [promise, uuid]() { - promise->set_value(uuid); - }); - }); - - return future; -} - void DroneCoreBackend::setup_port(grpc::ServerBuilder &builder) { std::string server_address("0.0.0.0:50051"); diff --git a/grpc/server/src/backend.h b/grpc/server/src/backend.h index 01278d8694..cf94a20adc 100644 --- a/grpc/server/src/backend.h +++ b/grpc/server/src/backend.h @@ -1,7 +1,8 @@ -#include #include #include +#include +#include "connection_initiator.h" #include "dronecore.h" namespace dronecore { @@ -16,17 +17,12 @@ class DroneCoreBackend bool run(const int mavlink_listen_port = 14540); private: - bool connect_to_vehicle(int port); - bool add_udp_connection(int port); - void log_uuid_on_timeout(); - void wait_for_discovery(); - std::future wrapped_register_on_discover(); - bool run_server(); void setup_port(grpc::ServerBuilder &builder); - dronecore::DroneCore dc; - std::unique_ptr server; + dronecore::DroneCore _dc; + dronecore::backend::ConnectionInitiator _connection_initiator; + std::unique_ptr _server; }; } // namespace backend diff --git a/grpc/server/src/connection_initiator.h b/grpc/server/src/connection_initiator.h new file mode 100644 index 0000000000..dc48a81ede --- /dev/null +++ b/grpc/server/src/connection_initiator.h @@ -0,0 +1,82 @@ +#include + +#include "connection_result.h" +#include "log.h" + +namespace dronecore { +namespace backend { + +template +class ConnectionInitiator +{ +public: + ConnectionInitiator() {} + ~ConnectionInitiator() {} + + bool start(DroneCore &dc, const int port) + { + init_mutex(); + init_timeout_logging(dc); + + _discovery_future = wrapped_register_on_discover(dc); + + if (!add_udp_connection(dc, port)) { + return false; + } + + return true; + } + + void wait() + { + _discovery_future.wait(); + } + +private: + void init_mutex() + { + _discovery_promise = std::make_shared>(); + } + + void init_timeout_logging(DroneCore &dc) const + { + dc.register_on_timeout([](uint64_t uuid) { + LogInfo() << "Device timed out [UUID: " << uuid << "]"; + }); + } + + bool add_udp_connection(DroneCore &dc, const int port) + { + dronecore::ConnectionResult connection_result = dc.add_udp_connection(port); + + if (connection_result != ConnectionResult::SUCCESS) { + LogErr() << "Connection failed: " << connection_result_str(connection_result); + return false; + } + + return true; + } + + std::future wrapped_register_on_discover(DroneCore &dc) + { + auto future = _discovery_promise->get_future(); + + LogInfo() << "Waiting to discover device..."; + + dc.register_on_discover([this](uint64_t uuid) { + std::call_once(_discovery_flag, [this, uuid]() { + LogInfo() << "Device discovered [UUID: " << uuid << "]"; + _discovery_promise->set_value(uuid); + }); + }); + + return future; + } + + std::once_flag _discovery_flag; + std::shared_ptr> _discovery_promise; + std::future _discovery_future; +}; + +} // backend +} // dronecore diff --git a/grpc/server/test/CMakeLists.txt b/grpc/server/test/CMakeLists.txt new file mode 100644 index 0000000000..5ccb79f917 --- /dev/null +++ b/grpc/server/test/CMakeLists.txt @@ -0,0 +1,21 @@ +cmake_minimum_required(VERSION 3.1) + +add_executable(unit_tests_backend + connection_initiator_test.cpp +) + +set_target_properties(unit_tests_backend PROPERTIES COMPILE_FLAGS ${warnings}) + +target_include_directories(unit_tests_backend + PRIVATE + ${CMAKE_CURRENT_SOURCE_DIR}/../src +) + +target_link_libraries(unit_tests_backend + backend + gtest + gmock + gmock_main +) + +add_test(unit_tests unit_tests_backend) diff --git a/grpc/server/test/connection_initiator_test.cpp b/grpc/server/test/connection_initiator_test.cpp new file mode 100644 index 0000000000..7bd1983550 --- /dev/null +++ b/grpc/server/test/connection_initiator_test.cpp @@ -0,0 +1,84 @@ +#include +#include + +#include "connection_initiator.h" +#include "mocks/dronecore_mock.h" + +using testing::_; +using testing::NiceMock; + +using event_callback_t = dronecore::testing::event_callback_t; +using MockDroneCore = NiceMock; +using ConnectionInitiator = dronecore::backend::ConnectionInitiator; + +static constexpr auto ARBITRARY_PORT = 1291; +static constexpr auto ARBITRARY_UUID = 1492; + +ACTION_P(SaveCallback, event_callback) +{ + *event_callback = arg0; +} + +TEST(ConnectionInitiator, registerDiscoverIsCalledExactlyOnce) +{ + ConnectionInitiator initiator; + MockDroneCore dc; + EXPECT_CALL(dc, register_on_discover(_)) + .Times(1); + + initiator.start(dc, ARBITRARY_PORT); +} + +TEST(ConnectionInitiator, startAddsUDPConnection) +{ + ConnectionInitiator initiator; + MockDroneCore dc; + + EXPECT_CALL(dc, add_udp_connection(_)); + + initiator.start(dc, ARBITRARY_PORT); +} + +TEST(ConnectionInitiator, startHangsUntilDeviceDiscovered) +{ + ConnectionInitiator initiator; + MockDroneCore dc; + event_callback_t discover_callback; + EXPECT_CALL(dc, register_on_discover(_)) + .WillOnce(SaveCallback(&discover_callback)); + + auto async_future = std::async(std::launch::async, [&initiator, &dc]() { + initiator.start(dc, ARBITRARY_PORT); + initiator.wait(); + }); + + EXPECT_TRUE(async_future.wait_for(std::chrono::milliseconds(100)) == std::future_status::timeout) << + "Call to 'start(...)' should hang until 'discover_callback(...)' is actually called!"; + discover_callback(ARBITRARY_UUID); +} + +TEST(ConnectionInitiator, connectionDetectedIfDiscoverCallbackCalledBeforeWait) +{ + ConnectionInitiator initiator; + MockDroneCore dc; + event_callback_t discover_callback; + EXPECT_CALL(dc, register_on_discover(_)) + .WillOnce(SaveCallback(&discover_callback)); + + initiator.start(dc, ARBITRARY_PORT); + discover_callback(ARBITRARY_UUID); + initiator.wait(); +} + +TEST(ConnectionInitiator, doesNotCrashIfDiscoverCallbackCalledMoreThanOnce) +{ + ConnectionInitiator initiator; + MockDroneCore dc; + event_callback_t discover_callback; + EXPECT_CALL(dc, register_on_discover(_)) + .WillOnce(SaveCallback(&discover_callback)); + + initiator.start(dc, ARBITRARY_PORT); + discover_callback(ARBITRARY_UUID); + discover_callback(ARBITRARY_UUID); +}