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

Generalizing MAVSDK for non-autopilot control #1023

Open
coderkalyan opened this issue Mar 29, 2020 · 17 comments
Open

Generalizing MAVSDK for non-autopilot control #1023

coderkalyan opened this issue Mar 29, 2020 · 17 comments

Comments

@coderkalyan
Copy link
Contributor

Traditionally, MAVSDK has been used to connect solely to a drone in order to control it; connecting to other peripherals has been more of an addon than a main feature. While this is definitely the main use case of MAVSDK, I think it would be nice to be able to use MAVSDK as a more generic high level platform for any MAVLink communication. This would increase the number of use cases and platforms MAVSDK can be deployed to, and also allow some very interesting new possibilities. Below are some comments on what the current state of affairs is and what can be done to improve this without sacrificing ease-of-use.

The base Mavsdk class define the source system and component, which defines the local side of the communication. With the recent addition of the Configuration class, this is fine - the system and component ID can be changed without restrictions, and there are sane defaults. From here on, though, things get a bit more complicated.

The System class, although designed to connect to any MAVLink compatible system, is in reality only tailored to connect to a single autopilot. This causes problems, obviously, when connecting to anything other than the autopilot. Peripherals such as gimbals and cameras are supported; however, they are all connected to the parent System rather than a parent Mavsdk, which makes more sense to me. As a result, you need to connect to an autopilot before getting access to any peripherals. Additionally, although MAVSDK can be used to control an autopilot from a companion computer, how MAVSDK handles connecting to a different component on the same system is uncertain and definitely undocumented.

To be completely honest, from looking into the core source code, I feel the code to handle system and component ids is very messy; a good portion of it is probably very old code, but I think we can do better. For example, there is a lot of hard-coding "let's throw away messages from these ids", "lets assume this component ID means this", etc. This leaves very little room for anything that deviates from the standard "I have a ground station which connects to my drone, both with standard IDs." I'd like to be able to use MAVSDK for more complex things including intra-system (inter-component) communication. I'm happy to take charge of cleaning some of it up + documenting it, if we can agree on what to do! For starters, I think we should all read https://mavlink.io/en/guide/routing.html 😄

I'm not sure the best way to do this, since I don't want to break the current API (create a System from a Mavsdk, create plugins from the System). One idea I had was to create a Component or similar, which is a more high level representation of a target that MAVSDK can connect to. The Mavsdk class can then search for and connect to multiple Components, and the existing System would be a type of Component (not sure if System is even the right word for it, in that sense, but I'm hesitant to break the current API). Or we could rename System to Component, and enable different plugins based on the type of Component, such as Autopilot, Companion Computer, Gimbal Device, Camera, custom component, etc.

Anyway, let me know what you guys think! Again, I'm happy to implement the changes!

@Mapleguy
Copy link

In theory this would be useful. This is something I'm somewhat looking into soon myself, but for work. That's sorta the catch. For my personal use, I don't have a need for this, what we have is fine. For work, well, we have somebody (me) who is supposed to overcome this and tailor a solution for our very specific needs. I'm sure it could be done, but the question is if it's worth the effort for the amount of hobbyist who can't work full time to make a solution or for companies that want some time savings if they happen to need this feature.

@coderkalyan
Copy link
Contributor Author

@Mapleguy thats an interesting point, thanks for sharing! Personally, my only work is in a hobbyist fashion, as well as a competition that my friends and I are looking to participate in. Either way, its not for profit/companies/professional. I'm just looking to improve MAVSDK, however I can, and I'd rather spend the time to write the mavlink stuff once and keep my deployed code as clean as possible!

@julianoes
Copy link
Collaborator

I feel the code to handle system and component ids is very messy; a good portion of it is probably very old code, but I think we can do better.

I don't disagree 😄. Yes, there are some hacks in place for the common ground station <-> autopilot case, and it would be nice to get rid of them and make it more generic!

To me, the concept around the System makes sense even if the System is not always an autopilot. So, for the case where MAVSDK is used on a companion computer and should detect the ground station, the System has a GroundStation component and no Autopilot component. And that should "be allowed".

@julianoes
Copy link
Collaborator

@Mapleguy I'm not sure what you mean with hobbyist vs. professional use. In the end this is an open source project and contributions and collaborations from hobbyists and professionals should be equally valuable and endorsed.

Maybe you're saying that you are not able to contribute back "at work" because it's not allowed in which case I would try to challenge that internally and convince them that it is:

  1. the right thing to do because you are probably benefiting a good deal from open source projects and it's good to give back.
  2. you can have an influence in an open source project (like in the discussion here) which means features that your company need will get done earlier or more inline with what you need.
  3. it makes it easier/cheaper in the long run because you don't need to maintain what you contributed back. (Or said differently you don't need to keep fixing merge conflicts later.)

Anyway, this is a bit of a meta-discussion. Let's get back to the problem at hand...

@coderkalyan
Copy link
Contributor Author

@julianoes Understood, but the only reason I'm hesitant to call it a System is because if we go by the book of how routing works in MAVLink, everything on a drone (autopilot, companion computer, camera(s), gimbal(s), are all different "components" in the same "system". Going by this, if we were to initiate direct communication between a companion computer and a camera, it would be a bit weird... to me Node makes more sense as a name...

the System has a GroundStation component and no Autopilot component. And that should "be allowed".

Anything in particular you would like to see in this? Or how you would have it structured?

This has been a pet peeve of mine for a while now, as I was looking to use MAVSDK for some intra-system MAVLink messaging and was forced to fallback to raw mavlink due to routing issues. Would be nice to see this happen, and as I said earlier, I'm happy to implement this, just tell me what we want to do 😉

@Mapleguy
Copy link

@julianoes There’s some I may eventually be able to contribute from what I learn at work, but ultimately not sure since, again, I don’t use MAVSDK on my own, really. Not right now at least. Details on why not just contribute from work says more about who I work for than I’m comfortable saying.

As for the professional vs hobbyist thing, my original comment was maybe a bit silly. A feature’s a feature. I think this would be good, but I’m still curious how useful it is. I’d give people who use MAVSDK more in its stock form more weight in this. I’d rather see better documentation, build tools, small support stuff than big features. But that’s only one use case that might stand alone in that.

@coderkalyan
Copy link
Contributor Author

@Mapleguy No worries. I brought this up mainly because I have been designing a modular ground control system for talking to MAVLink drones, and I'd like to use MAVSDK to power that + contribute it into MAVSDK when its ready.

@julianoes Please let me know what your thoughts are, I'm going to go through the System class to try to figure out how we can clean it up, but I don't want to do something clunky thats not going to last.

@JonasVautherin Since you're a maintainer too 😉 do you have any thoughts on this?

@julianoes
Copy link
Collaborator

Going by this, if we were to initiate direct communication between a companion computer and a camera, it would be a bit weird...

Hm, right. So you're saying we need some sort of configuration saying that we are part of a system. Or at least that our system is the same as the one we're talking to.
I haven't thought about that I guess.

Anything in particular you would like to see in this? Or how you would have it structured?

I'm not sure I have a good suggestion just yet. I'll do some thinking but feel free to make suggestions.

@coderkalyan
Copy link
Contributor Author

Okay there are probably less radical solutions, but I'm just throwing this out there:

So starting with Mavsdk as the client, we can connect to a Node, which would be anything sending a heartbeat which has a unique system and component ID. Then we can have multiple types of Nodes, including the most obvious, Autopilot. We can also have a Gimbal and Camera node, for example. Plugins that interact with a node can then connect to any kind of node, and some plugins may or may not be able to interface with a node depending on its characteristics.

@Katawann
Copy link
Contributor

I like the concept of Node. I am using MAVSDK on a companion computer for my company and at some point I would like to send some information to the ground station without the need to use the system Autopilot. Same for a camera that will do vision recognition

@coderkalyan
Copy link
Contributor Author

@Katawann @julianoes I spent a couple hours writing up a proposal outlining the problems and my proposed solution: https://docs.google.com/document/d/1UKXxG3Ph_9Nqws4hybiA0PPDo6L2PUYLB7mp_EwvI54/edit#. Please take a look and comment! Note that it's a WIP, and covers most (but not all) of what needs to be done.

It uses the concept of a Node that I discussed earlier.

@Katawann
Copy link
Contributor

Totally inline with my further development where the camera mounted on one drone could be used by another drone flying in the same area. Rather than to connect to the drone with the camera I could just connect to the node CameraNode and be used by any Systems on the same network

@coderkalyan
Copy link
Contributor Author

@Katawann glad you approve! Do you mind commenting on the doc where you see fit so we can improve the proposal?

@julianoes
Copy link
Collaborator

Very nice write-up, see some small inline comments.

Overall, I like the proposal. I have, however, one concern with it, and that's backwards compatibility. So, it sounds great to just change system to node and go ahead with many of the suggested changes, however, we need to find a way to not break existing implementations. One way might be to add the Node but keep the System, at least for now.

@coderkalyan
Copy link
Contributor Author

@julianoes I had the same concern, I'm wondering if we can reimplement System on top of Node, maybe just typedef it to Autopilot node or something like that

@julianoes
Copy link
Collaborator

Do you think we should do more here, and if so, what specifically?

@coderkalyan
Copy link
Contributor Author

Let me get back to you on that; I haven't had a change to follow up with all of MAVSDK's changes implemented recently, including the revised system connection API. I think there is still some room for improvement but it has definitely gotten a lot better since I first filed the issue...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants