Skip to content

Commit

Permalink
Merge pull request #2300 from mavlink/pr-lockup-fixes
Browse files Browse the repository at this point in the history
core: deadlock fix, remove empty system, relax version request
  • Loading branch information
julianoes authored May 14, 2024
2 parents 8932698 + 04dff58 commit e815b8b
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 28 deletions.
19 changes: 1 addition & 18 deletions src/mavsdk/core/mavsdk_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -372,19 +372,6 @@ void MavsdkImpl::receive_message(mavlink_message_t& message, Connection* connect

std::lock_guard<std::recursive_mutex> lock(_systems_mutex);

// The only situation where we create a system with sysid 0 is when we initialize the connection
// to the remote.
if (_systems.size() == 1 && _systems[0].first == 0) {
LogDebug() << "New: System ID: " << static_cast<int>(message.sysid)
<< " Comp ID: " << static_cast<int>(message.compid);
_systems[0].first = message.sysid;
_systems[0].second->system_impl()->set_system_id(message.sysid);

// Even though the fake system was already discovered, we can now
// send a notification, now that it seems to really actually exist.
notify_on_discover();
}

bool found_system = false;
for (auto& system : _systems) {
if (system.first == message.sysid) {
Expand Down Expand Up @@ -559,12 +546,8 @@ std::pair<ConnectionResult, Mavsdk::ConnectionHandle> MavsdkImpl::setup_udp_remo
new_conn->add_remote(remote_ip, remote_port);
auto handle = add_connection(new_conn);
std::lock_guard<std::recursive_mutex> lock(_systems_mutex);
if (_systems.empty()) {
make_system_with_component(0, 0);
}

// With a UDP remote, we need to initiate the connection by sending
// heartbeats.
// With a UDP remote, we need to initiate the connection by sending heartbeats.
auto new_configuration = get_configuration();
new_configuration.set_always_send_heartbeats(true);
set_configuration(new_configuration);
Expand Down
15 changes: 6 additions & 9 deletions src/mavsdk/core/system_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -504,8 +504,8 @@ void SystemImpl::send_autopilot_version_request()

if (fut.get() == MavlinkCommandSender::Result::Unsupported) {
_old_message_520_supported = false;
LogWarn() << "Trying alternative command (512).";
send_autopilot_version_request();
LogWarn()
<< "Trying alternative command REQUEST_MESSAGE instead of REQUEST_AUTOPILOT_CAPABILITIES next.";
}
}

Expand Down Expand Up @@ -541,12 +541,6 @@ void SystemImpl::set_connected()

_connected = true;

// System with sysid 0 is a bit special: it is a placeholder for a connection initiated
// by MAVSDK. As such, it should not be advertised as a newly discovered system.
if (static_cast<int>(get_system_id()) != 0) {
_mavsdk_impl.notify_on_discover();
}

// We call this later to avoid deadlocks on creating the server components.
_mavsdk_impl.call_user_callback([this]() {
// Send a heartbeat back immediately.
Expand All @@ -568,6 +562,9 @@ void SystemImpl::set_connected()
}
// If not yet connected there is nothing to do/
}

_mavsdk_impl.notify_on_discover();

if (enable_needed) {
if (has_autopilot()) {
send_autopilot_version_request_async(nullptr);
Expand All @@ -591,10 +588,10 @@ void SystemImpl::set_disconnected()
//_heartbeat_timeout_cookie = nullptr;

_connected = false;
_mavsdk_impl.notify_on_timeout();
_is_connected_callbacks.queue(
false, [this](const auto& func) { _mavsdk_impl.call_user_callback(func); });
}
_mavsdk_impl.notify_on_timeout();

_mavsdk_impl.stop_sending_heartbeats();

Expand Down
2 changes: 1 addition & 1 deletion src/mavsdk/plugins/info/info_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ void InfoImpl::enable()
{
// We're going to retry until we have the version.
_system_impl->add_call_every(
[this]() { _system_impl->send_autopilot_version_request(); }, 1.0f, &_call_every_cookie);
[this]() { _system_impl->send_autopilot_version_request(); }, 2.0f, &_call_every_cookie);

if (!_flight_info_subscriptions.empty()) {
// We're hoping to get flight information regularly to update flight time.
Expand Down

0 comments on commit e815b8b

Please sign in to comment.