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

[WIP] common.xml: add new frame MAV_FRAME_GLOBAL_VEHICLERELATIVE_ALT #2100

Conversation

peterbarker
Copy link
Contributor

this is expected to be used to tell a vehicle to take off to 20m above current altitude.

ArduPlane already actually does this for some commands, but uses an inappropriate altitude frame.

Should also be useful for "go up 20m".

ArduPilot's PR is here: ArduPilot#358

For discussion at this point only, please. Need to make sure this is fit-for-purpose in the ArduPilot codebase :-)

@hamishwillee hamishwillee changed the title common.xml: add new frame MAV_FRAME_GLOBAL_VEHICLERELATIVE_ALT [WIP] common.xml: add new frame MAV_FRAME_GLOBAL_VEHICLERELATIVE_ALT Apr 3, 2024
@hamishwillee hamishwillee marked this pull request as draft April 3, 2024 05:24
@hamishwillee
Copy link
Collaborator

@peterbarker I think it's a great idea. @julianoes @auturgy ANy thoughts?

FYI, have converted to draft and added WIP to title to stop me trying to merge it. Move to ready for review when it is ready to merge.

this is expected to be used to tell a vehicle to take off to 20m above current altitude.

ArduPlane already actually does this for some commands, but uses an inappropriate altitude frame.

Should also be useful for "go up 20m".
@peterbarker peterbarker force-pushed the pr/VEHICLERELATIVE_ALT-mavlink/mavlink branch from f9dafee to a473018 Compare April 3, 2024 06:34
@peterbarker
Copy link
Contributor Author

This one got yelled down at ArduPilot DevCall - Randy doesn't like it.

Within the ArduPilot code Plane is using MAV_FRAME_LOCAL_OFFSET_NED (it only needs the altitude); that's a frame which moves with the vehicle, so "D" should be relative to vehicle current location. Perhaps that will have to do for now....

@hamishwillee
Copy link
Collaborator

hamishwillee commented Apr 10, 2024

What @rmackay9 is saying will of course "work" but I don't like it very much. Mostly because if you're working in global frames it isn't likely to be obvious that you need to switch to this local frame to get the desired behaviour. Also you may have to do lots of messing around to work out correct lat/lon mapping from your local frame. Doable, but harder than it needs to be.

But MAVLink doesn't invent things without an interested party who wants to implement them. So if you're not going to do this, let me know and we can close this.

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.

I'd like this one.

@rmackay9
Copy link
Contributor

Yeah, sorry, I'm really not a fan of this one. I'm happy with the general idea of adding support for altitudes and Locations specified as an offset from the vehicle but we should not underestimate the amount of effort required to do that. The safety feature in AP that led to this PR doesn't really need the new altitude frame.

@hamishwillee
Copy link
Collaborator

Yeah, sorry, I'm really not a fan of this one. I'm happy with the general idea of adding support for altitudes and Locations specified as an offset from the vehicle but we should not underestimate the amount of effort required to do that. The safety feature in AP that led to this PR doesn't really need the new altitude frame.

Thanks @rmackay9

From my perspective this is a clear improvement on the "API definition". If we added this there would be no requirement for you to implement it - the other frame is valid, it's just nothing that anyone would ever think of when commanding a vehicle or setting a mission item.

But, we normally wouldn't do so unless there is interest from at least one flight stack to implement. Otherwise it is clutter, because there is no easy way (at least in missions) for users to know what frames are supported by a flight stack.

@peterbarker @julianoes Unless you have some intent to implement or know someone who might, we can close this?

@auturgy
Copy link
Collaborator

auturgy commented Apr 24, 2024

The concept is generally supported, however with no commitment to implement it should be closed.
Please re-open if I have mis-read the state of this.

@auturgy auturgy closed this Apr 24, 2024
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.

5 participants