Skip to content

Commit

Permalink
PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
Jon Reeves committed Dec 12, 2024
1 parent 10b2d11 commit a259e0b
Show file tree
Hide file tree
Showing 12 changed files with 52 additions and 607 deletions.
2 changes: 1 addition & 1 deletion proto
Submodule proto updated 1 files
+3 −14 protos/mission_raw_server/mission_raw_server.proto
43 changes: 0 additions & 43 deletions src/mavsdk/core/mavlink_mission_transfer_server.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,6 @@ MavlinkMissionTransferServer::receive_incoming_items_async(
uint8_t target_component,
ResultAndItemsCallback callback)
{
if (!_int_messages_supported) {
if (callback) {
LogErr() << "Int messages are not supported.";
callback(Result::IntMessagesNotSupported, {});
}
return {};
}

auto ptr = std::make_shared<ReceiveIncomingMission>(
_sender,
_message_handler,
Expand All @@ -64,14 +56,6 @@ MavlinkMissionTransferServer::send_outgoing_items_async(
uint8_t target_component,
ResultCallback callback)
{
if (!_int_messages_supported) {
if (callback) {
LogErr() << "Int messages are not supported.";
callback(Result::IntMessagesNotSupported);
}
return {};
}

auto ptr = std::make_shared<SendOutgoingMission>(
_sender,
_message_handler,
Expand Down Expand Up @@ -377,33 +361,6 @@ void MavlinkMissionTransferServer::SendOutgoingMission::start()
std::lock_guard<std::mutex> lock(_mutex);

_started = true;

for (unsigned i = 0; i < _items.size(); ++i) {
if (_items[i].seq != i) {
LogWarn() << "Invalid sequence";
callback_and_reset(Result::InvalidSequence);
return;
}
}

if (_items.size() > 0) {
int num_currents = 0;
std::for_each(_items.cbegin(), _items.cend(), [&num_currents](const ItemInt& item) {
num_currents += item.current;
});
if (num_currents != 1) {
callback_and_reset(Result::CurrentInvalid);
return;
}

if (std::any_of(_items.cbegin(), _items.cend(), [this](const ItemInt& item) {
return item.mission_type != _type;
})) {
callback_and_reset(Result::MissionTypeNotConsistent);
return;
}
}

_retries_done = 0;
_step = Step::SendCount;
_cookie = _timeout_handler.add([this]() { process_timeout(); }, _timeout_s);
Expand Down
3 changes: 0 additions & 3 deletions src/mavsdk/core/mavlink_mission_transfer_server.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -216,8 +216,6 @@ class MavlinkMissionTransferServer {
void do_work();
bool is_idle();

void set_int_messages_supported(bool supported);

// Non-copyable
MavlinkMissionTransferServer(const MavlinkMissionTransferServer&) = delete;
const MavlinkMissionTransferServer& operator=(const MavlinkMissionTransferServer&) = delete;
Expand All @@ -230,7 +228,6 @@ class MavlinkMissionTransferServer {

LockedQueue<WorkItem> _work_queue{};

bool _int_messages_supported{true};
bool _debugging{false};
};

Expand Down
147 changes: 0 additions & 147 deletions src/mavsdk/core/mavlink_mission_transfer_server_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -524,153 +524,6 @@ TEST_F(MavlinkMissionTransferServerTest, SendOutgoingMissionEmptyMission)
EXPECT_TRUE(mmt.is_idle());
}

TEST_F(MavlinkMissionTransferServerTest, SendOutgoingMissionDoesComplainAboutWrongSequence)
{
std::vector<ItemInt> items;
items.push_back(make_item(MAV_MISSION_TYPE_MISSION, 0));
items.push_back(make_item(MAV_MISSION_TYPE_MISSION, 2));

std::promise<void> prom;
auto fut = prom.get_future();

mmt.send_outgoing_items_async(
MAV_MISSION_TYPE_MISSION,
items,
target_address.system_id,
target_address.component_id,
[&prom](Result result) {
EXPECT_EQ(result, Result::InvalidSequence);
ONCE_ONLY;
prom.set_value();
});
mmt.do_work();

EXPECT_EQ(fut.wait_for(std::chrono::seconds(1)), std::future_status::ready);

mmt.do_work();
EXPECT_TRUE(mmt.is_idle());
}

TEST_F(
MavlinkMissionTransferServerTest,
SendOutgoingMissionDoesComplainAboutInconsistentMissionTypesInAPI)
{
std::vector<ItemInt> items;
items.push_back(make_item(MAV_MISSION_TYPE_FENCE, 0));
items.push_back(make_item(MAV_MISSION_TYPE_FENCE, 1));
items.push_back(make_item(MAV_MISSION_TYPE_FENCE, 2));

std::promise<void> prom;
auto fut = prom.get_future();

mmt.send_outgoing_items_async(
MAV_MISSION_TYPE_MISSION,
items,
target_address.system_id,
target_address.component_id,
[&prom](Result result) {
EXPECT_EQ(result, Result::MissionTypeNotConsistent);
ONCE_ONLY;
prom.set_value();
});
mmt.do_work();

EXPECT_EQ(fut.wait_for(std::chrono::seconds(1)), std::future_status::ready);

mmt.do_work();
EXPECT_TRUE(mmt.is_idle());
}

TEST_F(
MavlinkMissionTransferServerTest,
SendOutgoingMissionDoesComplainAboutInconsistentMissionTypesInItems)
{
std::vector<ItemInt> items;
items.push_back(make_item(MAV_MISSION_TYPE_MISSION, 0));
items.push_back(make_item(MAV_MISSION_TYPE_FENCE, 1));
items.push_back(make_item(MAV_MISSION_TYPE_MISSION, 2));

std::promise<void> prom;
auto fut = prom.get_future();

mmt.send_outgoing_items_async(
MAV_MISSION_TYPE_MISSION,
items,
target_address.system_id,
target_address.component_id,
[&prom](Result result) {
EXPECT_EQ(result, Result::MissionTypeNotConsistent);
ONCE_ONLY;
prom.set_value();
});
mmt.do_work();

EXPECT_EQ(fut.wait_for(std::chrono::seconds(1)), std::future_status::ready);

mmt.do_work();
EXPECT_TRUE(mmt.is_idle());
}

TEST_F(MavlinkMissionTransferServerTest, SendOutgoingMissionDoesComplainAboutNoCurrent)
{
std::vector<ItemInt> items;
items.push_back(make_item(MAV_MISSION_TYPE_MISSION, 0));
items.push_back(make_item(MAV_MISSION_TYPE_MISSION, 1));
items.push_back(make_item(MAV_MISSION_TYPE_MISSION, 2));
// Remove the current flag again.
items[0].current = 0;

std::promise<void> prom;
auto fut = prom.get_future();

mmt.send_outgoing_items_async(
MAV_MISSION_TYPE_MISSION,
items,
target_address.system_id,
target_address.component_id,
[&prom](Result result) {
EXPECT_EQ(result, Result::CurrentInvalid);
ONCE_ONLY;
prom.set_value();
});
mmt.do_work();

EXPECT_EQ(fut.wait_for(std::chrono::seconds(1)), std::future_status::ready);

mmt.do_work();
EXPECT_TRUE(mmt.is_idle());
}

TEST_F(MavlinkMissionTransferServerTest, SendOutgoingMissionDoesComplainAboutTwoCurrents)
{
std::vector<ItemInt> items;
items.push_back(make_item(MAV_MISSION_TYPE_MISSION, 0));
items.push_back(make_item(MAV_MISSION_TYPE_MISSION, 1));
items.push_back(make_item(MAV_MISSION_TYPE_MISSION, 2));
// Add another current.
items[1].current = 1;

std::promise<void> prom;
auto fut = prom.get_future();

mmt.send_outgoing_items_async(
MAV_MISSION_TYPE_MISSION,
items,
target_address.system_id,
target_address.component_id,
[&prom](Result result) {
EXPECT_EQ(result, Result::CurrentInvalid);
ONCE_ONLY;
prom.set_value();
});
mmt.do_work();

EXPECT_EQ(fut.wait_for(std::chrono::seconds(1)), std::future_status::ready);

mmt.do_work();
EXPECT_TRUE(mmt.is_idle());
}

TEST_F(MavlinkMissionTransferServerTest, SendOutgoingMissionDoesNotCrashIfCallbackIsNull)
{
ON_CALL(mock_sender, queue_message(_)).WillByDefault(Return(false));
Expand Down
41 changes: 0 additions & 41 deletions ...mavsdk/plugins/mission_raw_server/include/plugins/mission_raw_server/mission_raw_server.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,33 +185,6 @@ class MissionRawServer : public ServerPluginBase {
*/
void unsubscribe_incoming_mission(IncomingMissionHandle handle);

/**
* @brief Poll for 'MissionPlan' (blocking).
*
* @return One MissionPlan update.
*/
MissionPlan incoming_mission() const;

/**
* @brief Callback type for subscribe_outgoing_mission.
*/
using OutgoingMissionCallback = std::function<void(Result, MissionPlan)>;

/**
* @brief Handle type for subscribe_outgoing_mission.
*/
using OutgoingMissionHandle = Handle<Result, MissionPlan>;

/**
* @brief Subscribe to when a new mission download request completes (asynchronous)
*/
OutgoingMissionHandle subscribe_outgoing_mission(const OutgoingMissionCallback& callback);

/**
* @brief Unsubscribe from subscribe_outgoing_mission
*/
void unsubscribe_outgoing_mission(OutgoingMissionHandle handle);

/**
* @brief Callback type for subscribe_current_item_changed.
*/
Expand All @@ -233,13 +206,6 @@ class MissionRawServer : public ServerPluginBase {
*/
void unsubscribe_current_item_changed(CurrentItemChangedHandle handle);

/**
* @brief Poll for 'MissionItem' (blocking).
*
* @return One MissionItem update.
*/
MissionItem current_item_changed() const;

/**
* @brief Set Current item as completed
*
Expand Down Expand Up @@ -269,13 +235,6 @@ class MissionRawServer : public ServerPluginBase {
*/
void unsubscribe_clear_all(ClearAllHandle handle);

/**
* @brief Poll for 'uint32_t' (blocking).
*
* @return One uint32_t update.
*/
uint32_t clear_all() const;

/**
* @brief Copy constructor.
*/
Expand Down
26 changes: 0 additions & 26 deletions src/mavsdk/plugins/mission_raw_server/mission_raw_server.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,22 +32,6 @@ void MissionRawServer::unsubscribe_incoming_mission(IncomingMissionHandle handle
_impl->unsubscribe_incoming_mission(handle);
}

MissionRawServer::MissionPlan MissionRawServer::incoming_mission() const
{
return _impl->incoming_mission();
}

MissionRawServer::OutgoingMissionHandle
MissionRawServer::subscribe_outgoing_mission(const OutgoingMissionCallback& callback)
{
return _impl->subscribe_outgoing_mission(callback);
}

void MissionRawServer::unsubscribe_outgoing_mission(OutgoingMissionHandle handle)
{
_impl->unsubscribe_outgoing_mission(handle);
}

MissionRawServer::CurrentItemChangedHandle
MissionRawServer::subscribe_current_item_changed(const CurrentItemChangedCallback& callback)
{
Expand All @@ -59,11 +43,6 @@ void MissionRawServer::unsubscribe_current_item_changed(CurrentItemChangedHandle
_impl->unsubscribe_current_item_changed(handle);
}

MissionRawServer::MissionItem MissionRawServer::current_item_changed() const
{
return _impl->current_item_changed();
}

void MissionRawServer::set_current_item_complete() const
{
_impl->set_current_item_complete();
Expand All @@ -80,11 +59,6 @@ void MissionRawServer::unsubscribe_clear_all(ClearAllHandle handle)
_impl->unsubscribe_clear_all(handle);
}

uint32_t MissionRawServer::clear_all() const
{
return _impl->clear_all();
}

bool operator==(const MissionRawServer::MissionItem& lhs, const MissionRawServer::MissionItem& rhs)
{
return (rhs.seq == lhs.seq) && (rhs.frame == lhs.frame) && (rhs.command == lhs.command) &&
Expand Down
30 changes: 0 additions & 30 deletions src/mavsdk/plugins/mission_raw_server/mission_raw_server_impl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -320,24 +320,6 @@ void MissionRawServerImpl::unsubscribe_incoming_mission(
_incoming_mission_callbacks.unsubscribe(handle);
}

MissionRawServer::OutgoingMissionHandle MissionRawServerImpl::subscribe_outgoing_mission(
const MissionRawServer::OutgoingMissionCallback& callback)
{
return _outgoing_mission_callbacks.subscribe(callback);
}

void MissionRawServerImpl::unsubscribe_outgoing_mission(
MissionRawServer::OutgoingMissionHandle handle)
{
_outgoing_mission_callbacks.unsubscribe(handle);
}

MissionRawServer::MissionPlan MissionRawServerImpl::incoming_mission() const
{
// TO-DO
return {};
}

MissionRawServer::CurrentItemChangedHandle MissionRawServerImpl::subscribe_current_item_changed(
const MissionRawServer::CurrentItemChangedCallback& callback)
{
Expand All @@ -361,18 +343,6 @@ void MissionRawServerImpl::unsubscribe_clear_all(MissionRawServer::ClearAllHandl
_clear_all_callbacks.unsubscribe(handle);
}

uint32_t MissionRawServerImpl::clear_all() const
{
// TO-DO
return {};
}

MissionRawServer::MissionItem MissionRawServerImpl::current_item_changed() const
{
// TO-DO
return {};
}

void MissionRawServerImpl::set_current_item_complete()
{
if (_current_seq + 1 > _current_mission.size()) {
Expand Down
Loading

0 comments on commit a259e0b

Please sign in to comment.