mavlink icon indicating copy to clipboard operation
mavlink copied to clipboard

Published 20 hours ago • verified publisher icon mavlink

Common.xml - WIP message review

Open hamishwillee opened this issue 3 years ago • 28 comments

The following messages, commands, enums, enum values have been tagged as "Work in Progress" in common.xml/minimal.xml. WIP tagging is not intended to last forever, and under the current governance model messages that are "work in progress" should not be included in common.xml at all (they should go into development.xml until a prototype has been created, and they are approved for inclusion).

The proposal is to review all of these and either remove WIP or determine a timeframe for removing WIP. If no implementation exists or is being developed imminently, then these might alternatively be moved to development.xml.

Mixed bag propose go in as I believe their are implementations:

Command protocol - long running commands

ESC info - still in discussion - must remain WIP.

Orbit

Event Protocol

Unordered leftovers

Camera and Gimbal

In review/on hold pending dwalter update discussions related to combined Camera and Gimbal - as discussed in dev call: https://github.com/mavlink/mavlink/wiki/20210901-Dev-Meeting

Julian Oes to report back.

Gimbal Protocol

Note, the PITCHYAW are not mentioned in https://mavlink.io/en/services/gimbal_v2.html and possibly have some duplication. @julianoes can you check all of the above ended up actually being used?

Addressed cases

Cellular config

These are supported in PX4 for logging and in custom GCS for setting status via companion computer. Remove WIP.

Parameter transactions

Move to development.xml

OpenDroneID Protocol

Agree in dev call to keep as WIP. The messages are good and it is possible they will get an implementation.

Camera Protocol

Camera Tracking

hamishwillee avatar Aug 12 '21 01:08 hamishwillee

@auturgy @julianoes I have listed all the WIP entities in common.xml above. Are there any cases above where we can agree that WIP tag should be removed.

I think the stuff in "Mixed bag propose go in as I believe their are implementations" can go in, along with the "Camera Protocol" (zoom etc) and Orbit. I think we should hold off on the Event protocol for now. I think we need to discuss the Open Drone ID case.

Any disagreement.

@julianoes can you comment on the status of gimbal protocol and "Camera Tracking" messages? Also did the param transaction stuff end up being implemented. If not, I think we should remove it.

@auturgy What are your thoughts on my initial set to remove WIP on? Are there any others you think can definitely have WIP removed for?

hamishwillee avatar Aug 12 '21 01:08 hamishwillee

Looks sensible. I’d love to de-WIP opendroneID, but no public flight stack implementation that I’m aware of. I’ll look into AIS: we recently added lighthouses/fixed obstacles in the nmea library but not sure whether that’s captured in MavLink.

Regards,

James

On 12 Aug 2021, at 11:09 am, Hamish Willee @.***> wrote:

 @auturgy @julianoes I have listed all the WIP entities in common.xml above. Are there any cases above where we can agree that WIP tag should be removed.

I think the stuff in "Mixed bag propose go in as I believe their are implementations" can go in, along with the "Camera Protocol" (zoom etc) and Orbit. I think we should hold off on the Event protocol for now. I think we need to discuss the Open Drone ID case.

Any disagreement.

@julianoes can you comment on the status of gimbal protocol and "Camera Tracking" messages? Also did the param transaction stuff end up being implemented. If not, I think we should remove it.

@auturgy What are your thoughts on my initial set to remove WIP on? Are there any others you think can definitely have WIP removed for?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

auturgy avatar Aug 12 '21 02:08 auturgy

@auturgy OK, I'll do PRs for some of my "no brainers" then. How about I PR moving opendroneid into development.xml and we discuss that with the owner through that PR? Noting that until we have fixed the enum issue they won't be able to do too much with that.

hamishwillee avatar Aug 12 '21 04:08 hamishwillee

this is really fantastic, that this is happening!!

TUNNEL: I can confirm that it is implemented in the STorM32 gimbal controller, and is used

COMPONENT_INFORMATION: I still find it very sad that some basic fields with the most basic info on the component have been stripped and that these simple info needs to be retrieved going through all the mavftp heavy weight. Given that this is a very low frequency message it's not easy to rationalize why these fields can't be there and what the advantage of them stripped off should be. At the same time it has fields which easily could have been placed into a mavftp-retrievable json and thus appear totally superfluous, also contradicting an argument that there is some message size issue. So, a last shout here in the hope this might be rethought. :)

COMMAND_ACK: this is probably the MOST important of all of the above. Some commands are typically called at higher frequency (gimbal) and the missing targets, or that they are replaced by zeros for those systems which don't understand the extended version, leads to a quite significant spilling of the network. For the wired links this might not be considered terrible, but it is for the wireless link. It is illogical to attempt long-range MAVLink links but at the same time to flood it with totally superfluous - and totally easily avoided - COMMAND_ACKs. In fact, I yet fail to see any rational-based reasoning why one would not want this extension. So, this is IMHO long long long overdue. So, pl pl pl do it, and ArduPilot, pl pl pl implement it.

