Skip to content
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

Events plugin #2338

Merged
merged 9 commits into from
Jul 15, 2024
Merged

Events plugin #2338

merged 9 commits into from
Jul 15, 2024

Conversation

bkueng
Copy link
Member

@bkueng bkueng commented Jun 12, 2024

This brings a plugin for the events protocol (https://mavlink.io/en/services/events.html), allowing to listen for events and arming checks.

  • It is using the component metadata, and therefore moves the implementation to the core, so it's usable for plugins.
  • Adds an example for the component metdata & events

@bkueng bkueng force-pushed the events branch 3 times, most recently from dfa7422 to 4703ec2 Compare June 13, 2024 13:47
Copy link
Collaborator

@julianoes julianoes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing, thanks for this! I'll do a more in-depth review of the events internals when CI is ready, and after having reviewed the events API.

if(NOT MSVC)
add_compile_options(component_metadata PRIVATE -Wall -Wextra)
else()
add_compile_options(component_metadata PRIVATE -WX -W2)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No more -WX please.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's gone

Comment on lines +103 to +107
std::cout << "All Reports:" << std::endl;
for (const auto& problem : report.all_problems) {
std::cout << " [" << problem.log_level << "] [" << problem.health_component
<< "]: " << problem.message << std::endl;
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing, that's great to have!

@@ -82,11 +82,11 @@ TEST_F(CurlTest, Curl_DownloadFile_WithoutProgressFeedback_FileNotFound)
TEST_F(CurlTest, Curl_DownloadFile_ProgressFeedback_Success)
{
int last_progress;
Status last_status;
HttpStatus last_status;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops, thanks!

Comment on lines +27 to +37
message(STATUS "Preparing external project \"libevents\" with args:")
foreach(CMAKE_ARG ${CMAKE_ARGS})
message(STATUS "-- ${CMAKE_ARG}")
endforeach()

ExternalProject_Add(
libevents
GIT_REPOSITORY https://github.com/mavlink/libevents.git
GIT_TAG 9474657606d13301d426e044450c4f84de2221be
SOURCE_SUBDIR libs/cpp
CMAKE_ARGS "${CMAKE_ARGS}"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, that seems pretty clean like that!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I spent a bit of time fiddling with cmake until it all worked.

@@ -1,5 +1,6 @@
#include "integration_test_helper.h"
#include "mavsdk.h"
#include <algorithm>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GCC 14 🤯

@bkueng
Copy link
Member Author

bkueng commented Jun 14, 2024

I'll do a more in-depth review of the events internals when CI is ready, and after having reviewed the events API.

android-x86 had some unrelated request timeout, let's see if it passes now.

Copy link

Quality Gate Passed Quality Gate passed

Issues
108 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@julianoes
Copy link
Collaborator

Looks like just a fix_style required now...

@bkueng
Copy link
Member Author

bkueng commented Jul 15, 2024

I think it's because the proto submodule isn't merged/updated yet. At least when I ran it last I didn't get any diff.

bkueng and others added 9 commits July 15, 2024 19:48
When moving the component metadata to core, the following error was raised:
MAVSDK/src/mavsdk/plugins/camera/camera.cpp:18:7: error: conflicting declaration ‘using mavsdk::Status = struct mavsdk::Camera::Status’
   18 | using Status = Camera::Status;
      |       ^~~~~~
In file included from MAVSDK/src/mavsdk/core/curl_wrapper.h:6,
                 from MAVSDK/src/mavsdk/core/http_loader.h:9,
                 from MAVSDK/src/mavsdk/core/mavlink_component_metadata.h:5,
                 from MAVSDK/src/mavsdk/core/system_impl.h:5,
                 from MAVSDK/src/mavsdk/core/plugin_impl_base.h:2,
                 from MAVSDK/src/mavsdk/plugins/camera/camera_impl.h:8,
                 from MAVSDK/src/mavsdk/plugins/camera/camera.cpp:7:
Small wrapper to create unique ID's.
Avoids the need for MavsdkImpl and CallbackListImpl to have private access
to Handle.
This follows mavlink_ftp_client, so that other plugins can access component
metadata as well.
The Plugin API remains unchanged.
Fixes errors in the form of:
MAVSDK/src/mavsdk/core/call_every_handler.cpp:33:20: error: ‘find_if’ is not a member of ‘std’

This is with GCC 14.1.1
This is to avoid locking us out when doing subsequent downloads. For
some reason this locked up on certain platforms.
@julianoes
Copy link
Collaborator

Merged proto, rebased, fixed up and force pushed. Let's hope this clears CI.

@julianoes julianoes merged commit 8d42ed8 into mavlink:main Jul 15, 2024
31 checks passed
@bkueng bkueng deleted the events branch July 23, 2024 09:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants