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

sample coro #2126

Closed
wants to merge 1 commit into from
Closed

Conversation

jperez-droneup
Copy link

No description provided.

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.

Interesting! It would be nice indeed to have most (if not all) work on one thread and get rid of all the locking. MAVSDK isn't really doing any compute but just IO. And coroutines could be one way to do that.

By the way, I assume you're aware: the API files like action.h is generated, so eventually changes would have to be in the template. And some of the promise/future stuff is right now in the impl, so you could move your implementation there:
https://github.com/mavlink/MAVSDK/blob/main/src/mavsdk/plugins/action/action_impl.cpp#L55-L63

One more comment: we need to check when/if we can switch to C++20 because we have to compile for all sorts of architectures (e.g. cross compilation for Android, RPi, etc.), so we need to check what compilers are available there.

@jperez-droneup
Copy link
Author

jperez-droneup commented Sep 1, 2023

I wasn't sure if this would be visible, how embarrassing haha. Yeah I am experimenting to see if there was an easy way to adapt at least the _aync functions to work with coroutines. We managed to get it working on our end with boost::asio, so it should be possible to port over eventually. Though of course the real goal would be to drop the threading done in MAVSDK and to use coroutines instead. This was just an attempt to shim it to allow clients to call in with coroutines.

If I can get something working super basically in the action.h, then yeah I would generalize it and put it into the template. I wanted to see if it was even possible/feasible using coroutines across threads and with the MAVSDK callback structure.

@julianoes
Copy link
Collaborator

I'll close this PR but let me know when you find more or have suggestions.

@julianoes julianoes closed this Nov 19, 2023
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