If there is some dispute over the other two extension fields, then pl simply un-wip the two target fields, and keep the other two as wip.

GIMBAL_DEVICE_INFORMATION: I can confirm that it is implemented in the STorM32 gimbal controller, and much used

AUTOPILOT_STATE_FOR_GIMBAL_DEVICE: I can confirm that it is implemented in the STorM32 gimbal controller, and much used

GIMBAL_DEVICE_SET_ATTITUDE: it is really sad that a consensus could not be reached for this message, for what from my perspective are really "harmless" things but which make it for gimbals like the STorM32 unfortunately totally impractical. In fact, if one compares with the analog message in the storm32.xml dialect (https://github.com/mavlink/mavlink/blob/master/external/dialects/storm32.xml#L410-L420) one can see that - besides some invalid attributes - there is a difference only in that the storm32.xml defines additional flags, two for getting along with the issue with absolute and relative yaw, and two for getting along with handling remote control inputs (https://github.com/mavlink/mavlink/blob/master/external/dialects/storm32.xml#L113-L146). In particular the flags concerning absolute/relative yaw are most important, since a gimbal like the STorM32 (and all other gimbals of similar logical construction, tarot etc) simply can't satisfy the current specification. So, a last shout here in the hope this might be rethought. :)

GIMBAL_DEVICE_ATTITUDE_STATUS: here too it is sad that a consensus could not be reached. If one compares with the analog message in the storm32.xml dialect (if one compares with the analog message in the storm32.xml dialect (https://github.com/mavlink/mavlink/blob/master/external/dialects/storm32.xml#L396-L409) one can see that there are only small differences. First, the flags has the 4 additional entries as described before. Second, the failure_flags has one additional flag for no manager (https://github.com/mavlink/mavlink/blob/master/external/dialects/storm32.xml#L147-L180). And third, the storm32 version has an additional yaw_absolute field. These are more differences than for GIMBAL_DEVICE_SET_ATTITUDE, yet here too a last shout. :)

it would have been nice if at least the gimbal device related messages would be harmonized (the gimbal manager related messages are not used by the STorM32 gimbal). I personally feel that the naming GIMBAL_DEVICE_CONTROL and GIMBAL_DEVICE_STATUS is also better, but that's arguably personal taste.

:)

olliw42 avatar Aug 17 '21 05:08 olliw42

@hamishwillee

Note, the PITCHYAW are not mentioned in https://mavlink.io/en/services/gimbal_v2.html

It looks like the messages/commands were changed from TILTPAN to PITCHYAW and the gimbal v2 documentation doesn't reflect that.

Although I do prefer the use of TILT/PAN when referring to gimbals to avoid overloading the term with aircraft roll and pitch - but it looks like that ship may have sailed in the protocol.

dlwalter avatar Aug 18 '21 22:08 dlwalter

Thanks @dlwalter and @olliw42 . The first set of changes merged (see checked items above).

I am keeping gimbal stuff WIP until @julianoes comes back. I "think" that the ship has indeed sailed for changes, but I'm not making that call.

@dlwalter Thanks. I've PRd in the changes inhttps://github.com/mavlink/mavlink-devguide/pull/375. If you see missing stuff you're welcome to do the same.

hamishwillee avatar Aug 19 '21 00:08 hamishwillee

The first set of changes merged (see checked items above).

FANTASTIC, especially that COMMAND_ACK made it ... really great

gimbal stuff ... I "think" that the ship has indeed sailed for changes

I understand that but I nevertheless wanted to give it a last try... one can't complain if one hasn't made the effort to speak up, right ;) LOL

olliw42 avatar Aug 19 '21 04:08 olliw42

Hi Everyone,

This is some awesome work! We've been trying to wrap our hands around the command/control aspect of smart gimbals using Mavlink for a while now, and I think this is the first post that made it clear all the new messages that are in play to help do this.

On the issue of removing WIP, I would strongly recommend that documentation for the gimbal and camera protocols be updated before removing the WIP notification. It would be a good sanity check to verify if it's ready to try and stand on its own. From a personal note, we were looking for some of the features that the Camera messages provide but since they weren't mentioned in the context of the Gimbal doc we missed them for several months!

It seems like the gimbal and camera parts of the message spec were developed as independent as possible, and I think that makes sense from a code topology perspective. But we need to be aware that one of the most powerful applications of drones involves a feature set that requires the two work together.

Just to ground my statement in a use example - consider how you would (A) command a UAS to look at a point on the map, (B) observe the point where the camera is actually looking on the map (aka Sensor Point of Interest). My point is in the process of commanding a "gimbal" using that protocol, the user might be looking for feedback from the "camera" using the other.

aerialhedgehog avatar Aug 20 '21 23:08 aerialhedgehog

also for what it's worth, the Pan-Tilt nomenclature is my preference also, for two reason: 1. it is how camera operators think of moving a camera, and two it disambiguates from aircraft control.

aerialhedgehog avatar Aug 20 '21 23:08 aerialhedgehog

@aerialhedgehog I have just posted a related issue concerning integrated gimbal/cameras here:

https://github.com/mavlink/mavlink/issues/1678

Glad to have anyone's input on that thread as well.

dlwalter avatar Aug 20 '21 23:08 dlwalter

It's great to have that discussion but I'd just like to clarify that there are no guarantees that any messages will have the tags removed, or that they won't be changed. Any use of WIP messages in production is at the risk of whoever is doing it (which is why we've now created development.xml, to make this clearer and harder to mess up). It's likely that a number of messages will have the tags removed, but equally likely that some will be moved to development.xml

auturgy avatar Aug 21 '21 02:08 auturgy

@aerialhedgehog @dlwalter Just FYI, the reason that I haven't progressed dealing with the gimbal/camera messages is that I am waiting on @julianoes to return from holiday (the gimbal stuff was developed by him and @olliw42). I don't know how well tested/stable this is yet. I HOPE that we can still modify/extend it if needed, but as @auturgy says, a) there are no guarantees on these b) we are improving our process to more clearly separate stable and development APIs. [I'd also like to separate out APIs like camera, gimbal into their own xml, to make them easier to manage and grok. There are toolchain issues that prevent that right now]

@julianoes Either way, it sounds like we need you/Ollie to revisit the docs and confirm they accurately match the API. Further, we probably should add a section that links to the camera API and in particular properly joins the camera and gimbal behaviour that is intrinsically linked (I'm thinking target tracking, but there might be others). Appreciate you'll be hammered for some weeks after you get back.

Perhaps we should plan this to give anyone who wants to be involved a realistic timeline and/or add a specific meeting to work through the details and make sure that this does do what is needed.

hamishwillee avatar Aug 22 '21 22:08 hamishwillee

Ideally the camera definition file would be deprecated and component definitions used for every component that needs such a thing. But in my view the component service isn’t sufficiently settled, so we can’t do that just yet. Pulling the microservices out into their own xml’s would be a good thing, when the infrastructure supports that.

On 23 Aug 2021, at 8:37 am, Hamish Willee @.***> wrote:

 @aerialhedgehog @dlwalter Just FYI, the reason that I haven't progressed dealing with the gimbal/camera messages is that I am waiting on @julianoes to return from holiday (the gimbal stuff was developed by him and @olliw42). I don't know how well tested/stable this is yet. I HOPE that we can still modify/extend it if needed, but as @auturgy says, a) there are no guarantees on these b) we are improving our process to more clearly separate stable and development APIs. [I'd also like to separate out APIs like camera, gimbal into their own xml, to make them easier to manage and grok. There are toolchain issues that prevent that right now]

@julianoes Either way, it sounds like we need you/Ollie to revisit the docs and confirm they accurately match the API. Further, we probably should add a section that links to the camera API and in particular properly joins the camera and gimbal behaviour that is intrinsically linked (I'm thinking target tracking, but there might be others). Appreciate you'll be hammered for some weeks after you get back, so perhaps we should plan this to give anyone who wants to be involved a realistic timeline. ...and/or add a specific all to work through detail.s

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

auturgy avatar Aug 23 '21 01:08 auturgy

Ideally the camera definition file would be deprecated and component definitions used for every component that needs such a thing

maybe, maybe not. In principle there is really no problem with having a component information to retrieve various more general things for all components and having component specific camera information, gimbal information, and so on. (and recall comp info was actually supposed to help GUIs with some metadata and not to be an all purpose storage class, one can find good counter arguments for the latter)(but sadly it seems to go in that latter direction) (I think we also should start to send all data, like attitude etc pp, via component information ... this would avoid msgid clashes and all the issues with the extra crc and more advantages)(just making fun of course...)

In addition I actually hope it won't. In addition to the above (https://github.com/mavlink/mavlink/issues/1669#issuecomment-899999622) I'd like to reproduce what I just wrote in another thread: "this sounds compelling but given the deficiencies of component information in its current form (and I suspect also future form) I seriously hope it won't... (my 2 cents)(recall that component information initially was drafted similarly to camera information but most bits there then removed, so it isn't a 1to1 substitute)" (https://github.com/mavlink/mavlink/issues/1680#issuecomment-903384633)

:) :)

olliw42 avatar Aug 23 '21 02:08 olliw42

I commented on https://github.com/mavlink/mavlink/issues/1669#issuecomment-903393762. In summary I somewhat agree.

The camera info message was designed to enable basic camera info via normal mavlink messages, with access to more advanced features via information files.

The component information file started the same way - but when we needed more space for the component info we dropped the basic information. That made sense for GCS which probably wants to implement mavftp anyway, but perhaps is not suitable for gimbals and other simple components.

What we probably should have done is kept the old component info message and had a new message like COMPONENT_META for this feature. At that point you have the best of both worlds again. I don't think we can change COMPONENT_INFO, but perhaps we can add a differently message that does what you need. If you PR it I'll happily champion it in the dev call.

hamishwillee avatar Aug 25 '21 01:08 hamishwillee

EDITED: DISCUSSION MOVED TO https://github.com/mavlink/mavlink/issues/1686

@olliw42 Very relevant discussion to have, but a bit off topic. I have moved

olliw42 avatar Aug 25 '21 02:08 olliw42

CELLULAR_CONFIG message is mark as done but has a WIP tag: https://github.com/mavlink/mavlink/blob/f2a61dc48d1f056db7e0e2ab4d885bb726b7e57a/message_definitions/v1.0/common.xml#L6659-L6662

python36 avatar Nov 02 '21 10:11 python36

@python36 Thanks- removed in https://github.com/mavlink/mavlink/pull/1736

hamishwillee avatar Nov 03 '21 01:11 hamishwillee

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

stale[bot] avatar Apr 16 '22 09:04 stale[bot]

Are there any updates on camera tracking functionality?

damurashov avatar Oct 28 '22 11:10 damurashov

Nope. A lot of these are WIP because they are in common and deployed, but only on one flight stack - so they aren't particularly standard.

In the case of camera, PX4 and QGC have support, but Mission planner achieves similar things in different ways that predate it (I think ArduPilot does MAVlink passthrough, so will still work with a compliant GCS, but doesn't do anything with it directly). Upshot, we've been dragging our feet on these. It came up again in the last dev call. I'll put in a PR to remove these from the camera API and see if I can get agreement in the next week.

hamishwillee avatar Nov 01 '22 22:11 hamishwillee

Thank you! That's really good news for me, because I was thinking on using those, but hesitated because of "WIP" tag.

I'll cross-reference this and that (https://groups.google.com/g/mavlink/c/riVSIkXLek0) threads to provide a context for those who follow.

damurashov avatar Nov 02 '22 06:11 damurashov

@damurashov When you say thinking of using those, do you mean "to call" or implementing in a mavlink camera?

Because it may be I was wrong, and that these have not yet been implemented in any cameras. I am exploring in https://github.com/mavlink/mavlink/pull/1909

hamishwillee avatar Nov 02 '22 22:11 hamishwillee

@hamishwillee Hello! Not quite sure what you meant, so let me give you the entire picture. I port an object tracking algorithm on ESP32 chip mounted on a drone, using MAVLink as the protocol for accessing the tracking functionality (providing an API for a user, i.e. application programmer). Using UDP / Wi-Fi stack as the transport, the chip is supposed to react upon incoming MAVLink messages according to the message exchange scheme implied by the protocol, and provide the user with the relevant info on tracking status.

The way it works is that I use a C MAVLink library generated by mavgen. It merely provides me with serialization / deserialization API ("pack", "unpack", and structure definitions). All the rest is my responsibility. So I receive MAVLink packets over UDP, process them, send responses (if any were needed) and relevant telemetry messages.

The user will operate with the following messages:

  • MAV_CMD_CAMERA_TRACK_RECTANGLE to set the ROI for the tracking algorithm implementation;
  • MAV_CMD_CAMERA_STOP_TRACKING to stop the tracking process, if any takes place at a moment;
  • CAMERA_TRACKING_IMAGE_STATUS requested by MAV_CMD_SET_MESSAGE_INTERVAL to acquire the status.

The choice I am facing now is that (1) I either use the tracking-related part of the API for message serialization / deserialization, (2) or find a workaround through re-purposing other MAVLink messages (like DEBUG_VECT).

damurashov avatar Nov 05 '22 07:11 damurashov

Thanks. You will be able to do "1". I'm going to remove WIP as there are implementations and this has been "approved". Just haven't merged it yet (want to do a final sanity check) - probably when next I work on this stuff, which is Wednesday.

hamishwillee avatar Nov 06 '22 05:11 hamishwillee

That's great, thank you!

damurashov avatar Nov 07 '22 06:11 damurashov

@damurashov It was done :-). Looking forward to seeing your implementation floating around!

hamishwillee avatar Nov 08 '22 22:11 hamishwillee

@hamishwillee Great news! Thank you!

damurashov avatar Nov 09 '22 06:11 damurashov