Using COMPONENT_INFORMATION to obtain vehicle metadata #1346
Comments
|
Don, thanks - really appreciate you taking the lead. We need you!
In the dev call we suggested that we might have just one hash and always query the whole file. The argument was that this is a pre-arm/non-flying query, so bandwidth consumption etc isn't relevant. However I don't think that is correct - this query gets made when the GCS connects, so it could happen when flying or any time. @auturgy @julianoes Did I misunderstand? My thinking is that giving it the GCS the choice to query parts of the file is more flexible - and nothing stops us from giving it an option to query everything?
This doesn't break anything, except that if we do this we would need to update each message that takes an index to specify the field for the index. That's one reason this exists - to minimise the churn on other docs to support the use case. To me this looks better.
Depends on what is agreed above. Some points though:
FYI only, we're currently working on first type bing "supported_mission_commands" Other ideas:
How do we currently get metadata, and why is this better? My concerns are size of info on firmware needed for this, and making sure that we can support translation. |
I don't see it as having much to do with bandwidth consumption. In fact if the vehicle is flying I would contend you want to skip doing this all-together possibly. I see it having to do with the amount of time it takes from connect to the vehicle being ready for use. The bigger the download is the slower that will be. I'm not going to want to download a pile of stuff over a SiK radio unless I really need to. Also by creating only a single entity you are increasing the likelihood of needing a re-download. Even if one smaller bit of it changes you then still need to change everything. I'd expect some of this stuff will be more stable than other parts and not change much. All of that said it can certainly work as a single entity.
Lorenz's mission command proposal is json so I went with that. In general I find json much easier to deal with though in code, also the the json schema stuff seems pretty straightforward as well.
I would contend that parameter meta is just as important since it already exists whereas "supported mission commands" does not exist in anyplace other than gcs code.
There is no duplication with my reworked COMPONENT_INFORMATION. My definition above is a completed replacement for the existing WIP defiuniction of COMPONENT_INFORMATION.
There is no mavlink spec as to how to get parameter metadata. It's all firmware specific. PX4 has a format for it and ArduPilot has a different format for it. Where you get it and how you use it in a GCS is different for each firmware. There is no mechanism for example to deal with a vehicle which has new firmware on it which may need new parameter metadata. You just hope for two things:
|
|
Thanks @DonLakeFlyer - lots of things I didn't know.. Do you think you might be able to attend a meeting specifically to lock all this down - ideally the MAVLink call, but if not, another? A few thoughts:
|
Sure, just give me a time and how. to connect. |
There is no reason why both JSON and XML can be supported. My opinion would be for new things which don't currently exist json is a better choice. |
Also I should have said that my proposal is essentially the data QGC stores for parameter meta data which was designed to support both current PX4 and ArduPilot metadata implementations. |
|
@DonLakeFlyer thanks!
I would use
I would not use a .zip archive but just compress the file stream with gzip, so it becomes
Makes sense, I agree. However, while we're talking about file size, we should consider localization which is currently possible for the camera definition file but not addressed here yet. In the ideal case where a couple of languages have been added this causes the file to blow up quite a bit (see example). Therefore, we could consider stripping unused languages before sending it if the request contains the favored language. |
|
I updated to switch to
There currently is a ton of un-loced json in QGC already, so it needs support for that anyway. The same thing would work in json since it's just a simple string matching table. Is there tooling around loc support for xml camera files? Or is it just people editing the file directly to add the loc strings? |
Yes that was just directly edited by Yuneec. |
|
Ok, I'll add loc to the json spec. I need it anyway for QGC so might as well get around to finally doing it! |
|
@julianoes @DonLakeFlyer The loc issue may actually make it necessary that we allow different definitions to be queried in different requests. Because we may wish to host localisations on the Internet and not mavftp. It would be good if we can find a scheme to specify that we want to find out what individual localisations of parameter (or other) metadata are available and query them separately. |
Got it. I'll work something up. |
|
I've updated the spec:
|
|
My latest update is missing the ability the request a specific language for the loc files. Which is likely needed to reduce download sizes. I'll work on something for that. |
|
Hold off on commenting on the Json localization support. I'm doing some more research to see how we can do this in a way that would allow us to translate these files in crowdin. |
@DonLakeFlyer what was your thought behind using an array for the params rather than a dict/map with the param_name as the index? |
Not quite sure either what the question is or how to answer. There isn't such a thing as a dict/map in json? You only have objects, arrays and values. |
|
|
Ok, understood. I think that ends up being a little more complex to traverse and write decoding code for it. I don't see it being quite as readable as well. Since it requires an extra level of indentation. If there some benefit to doing it that way? |
Propose "supportedDirectCommands": {
"all_modes": { //List the ones supported by all.
"23": {
"supportedParams": 8
},
"22": {
"supportedParams": 8
}
},
"offboard": { //List the ones supported by offboard that are no in all.
"33": {
"supportedParams": 1
},
}
}The other way to do this would be to include modes inside the message. Chose not to do that because if doesn't work if we ever have cases where the supported params depend on the mode. |
|
@DonLakeFlyer right, sorry dict/map was confusing. What I meant is just the normal object structure. So instead of: it would be: I would think that's easier to query because you can do: and don't need to loop through the array: Anyway, if you disagree, I'm fine with both. |
|
If you kept the items in a json object internally that would be correct. But for example with QGC that never happens with any json. The json is traversed and the data is stuffed into other various internal data structures and the json objects are thrown away. With that parsing arrays are easier to deal with than some sort of key type object map thing. I think that leads to odd looking json structure. In reality it's a 50/50 thing. Either way works since it's just data. But I do think readability of simple arrays is better, and less code to parse. It also lets you type and validate with schema better the key values (if they exist) I think since they don't need to be strings. They can be any type. Like ints for command ids. |
|
@julianoes I documented how QGC deal with all the command idiosyncrasies here: https://github.com/mavlink/qgc-dev-guide/blob/master/en/plan/MissionCommandTree.md. Some of that should come from mavlink command spec .xml file. But I think some needs to come from the command supported stuff. For example the concept of paramRemove which tells QGC not to show that params for the command. Which is essentially your "supportedParams" info. I haven't gone back to look at all of what QGC needs with respect to what else may be needed in the command supported metadata. Also with respect to "supportedParams". I think the way QGC lists the params to remove as a list is way better from a human readable standpoint. Trying to think through bitmasks in your head is painful. |
@DonLakeFlyer FWIW
@julianoes Thanks.
We will need to capture that in final docs.
The more information you can provide the UI the better it can control UI presentation (at the cost of complexity). Knowing this would certainly aid testing, but not sure how interesting it would be to a GUI app. My gut feeling is that if we built it then it will get used. It may be a little harder for a flight stack to generate the list though. @DonLakeFlyer Do you have an opinion on how useful would it be to GCS to know that a particular command is supported (or not) in a particular mode for an "unknown UI"? Are the any things that you only present if you know they are available in a mode?
Partially I just want to get a definition in, and I think we can do that more easily for missions. We can afford to iterate a little longer though.
Agreed. |
Not suggesting a change like that. Just suggesting to list the supported param indices instead of a bitmask.
An important point of Json and/or xml for that matter is to make the format human readable. That makes things easier to debug. Especially if these json files are going to be cached locally. That also provides a place where you can go and look at the contents of the file to check things. Instead of needing to crank up a debugger and step through the loading to gather more esoteric bits of information which is hidden behind computer friendly but not user friendly format. That is why if you look at the spec for the MAV_COMMAND info json in QGC you'll find things like command ids as ints, but also the standard is to include the command name as a string. That what you can scan the file and understand what you are looking at. If all you have is "16" for example, what command is that? It's included in the QGC command json spec as "MAV_CMD_NAV_WAYPOINT" as the rawName field. Which is super useful. QGC uses a ton of these json formats for internal metadata. And the fact that they are easily human readable has made my life easier with debugging for years now. This is the main reason I don't like the "dict" format as opposed to a simpler "array" format. To me it makes it harder to scan the content visually with your eyes. |
|
Also now that I have most of this working for real 70 chars for an http uri is useless. So you need to do a bitly or something to get a short url. The real question though is whether 70 chars is also useless for a mavlinkftp location on the vehicle? I don't know the answer to that. Firmware folks will need to chime in. |
|
Also on the uid's which are in COMPONENT_INFORMATION I'm thinking the scoping of the "uniqueness" of those ids is scoped to firmware type. So a PX4 Firmware uid which is the same as an ArduPilot firmware uid does not indicate they are the same. But it also means that for the same firmware, uids for one vehicle type and another uid for a different vehicle type refers to the same metadata if the uids are the same. This specs out the semantics of how you can cache these. |
|
@DonLakeFlyer Thanks.
It gets worse - there is some pull to put the version info back into the message itself COMPONENT_INFORMATION. This perhaps indicates that there is value in a separate COMPONENT_METADATA message. Some ideas?
|
I understand that you mean uids are unique across vehicles in a flight stack, but not across flight stacks. But which UIDs are you talking about? |
You either buy into the versioned json metadata concept, or you spend the rest of your life with |
The whole point is that a GCS should not need to know about the component in order to us the information. I'd be more fine with things differing by vehicle type than by flight stack. Of course that is ideology. So we would need to talk specific cases. |
Now that I've gotten further into the implementation I'm realizing this is wrong. That only works for metadata associated with the MAV_COMP_ID_AUTOPILOT1 component. For random other components it's trickier since the main firmware isn't necessarily in control of those. Those may need to take into account vendor and model names to know the scoping where those uids are unique.
Don't know what you mean by that. The GCS doesn't need to know anything more than what comes to it from COMPONENT_INFORMATION data. |
|
… On 2 Jul 2020, at 2:23 pm, Don Gagne ***@***.***> wrote:
Also on the uid's which are in COMPONENT_INFORMATION I'm thinking the scoping of the "uniqueness" of those ids is scoped to firmware type. So a PX4 Firmware uid which is the same as an ArduPilot firmware uid does not indicate they are the same. But it also means that for the same firmware, uids for one vehicle type and another uid for a different vehicle type refers to the same metadata if the uids are the same. This specs out the semantics of how you can cache these.
Now that I've gotten further into the implementation I'm realizing this is wrong. That only works for metadata associated with the MAV_COMP_ID_AUTOPILOT1 component. For random other components it's trickier since the main firmware isn't necessarily in control of those. Those may need to take into account vendor and model names to know the scoping where those uids are unique.
The whole point is that a GCS should not need to know about the component in order to us the information.
Don't know what you mean by that. The GCS doesn't need to know anything more than what comes to it from COMPONENT_INFORMATION data.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
|
One option would be to make the uids non-opaque. And instead use say a crc32 on the file as unique identifier. Then the uniqueness scope would be global. And it would be simple to cache these things in a GCS. |
|
You can see an example of Json parameter meta which I use in my test harness here: https://github.com/mavlink/qgroundcontrol/blob/master/src/comm/MockLink.Parameter.MetaData.json |
That makes sense to me. What would be the argument against? |
How would you validate this? Ie we have a key "type" that is also a keyword. {
"type": "object",
"properties": {
"version": { "type": "number" },
"uid": { "type": "number" },
"scope": { "type": "string" },
"parameters": {
"type": "array",
"items": {
"type": "object",
"name": { "type": "string" },
>>>>>>
"category": { "type": "string" },
"shortDescription": { "type": "string" },
"longDescription": { "type": "string" },
"units": { "type": "string" },
"defaultValue": { "type": "number" },
"decimalPlaces": { "type": "number" },
"minValue": { "type": "number" },
"minValue": { "type": "number" }
}
}
}
}Also, is the above OK (other than type)? i.e. are any of those values in a known set - e.g. units can only be certain types, etc? |
The schema definition is wrong. There is a "properties" definition missing under "parameters". I'll fix that. "units" can be any string but if you want conversions (metes->feet, ...) to work then you should use one of the mavlink supported types which I believe is spec'ed with the mavlink command spec someplace. |
|
@hamishwillee Wait, where did you pull that schema from? It isn't what is posted. What is posted above is correct: |
|
@DonLakeFlyer Thanks. I missed your schema and was trying to work out what it might look like. But, your schema above appears to be broken when tested in https://www.jsonschemavalidator.net/ What I want to do here is put the schema along with some examples under mavlink/mavlink. |
|
@DonLakeFlyer Can you help me fix that schema above? Also, would this metadata system also cover PARAM_EXT? (ie. it should). |
|
I'll fixed up the schema and put together a pull to stuff them in the places you talk about above.
I assume so. I don't know much about that though. |
|
We discussed this on the dev call today and would love to move this forward. |
|
I'm closing this issue. I'll create a new one to resolve the remaining issues with the |
This is the higher level proposal related to #1339 for how you get the various pieces of metadata which a GCS needs from a connected vehicle. I've also included my proposed spec for the parameter metadata json format.
Vehicle metadata request protocol
MAV_CMD_REQUEST_MESSAGE.param1 = COMPONENT_INFORMATIONMAV_CMD_REQUEST_MESSAGE.param2 = COMP_METADATA_TYPE_VERSIONto the vehiclecomp_metadata_uidis not already cached GCS does ftp download from specified vehicle uri.MAV_CMD_REQUEST_MESSAGE.param1 = ...for each optional metadata it wants. Including the cache dance semantics.MAV_CMD_REQUEST_MESSAGENotes:
COMPONENT_INFORMATIONNotes:
COMP_METADATA_TYPE_VERSIONtype metadata file spec.MAV_CMD_REQUEST_MESSAGE.param2uint32_ttime_boot_msuint32_tcomp_metadata_typeCOMP_METADATA_TYPE).uint32_tcomp_metadata_uidchar[140]component_metadata_urimavlinkftp:\\file is downloaded from vehicle over mavlink ftp protocol. If prefixhttp[s]:\\file is downloaded over internet. Files are json format. They can end in.gzto indicate file is in gzip format.uint32_tcomp_translation_uidchar[140]component_translation_uricomponent_metadata_uri. Files are in Json Translation spec format.MAV_CMD_REQUEST_MESSAGE.param1specifies requestedCOMP_METADATA_TYPE.COMP_METADATA_TYPE (enum)Support for
COMP_METADATA_TYPE_VERSIONis required all other types are optional.COMP_METADATA_TYPE_VERSIONCOMP_METADATA_TYPE's. Must be supported. Only downloadable from vehicle.COMP_METADATA_TYPE_PARAMETERCOMP_METADATA_TYPE_COMMANDSCOMP_METADATA_TYPE_...Metadata file formats
Notes:
COMP_METADATA_TYPE_VERSIONNotes:
COMP_METADATA_TYPEinformation.COMPONENT_INFORMATIONspec.COMP_METADATA_TYPE_PARAMETERNotes:
MAV_TYPE. But now is the time to extend to provide additional information beyond that simplest of info. This should follow what is happening with respect to defining additionalCOMMAND_LONGparameter metadata.Json Translations Format
String translations for a metadata file can either be included directly in the metadata file, be in a standalone file or in both places. If in both places, the external translations supercede the internal transations. If they are included directly in a metadata file that file would have a top level "translations" object which follows this spec.
A case where it is better the include them in both places is for long strings. For example the longDescription key in a parameter metadata file can contain quite a long string. You don't want to have to duplicate that long string in all the translations.original instances. Instead you do the following:
Instead of putting the actual long description for the parameter in the metadata you put a unique string which help to describes the string context:
"longDescription": "MAV_SYS_ID-longDescription"Within the parameter metadata file there is a "translations" section for "en_US" locale which translates from the unique string to the real english description:
And then the standalone translations file also uses the
"MAV_SYS_ID-longDescription"reference as the "original" string for it's translations. But the person doing the translations can still go back to the english text to base there translation on. This also prevents needing to update the translations when the english version of the string changes. Which begs the question as to whether this mechanism should always be used.The text was updated successfully, but these errors were encountered: