mavlink / MAVSDK

API and library for MAVLink compatible systems written in C++17
https://mavsdk.mavlink.io
BSD 3-Clause "New" or "Revised" License
635 stars 508 forks source link

Rally points uploading mission. #938

Closed itrigitech closed 5 months ago

itrigitech commented 4 years ago

I need to upload missions with all new features like rally points. My vehicle is a vtol so I need to have rally points for long mission. Is it implemented? Or someone is thinking to implement this?

JonasVautherin commented 4 years ago

Rally points are not implemented in MAVSDK, though it could be added.

A first step would be to explain here how that's supposed to work:

hamishwillee commented 4 years ago

@JonasVautherin The rally point sequence is exactly the same as the mission protocol except that you need to specify a mission_type of MAV_MISSION_TYPE_RALLY (instead of MAV_MISSION_TYPE_MISSION) in most messages and the payload is always MAV_CMD_NAV_RALLY_POINT.

There is a protocol bit you can check for support: MAV_PROTOCOL_CAPABILITY_MISSION_RALLY.

As to where it should go, the purist in me would like a separate plugin but the pragmatist says in mission plugin.

Ups to you.

hamishwillee commented 4 years ago

PS Geofence is also a variation on mission protocol.

julianoes commented 4 years ago

We should do some refactoring first and extract the mission protocol stuff (downloading and uploading) mission items into mavsdk and then re-use it in the mission, geofence, and rally plugin.

itrigitech commented 4 years ago

So do you suggest to create separate plugins? Conceptually i understand rally points and geofence as part of a mission as they are dependent of the mission items and the concret mission so in some way they go together. It's true that missionItems are different of rallyPoints but for me all of them belongs to a mission.

julianoes commented 4 years ago

Interesting take. I haven't thought about this in that way.

To me it's not necessarily connected. For instance I can fly using offboard but still require a geofence.

itrigitech commented 4 years ago

Yes, you are right. Depending the vehicle usage it can be thought in a way or another. I was thinking in the way i use the drone as i want to execute fixed missions and for me a mission includes everything. Rally points, geofence, etc. For a more generalistic use case probbly has more sense to split this and generate differents plugins though.

hamishwillee commented 4 years ago

@julianoes You're right in that the whole conceptual linking is rubbish - geofence and rally points apply everywhere, and the references to it being part of mission planning mean that isn't obvious.

The reason they get conceptually tied together is mostly because of the plan reuse. If you have a mission where geofence and rally points make sense then you want the plan file to include them too.

I don't think that we're obliged to mix the plugins; managing that would be a higher level operation.

JonasVautherin commented 4 years ago

Seems like nobody is strongly against having a separate plugin (my understanding is that it is the preferred choice). I would vote for having a RallyPoint plugin. It also makes it consistent with the already existing Mission and Geofence plugins.

@itrigitech would you be willing to start working on that? Feel free to open a draft pull request with whatever you get, even if it is just the header files. So we can comment on the interface :blush:.

JonasVautherin commented 4 years ago

Oh, just thought about @julianoes' comment:

We should do some refactoring first and extract the mission protocol stuff

That's actually true. Now I'm wondering how we would organize that. Have an src/flightplan folder that would contain something like:

CMakeLists.txt // This builds mission, geofence and rallypoint below
common // This contains the common protocol part
geofence // This is the actual geofence plugin, with its own CMakeLists.txt creating libmavsdk_geofence.so
mission // This is the actual mission plugin, with its own CMakeLists.txt creating libmavsdk_mission.so
rallypoint // This is the actual rallypoint plugin, with its own CMakeLists.txt creating libmavsdk_rallypoint.so

What do you guys think?

itrigitech commented 4 years ago

Ok when i have time i will try to start, i don't have so much free time.

@JonasVautherin for mee looks good what you propose.

Talking about the planning i see the current mission plugin only uploading waypoints. Do you know why is that? in my case i'm using MAVSDK in VTOL and i need take_off_and_transition mission item and transition_and_land. Without that i can't upload a mission.

There is any concrette reason for that? if not i will try to moddify this also to include this kind of mission items.

JonasVautherin commented 4 years ago

There is any concrette reason for that?

I believe we could add them. @julianoes an opinion on that?

hamishwillee commented 4 years ago

@JonasVautherin The list of currently supported mission items/corresponding commands is here: https://mavsdk.mavlink.io/develop/en/guide/missions.html#supported_mission_commands

It makes sense to add new VTOL mission items if needed, but didn't we decide that vehicle-specific functionality would be dealt with in vehicle-specific plugin variants?

JonasVautherin commented 4 years ago

but didn't we decide that vehicle-specific functionality would be dealt with in vehicle-specific plugin variants?

Indeed. Though I believe that was considering dynamically-loaded plugins, which we don't have yet. Also note that it should not be too difficult to move from one to the other.

hamishwillee commented 4 years ago

OK, Well if at all possible I think we should try to stick to our plan as close as possible :-)

itrigitech commented 4 years ago

Ok, so at the end what do you think?.

As I don't was in your previous discussion i don't understand exactly how you want to do this. Looking at the plugin, doesn't seem hard to add this kind of items but how do you want to do this if not? as i need it i will do it in not much time.

It's ok then to add it to the current mission plugin or not?

JonasVautherin commented 4 years ago

I would say let's add it to the current mission plugin, so that we get the MAVLink implementation already. And I will need to spend some time thinking about how to follow the plan :laughing:.

In short: yes, go ahead @itrigitech :+1:.

Bert-Tec commented 4 years ago

Is/have there been some work on this topic? I would appreciate if it is possible to add rally point handling to mavsdk.

Bert-Tec commented 4 years ago

Isn't it just possible to use the mission_raw plugin to upload a mission of MAV_MISSION_TYPE MAV_MISSION_TYPE_RALLY and have items of MAV_CMD_NAV_RALLY_POINT in it? Sure, not as nice as a plugin dedicated to it, but it should do the work.

julianoes commented 4 years ago

I would appreciate if it is possible to add rally point handling to mavsdk.

I can see how it might make sense to add a simple API to the mission plugin to add a couple of rally points. However, I'm a bit fuzzy on how rally points really work, especially depending on the RTL_TYPE param. I always want the API of MAVSDK to be as simple and safe as possible in order to prevent users from making mistakes or having unexpected behavior, so knowing how it all works, would be my first step.

And otherwise, we always welcome pull requests.

Isn't it just possible to use the mission_raw plugin to upload a mission of MAV_MISSION_TYPE MAV_MISSION_TYPE_RALLY and have items of MAV_CMD_NAV_RALLY_POINT in it?

@Bert-Tec I think it's currently not possible to upload a rally point using the mission_raw plugin because the overall mission type is hard-coded here: https://github.com/mavlink/MAVSDK/blob/2e95299c656b06a46cc9594b71a169cb6095f985/src/plugins/mission_raw/mission_raw_impl.cpp#L139. However, we could probably lift that restriction and infer the mission type from the provided items. (Note however that all items of one upload need to be the same, as far as I know.)

hamishwillee commented 4 years ago

@julianoes FWIW it makes perfect sense to implement the upload behaviour and it should be very easy to do - it is like mission upload with a different type set and just one "command". It makes no sense at all to try define how they are used because that is all outside spec - so the RTL_TYPE can all be ignored.

julianoes commented 4 years ago

so the RTL_TYPE can all be ignored.

From a protocol view, yes. From a usability view, no. When we add rally points we need to add a description of what it is or how it works. So we need to say something like "a rally point is an emergency landing spot that will be used if/when foo happens", and is that only for fixedwing, or VTOL in fixedwing mode, or also multicopter, and does it depend on RTL_TYPE? There needs to be a least one sane default behaviour that we can describe, and then refer to the description of RTL_TYPE for anything more complex.

hamishwillee commented 4 years ago

True. I take your point but I disagree because it makes a useful feature "forever unusable".

julianoes commented 4 years ago

I'm not saying I don't want it ever, I'm just saying that I need to either read into it and find a sane default or have someone explain that sane default with a clear use case and notes about caveats to me, best in a pull request.

hamishwillee commented 4 years ago

That can never ever happen. The only thing you can say is:

Rally points are alternative destinations that may be used instead of the home/launch locations in safe return modes (modes like "return to launch" and "return to home"). Typically if safe points are enabled a vehicle will return to the closest defined safe point when the safe return mode is engaged. However this is entirely defined by the flight stack implementation.

julianoes commented 4 years ago

That doesn't sound too bad @hamishwillee.

hamishwillee commented 4 years ago

In that case @julianoes, you're allowed to implement rally point upload :-)

steveb2014 commented 1 year ago

Did anyone end up implementing an easy way to upload rally points through mavsdk? Sure would be nice to an api!

SBHKoda commented 1 year ago

Is there any update on uploading rallypoints?

steveb2014 commented 1 year ago

I've not heard/seen anything...

julianoes commented 1 year ago

I'm sorry but this is just an open source project that I help maintain together with @JonasVautherin in my spare time. Feel free to contribute what you need. I would like to add this but I have a long list of things I'd like to do.

If you need something to be moved up on my todo list, you can always consider requesting my consulting services, just email me, and we can talk about it.

DonatelloX commented 1 year ago

I too am interested in this feature. I hope for some updates...

julianoes commented 5 months ago

That's implemented and working. Sorry for not updating this issue.